Last Comment Bug 557346 - The .message of a DOM Worker error event is not populated when the worker does |throw new Error("data");|
: The .message of a DOM Worker error event is not populated when the worker doe...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Other Branch
: All All
: -- normal (vote)
: ---
Assigned To: Blake Kaplan (:mrbkap)
:
:
Mentors:
: 512157 (view as bug list)
Depends on:
Blocks: 558182
  Show dependency treegraph
 
Reported: 2010-04-05 15:13 PDT by Jason Orendorff [:jorendorff]
Modified: 2010-06-27 10:56 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
.4-fixed
.10-fixed


Attachments
Proposed fix (1.03 KB, patch)
2010-04-08 14:00 PDT, Blake Kaplan (:mrbkap)
jorendorff: review+
mbeltzner: approval1.9.2.4+
mbeltzner: approval1.9.1.10+
Details | Diff | Splinter Review

Description Jason Orendorff [:jorendorff] 2010-04-05 15:13:12 PDT
<!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.
Comment 1 Ben Turner (not reading bugmail, use the needinfo flag!) 2010-04-05 15:15:49 PDT
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?
Comment 2 Blake Kaplan (:mrbkap) 2010-04-08 14:00:48 PDT
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.
Comment 3 Jason Orendorff [:jorendorff] 2010-04-08 14:15:01 PDT
Comment on attachment 437939 [details] [diff] [review]
Proposed fix

Thanks.
Comment 4 Ben Turner (not reading bugmail, use the needinfo flag!) 2010-04-08 16:01:42 PDT
http://hg.mozilla.org/mozilla-central/rev/34eb552d42f5
Comment 5 Ben Turner (not reading bugmail, use the needinfo flag!) 2010-04-08 16:03:22 PDT
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.
Comment 6 Mike Beltzner [:beltzner, not reading bugmail] 2010-04-09 13:56:21 PDT
Comment on attachment 437939 [details] [diff] [review]
Proposed fix

a=beltzner for 1.9.2.4
Comment 7 Ben Turner (not reading bugmail, use the needinfo flag!) 2010-04-09 16:53:09 PDT
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/fba0e7a3c42f
Comment 8 Ben Turner (not reading bugmail, use the needinfo flag!) 2010-04-09 17:17:57 PDT
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...
Comment 9 Mike Beltzner [:beltzner, not reading bugmail] 2010-04-23 13:37:25 PDT
Comment on attachment 437939 [details] [diff] [review]
Proposed fix

a=beltzner for 1.9.1.10
Comment 10 christian 2010-04-26 15:36:51 PDT
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.
Comment 11 Ben Turner (not reading bugmail, use the needinfo flag!) 2010-04-27 14:21:42 PDT
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.
Comment 12 christian 2010-04-27 16:30:02 PDT
Ah, thanks Ben. Sorry for assigning this to you prematurely.
Comment 13 Blake Kaplan (:mrbkap) 2010-04-27 16:49:12 PDT
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/c3cbe5039818
Comment 14 Ben Turner (not reading bugmail, use the needinfo flag!) 2010-06-27 10:56:05 PDT
*** Bug 512157 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.