Closed Bug 1253859 Opened 5 years ago Closed 5 years ago

Throwed symbols aren't handled by onerror handler and console

Categories

(DevTools :: Console, defect)

44 Branch
defect
Not set
normal

Tracking

(firefox48 fixed)

RESOLVED FIXED
Firefox 48
Tracking Status
firefox48 --- fixed

People

(Reporter: michalwadas, Assigned: arai)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:44.0) Gecko/20100101 Firefox/44.0
Build ID: 20160209234642

Steps to reproduce:

I entered following code into console:

foo = Symbol('foo');
window.addEventListener('error', (e)=>console.log(typeof e));
throw foo;



Actual results:

Nothing visible.


Expected results:

Uncaught exception should log twice.
Confirmed the issue on nightly.
Browser console shows following error message with the STR:

06:08:48.324 error occurred while processing 'evaluateJSAsync: TypeError: can't convert symbol to string
Stack: WCA_onEvaluateJS@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/webconsole.js:891:1
WCA_onEvaluateJSAsync@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/webconsole.js:842:20
DSC_onPacket@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/main.js:1643:15
LocalDebuggerTransport.prototype.send/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/transport/transport.js:569:11
exports.makeInfallible/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/ThreadSafeDevToolsUtils.js:101:14
exports.makeInfallible/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/ThreadSafeDevToolsUtils.js:101:14
Line: 891, column: 1
DSC__unknownError() main.js:1512
DSC_onPacket() main.js:1645
LocalDebuggerTransport.prototype.send/<() transport.js:569
exports.makeInfallible/<() ThreadSafeDevToolsUtils.js:101
exports.makeInfallible/<() ThreadSafeDevToolsUtils.js:101
1 main.js:1512:0


So, following line:
http://hg.mozilla.org/mozilla-central/file/5a2e0878d6c2/devtools/server/actors/webconsole.js#l889
>         let unsafeDereference = error && (typeof error === "object") &&
>                                 error.unsafeDereference();
>         errorMessage = unsafeDereference && unsafeDereference.toString
>           ? unsafeDereference.toString()
>           : "" + error;

it should check |typeof error === "symbol"| as well.
Status: UNCONFIRMED → NEW
Component: Untriaged → Developer Tools: Console
Ever confirmed: true
sorry, overlooked the "log twice" in the comment.
I think that error thrown from web console cannot be caught by window's onerror, regardless of the type of the thrown value, at least for now.
fitzgen, is an error thrown from web console supposed to be catch-able by window's onerror?
Flags: needinfo?(nfitzgerald)
You are right, errors directly thrown from console aren't handled by onerror handler. Anyway, `onerror` have problem with handling Symbols too:

foo = Symbol();
window.addEventListener('error', (e)=>console.log(e));
setTimeout(()=>{throw foo});

By the way, console shows two errors in this case.

TypeError: can't convert symbol to string
uncaught exception: unknown (can't convert to string)
"TypeError: can't convert symbol to string" part seems to be fixed in following range:
  https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=269290441727275e48387abe813e473a25dd4d87&tochange=b1796696599506c18f4e8afb2e03aa841ab8d749

"uncaught exception: unknown (can't convert to string)" is still reproducible on nightly.
"uncaught exception: unknown (can't convert to string)" is reproducible even in js shell, by just "throw Symbol.iterator" one liner.
it would be better filing separated bug for it, as at least it's totally different issue than comment #1's one.
for now, fixed the conversion error in devtools console.

|typeof error === "symbol"| case is handled in String function, and other cases are same as before (that calls ToString internally).
Assignee: nobody → arai.unmht
Attachment #8727456 - Flags: review?(nfitzgerald)
See Also: → 1254174
(In reply to Tooru Fujisawa [:arai] from comment #3)
> fitzgen, is an error thrown from web console supposed to be catch-able by
> window's onerror?

In general, usage of the devtools shouldn't be observable to content scripts unless the user does something "magic" with devtools that would otherwise be impossible (eg using the debugger to force return `false` from a function that always returns `true`).

One could argue that by explicitly running JS in the page, all bets are off. Indeed, all bets are definitely off if you call some function already defined by content, as it could inspect Error().stack and figure out whether the call would be "naturally" occurring or not. However, if the devtools user only ran JS that defined within the console and didn't pollute the global namespace, then it seems the content JS would have no way to determine for sure whether devtools are in use.

On the other hand, I would expect JS evaluated in the console to follow the JS and HTML specs and since the HTML spec says the "error" event should be fired[0], then I would expect it to be fired!

[0] http://www.w3.org/TR/html5/webappapis.html#runtime-script-errors

This is a long winded way to say "I'm not sure". I'm slightly leaning towards "it should be fired".

FWIW, chrome does NOT fire the "error" event for errors in the console.
Flags: needinfo?(nfitzgerald)
Comment on attachment 8727456 [details] [diff] [review]
Properly stringify thrown symbol in console.

Review of attachment 8727456 [details] [diff] [review]:
-----------------------------------------------------------------

Great! Thank you, :arai!

::: devtools/shared/webconsole/test/test_throw.html
@@ +63,5 @@
>    });
>  
> +  let symbolTestValues = [
> +    ["Symbol.iterator", "Symbol(Symbol.iterator)"],
> +    ["Symbol('foo')", "Symbol(foo)"],

For completeness, let's add "Symbol()" as well.
Attachment #8727456 - Flags: review?(nfitzgerald) → review+
Blocks: 1254255
Thank you for reviewing :)

Filed bug 1254255 for "error" event, as it's not related to symbol itself, and it will need some more discussion before fixing or closing.


To be clear, here is a summary for all issues reported in this bug:

  1. directly throwing symbol from console show nothing (comment #0)
     => fixed by attachment 8727456 [details] [diff] [review]

  2. window.onerror is not fired for an exception directly thrown from console (comment #0)
     => will be handled in bug 1254255

  3. throwing symbol from setTimeout in console shows "TypeError: can't convert symbol to string" (comment #4)
     => already fixed by one of https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=269290441727275e48387abe813e473a25dd4d87&tochange=b1796696599506c18f4e8afb2e03aa841ab8d749

  4. uncaught symbol is not converted to descriptive string in error message (comment #4)
     => fixed by bug 1254174
https://hg.mozilla.org/mozilla-central/rev/5d2ded37af5e
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
Reproduced this bug on firefox nightly 47.0a1 according to (2016-03-05)

It's verified on Latest Developer Edition 
Build ID   - 20160528004028
User Agent - Mozilla/5.0 (Windows NT 10.0; rv:48.0) Gecko/20100101 Firefox/48.0

Tested OS-- Windows 10(32bit)
QA Whiteboard: [bugday-20160525]
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.