The default bug view has changed. See this FAQ.

The .message of a DOM Worker error event is not populated when the worker does |throw new Error("data");|

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: jorendorff, Assigned: mrbkap)

Tracking

Other Branch
Points:
---

Firefox Tracking Flags

(status1.9.2 .4-fixed, status1.9.1 .10-fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

7 years ago
<!DOCTYPE html>
<html>
<head>
<script>
    var w = Worker("fail.js");  // throw new Error("fail") on first message.
    w.onerror = function (event) {
        alert("event.message is:" + uneval(event.message) + "\n" +
        "event.filename is:" + event.filename + "\n" +
        "event.lineno is:" + event.lineno);
    };
    w.postMessage("hello");
</script>
</head>
<body>
</body>
</html>

Expected: The worker's onerror handler should receive an event with
event.message === "data".

Observed: event.message === "".

Filing in JavaScript Engine for now, because bent took a look and says the JSErrorReport's ucmessage field is NULL. I'm not sure what that field is supposed to mean; perhaps not what http://mxr.mozilla.org/mozilla-central/source/dom/src/threads/nsDOMThreadService.cpp#553 thinks it means. If so, it's still partly our bug since this isn't documented anywhere and I should at least be able to tell bent what the right thing would be.
To clarify, the |aMessage| param is correct in this case, but the |ucMessage| member of the error report is null. Should I always be using |aMessage| whenever possible?
(Assignee)

Comment 2

7 years ago
Created attachment 437939 [details] [diff] [review]
Proposed fix

IMO this is just a bug -- we should fill in report->ucmessage whenever we can (especially to avoid chopping wide characters passed to the Error constructor). I still think that the worker code should deal with not having a ucmessage, and fall back onto using |message| in that case.
Assignee: general → mrbkap
Status: NEW → ASSIGNED
Attachment #437939 - Flags: review?
(Assignee)

Updated

7 years ago
Attachment #437939 - Flags: review? → review?(jorendorff)
(Reporter)

Updated

7 years ago
Attachment #437939 - Flags: review?(jorendorff) → review+
(Reporter)

Comment 3

7 years ago
Comment on attachment 437939 [details] [diff] [review]
Proposed fix

Thanks.
Blocks: 558182
http://hg.mozilla.org/mozilla-central/rev/34eb552d42f5
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Comment on attachment 437939 [details] [diff] [review]
Proposed fix

I'd really like to get this on 1.9.2. It fixes a hole in the error reporting mechanism that has been around for a while and breaks simple uses of 'throw new Error()' in script.
Attachment #437939 - Flags: approval1.9.2.4?
Comment on attachment 437939 [details] [diff] [review]
Proposed fix

a=beltzner for 1.9.2.4
Attachment #437939 - Flags: approval1.9.2.4? → approval1.9.2.4+
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/fba0e7a3c42f
status1.9.2: --- → .4-fixed
Comment on attachment 437939 [details] [diff] [review]
Proposed fix

Do we want this on 1.9.1? Simple patch, no real risk, fixes the error report mechanism when doing a |throw new Error();| which is probably pretty common...
Attachment #437939 - Flags: approval1.9.1.10?
Comment on attachment 437939 [details] [diff] [review]
Proposed fix

a=beltzner for 1.9.1.10
Attachment #437939 - Flags: approval1.9.1.10? → approval1.9.1.10+

Comment 10

7 years ago
Friendly notice: both the 1.9.1.10 and 1.9.2.4 code freezes are scheduled for *tomorrow*, Tuesday April 27th 2010 @ 11:59 pm PST.

Updated

7 years ago
Assignee: mrbkap → bent.mozilla
Blake's patch doesn't apply cleanly to 1.9.1, he's going to take a peek and make sure it's a safe merge.
Assignee: bent.mozilla → mrbkap

Comment 12

7 years ago
Ah, thanks Ben. Sorry for assigning this to you prematurely.
(Assignee)

Comment 13

7 years ago
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/c3cbe5039818
status1.9.1: --- → .10-fixed
Duplicate of this bug: 512157
You need to log in before you can comment on or make changes to this bug.