Closed Bug 1448431 Opened Last year Closed Last year

Don't pause on console expression exceptions

Categories

(DevTools :: Debugger, enhancement)

enhancement
Not set

Tracking

(firefox61 fixed)

RESOLVED FIXED
Firefox 61
Tracking Status
firefox61 --- fixed

People

(Reporter: jlast, Assigned: jlast)

References

Details

Attachments

(1 file)

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.
I agree that pause-on-exception should not cover console evaluation or watch expressions.
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.
Attachment #8961891 - Flags: review?(jimb) → review+
Pushed by jlaster@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4b1dc5e311b9
Don't pause on console expression exceptions. r=jimb
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]
Pushed by jlaster@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/19d54765481f
Don't pause on console expression exceptions. r=jimb
https://hg.mozilla.org/mozilla-central/rev/19d54765481f
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61
Assignee: nobody → jlaster
Duplicate of this bug: 1440472
Product: Firefox → DevTools
Blocks: 1388099
Flags: needinfo?(jlaster)
You need to log in before you can comment on or make changes to this bug.