Closed
Bug 1448431
Opened 6 years ago
Closed 6 years ago
Don't pause on console expression exceptions
Categories
(DevTools :: Debugger, enhancement)
DevTools
Debugger
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.
Comment hidden (mozreview-request) |
Comment 2•6 years ago
|
||
I agree that pause-on-exception should not cover console evaluation or watch expressions.
Comment 3•6 years ago
|
||
mozreview-review |
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+
Comment hidden (mozreview-request) |
Pushed by jlaster@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/4b1dc5e311b9 Don't pause on console expression exceptions. r=jimb
Comment 6•6 years ago
|
||
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
Flags: needinfo?(jlaster)
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 9•6 years ago
|
||
mozreview-review |
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 hidden (mozreview-request) |
Comment 11•6 years ago
|
||
Pushed by jlaster@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/19d54765481f Don't pause on console expression exceptions. r=jimb
Comment 12•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/19d54765481f
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox61:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61
Updated•6 years ago
|
Assignee: nobody → jlaster
Updated•6 years ago
|
Product: Firefox → DevTools
Assignee | ||
Updated•5 years ago
|
Flags: needinfo?(jlaster)
You need to log in
before you can comment on or make changes to this bug.
Description
•