1448431 - Don't pause on console expression exceptions

Status: RESOLVED FIXED

(DevTools :: Debugger, enhancement)

Description

[Jason Laster [:jlast]]

Today, we allow the debugger to pause on watch & console expression exceptions. The way this works, is a) the user sets "pause on exceptions", b) the user types something that throws e.g. `0()`.

When the debugger pauses it
1) shows the expression's source
2) re-evaluates other watch expresssions

If a watch expression throws while the user is stepping, it creates a weird experience where the user is shown the expression source.

It would be simpler if we prevent the debugger from pausing in an expression's exception. If we do, the user will be shown the expressions error in the console or watch panel, but it won't affect the current debugging experience.

Comment 1

[Jason Laster [:jlast]]

Attachment: Bug 1448431 - Don't pause on console expression exceptions.

Review commit: https://reviewboard.mozilla.org/r/230718/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/230718/

Comment 2

[Jim Blandy :jimb]

I agree that pause-on-exception should not cover console evaluation or watch expressions.

Comment 3

[Jim Blandy :jimb]

Comment on attachment 8961891 [details]
Bug 1448431 - Don't pause on console expression exceptions.

https://reviewboard.mozilla.org/r/230718/#review237306

Just asked for one comment.

::: devtools/server/actors/thread.js:1539
(Diff revision 1)
>      const generatedLocation = this.sources.getFrameLocation(youngestFrame);
>      const { originalSourceActor } = this.unsafeSynchronize(
>        this.sources.getOriginalLocation(generatedLocation));
>      const url = originalSourceActor ? originalSourceActor.url : null;
>  
> -    if (this.sources.isBlackBoxed(url)) {
> +    if (!url || this.sources.isBlackBoxed(url)) {

This needs a comment explaining why we continue here. The name 'isBlackBoxed' pretty clearly suggests why we shouldn't pause, but it's much less clear why the absence of a URL should matter.

Comment 4

[Jason Laster [:jlast]]

Comment on attachment 8961891 [details]
Bug 1448431 - Don't pause on console expression exceptions.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/230718/diff/1-2/

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4b1dc5e311b9
Don't pause on console expression exceptions. r=jimb

Comment 6

[Cristina Coroiu [:ccoroiu]]

Backed out changeset 4b1dc5e311b9 (bug 1448431) for xpcshell failures at devtools/server/tests/unit/test_pause_exceptions-01.js

Failure push: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=4b1dc5e311b92a54befa20bc4af71668fca6048f

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=171630305&repo=mozilla-inbound&lineNumber=2598

Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/d6bc182977b453cf4b1a5d4f94e34744c0c7db75

Comment 7

[Jason Laster [:jlast]]

Comment on attachment 8961891 [details]
Bug 1448431 - Don't pause on console expression exceptions.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/230718/diff/2-3/

Comment 8

[Jason Laster [:jlast]]

Comment on attachment 8961891 [details]
Bug 1448431 - Don't pause on console expression exceptions.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/230718/diff/3-4/

Comment 9

[Code Review Bot [:reviewbot]]

Comment on attachment 8961891 [details]
Bug 1448431 - Don't pause on console expression exceptions.

https://reviewboard.mozilla.org/r/230718/#review239298


Code analysis found 1 defect in this patch:
 - 1 defect found by mozlint

You can run this analysis locally with:
 - `./mach lint path/to/file` (JS/Python)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: devtools/server/tests/unit/test_ignore_caught_exceptions.js:11
(Diff revision 4)
>  
>  /**
>   * Test that setting ignoreCaughtExceptions will cause the debugger to ignore
>   * caught exceptions, but not uncaught ones.
>   */
>  

Error: More than 1 blank line not allowed. [eslint: no-multiple-empty-lines]

Comment 10

[Jason Laster [:jlast]]

Comment on attachment 8961891 [details]
Bug 1448431 - Don't pause on console expression exceptions.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/230718/diff/4-5/

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/19d54765481f
Don't pause on console expression exceptions. r=jimb

Comment 12

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/19d54765481f