1230194 - thrown exceptions during console evaluation don't have a stack

Status: RESOLVED FIXED

(DevTools :: Console, defect, P3)

Description

[Brian Grinstead [:bgrins]]

From: https://bugzilla.mozilla.org/show_bug.cgi?id=996060#c6

In console, run `throw new Error("Foo")`

It doesn't provide a stack trace in the browser console, which you can test by just running that code in the browser console. Doing that in the web console also doesn't work.

Comment 1

[Brian Grinstead [:bgrins]]

The console evaluation is using the debugger's executeInGlobalWithBindings which returns a 'throw' property if it threw: https://dxr.mozilla.org/mozilla-central/source/devtools/server/actors/webconsole.js#861.  error.unsafeDereference() does actually have a stack so seems like maybe a bug in createValueGrip.

Comment 2

[Nicolas Chevobbe [:nchevobbe]]

So, with the following expression:

function a() { b(); } 
function b() { c(); } 
function c() { bleah; }
a()

As pointed out by bgrins in Comment 1, evaluating

result.throw.unsafeDereference()

when paused at devtools/server/actors/webconsole/eval-with-debugger.js#138 does give the object with a nice stack:

ReferenceError: "bleah is not defined"
    c debugger eval code:1
    b debugger eval code:1
    a debugger eval code:1

but we don't have a proper stacktrace array property with the frames in the result of executeInGlobalWithBindings.

We could build it in devtools code from result.throw.unsafeDereference().stack, but I'd rather have the fix in executeInGlobalWithBindings

Comment 4

[Brian Hackett [Laid off!]]

Attachment: Bug 1230194 Part 1 - Supply stack trace in completion value when throwing, r=jorendorff.

Comment 5

[Brian Hackett [Laid off!]]

Attachment: Bug 1230194 Part 2 - Show stack for console evaluations that throw something, r=nchevobbe.

Depends on D27826

Comment 6

[Brian Hackett [Laid off!]]

Attachment: Bug 1230194 Part 3 - Add test for thrown values when doing console evaluations, r=nchevobbe.

Depends on D27827

Comment 7

[Brian Hackett [Laid off!]]

With Error objects, the associated SavedFrame objects are only available in C++, AFAICT (Error().stack is a string that has been filtered according to the permissions of the calling code). nsScriptErrorWithStack can get that stack and make it available to JS, but nsScriptErrorWithStack and the associated machinery is not involved when doing a console evaluation in the server.

One fix would be to make the Error's full SavedFrame stack available to the server code via a Debugger API, but this wouldn't help if the thrown value is not an Error object. Instead, these patches make the stack associated with the 'throw' (which was added in bug 996060) available to Debugger users, via the completion value. The server uses this to add a stack to the evaluation result packet, and some plumbing is needed to get this to display in the UI, as evaluation result messages do not currently support showing stacks.

Comment 8

[Brian Hackett [Laid off!]]

Attachment: jit-test patch

Hi, I'm trying to land this bug and had to make a change to the debug/Object-apply-01.js jit-test, per the attached patch. This test also has a binast version which needs to be updated, but when I followed the instructions for doing so in js/src/jsapi-tests/binast/README.md, I got the error below. (I get this error with both the modified and original versions of the test.) What is the best way to proceed here?

./encode.py --topsrcdir=/home/bhackett/mozilla-inbound --binjsdir=/home/bhackett/binjs-ref --binjs_encode=/home/bhackett/binjs-ref/target/debug/binjs_encode lazy/debug/Object-apply-01.binjs nonlazy/debug/Object-apply-01.binjs
encoding /home/bhackett/mozilla-inbound/js/src/jit-test/tests/debug/Object-apply-01.js
to /home/bhackett/mozilla-inbound/js/src/jit-test/tests/binast/lazy/debug/Object-apply-01.binjs
thread 'large stack dedicated thread' panicked at 'Could not enrich AST: ScopeError(MissingBindingKind)', src/libcore/result.rs:997:5
note: Run with RUST_BACKTRACE=1 environment variable to display a backtrace.
thread 'main' panicked at 'Error in dedicated thread: Any', src/libcore/result.rs:997:5
binjs_encode failed
encoding /home/bhackett/mozilla-inbound/js/src/jit-test/tests/debug/Object-apply-01.js
to /home/bhackett/mozilla-inbound/js/src/jit-test/tests/binast/nonlazy/debug/Object-apply-01.binjs

Comment 9

[Tooru Fujisawa [:arai]]

looks like a bug in the encoder side.
feel free to just remove the .binjs file from the tree and commit it.
I will fix it in other bug.

Comment 10

[Tooru Fujisawa [:arai]]

filed https://github.com/binast/binjs-ref/issues/409

Comment 11

[David Teller [:Yoric] - still alive but not very active]

Sounds like https://github.com/binast/binjs-ref/pull/407.

Comment 12

[David Teller [:Yoric] - still alive but not very active]

Ok, the patch is in the process of landing on binast.

Comment 13

[David Teller [:Yoric] - still alive but not very active]

Brian, can you see if the new version of binjs fixes your issue?

Comment 14

[Brian Hackett [Laid off!]]

(In reply to David Teller [:Yoric] (please use "needinfo") from comment #13)

Brian, can you see if the new version of binjs fixes your issue?

Yes, this update worked for me. Thanks!

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/155bf8609915
Part 1 - Supply stack trace in completion value when throwing, r=jorendorff.
https://hg.mozilla.org/integration/mozilla-inbound/rev/624da9352fd9
Part 2 - Show stack for console evaluations that throw something, r=nchevobbe.
https://hg.mozilla.org/integration/mozilla-inbound/rev/e9a1fd460a24
Part 3 - Add test for thrown values when doing console evaluations, r=nchevobbe.

Comment 16

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/84810c2018de
Backed out 3 changesets for devtools failures in browser_dbg_worker-console-02.js on a CLOSED TREE

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b583dd397599
Part 1 - Supply stack trace in completion value when throwing, r=jorendorff.
https://hg.mozilla.org/integration/mozilla-inbound/rev/068483d9fedf
Part 2 - Show stack for console evaluations that throw something, r=nchevobbe.
https://hg.mozilla.org/integration/mozilla-inbound/rev/26b7736f1f87
Part 3 - Add test for thrown values when doing console evaluations, r=nchevobbe.

Comment 18

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/b583dd397599
https://hg.mozilla.org/mozilla-central/rev/068483d9fedf
https://hg.mozilla.org/mozilla-central/rev/26b7736f1f87