Closed Bug 1623226 Opened 10 months ago Closed 10 months ago

Report filename, lineno, colno of problematic position in uncaught non-error exceptions

Categories

(Core :: JavaScript Engine, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla76
Tracking Status
firefox76 --- fixed

People

(Reporter: karlt, Assigned: evilpie)

References

(Blocks 2 open bugs)

Details

Attachments

(3 files)

./mach wpt testing/web-platform/tests/html/webappapis/scripting/processing-model-2/window-onerror-runtime-error-throw.html

Actual:
JavaScript error: , line 0: uncaught exception: bar

Expected:
JavaScript error: http://web-platform.test:8000/html/webappapis/scripting/processing-model-2/window-onerror-runtime-error-throw.html, line 32: uncaught exception: bar

"When the user agent is to report an exception E, the user agent must report the error for the relevant script, with the problematic position (line number and column number) in the resource containing the script, using the global object specified by the script's settings object as the target."

When tests for filename and lineno are added to window-onerror-runtime-error-throw.html

assert_equals: expected "http://web-platform.test:8000/html/webappapis/scripting/processing-model-2/window-onerror-runtime-error-throw.html" but got ""
assert_equals: expected 36 but got 0

Those additional assertions pass in Chrome.

Blocks: dom-error
See Also: → 1623230
Priority: -- → P3
Assignee: nobody → evilpies
Blocks: 1620583

I think this already works quite well and produces the expected error message from comment 0. I think the biggest question is how to initialize isMuted. I am not sure if we need to put that into SavedFrame somehow, or if there is some other way to get that information.

Component: DOM: Core & HTML → JavaScript Engine

The component has been changed since the backlog priority was decided, so we're resetting it.
For more information, please visit auto_nag documentation.

Priority: P3 → --

There is definitely stuff that is missing here like structured cloning etc.
Before implementing that I want to find someone who can tell me if this approach
is valid.

Okay, I am not sure who can really give feedback for this. Basically everyone who worked on SavedFrames isn't working on SpiderMonkey anymore. And not sure who besides Boris really knows about muted errors.

After reading through bug 1608027, I think the muted errors code here is less important than I first thought.
We (currently) only try to mute SyntaxErrors, which should always be proper error objects, so this code for uncaught exceptions doesn't apply.
The current code only initializes isMuted to true when NonBuiltinFrameIter finds a frame and as we can see in this bug, this can fail relatively often. The default value for isMuted is false ...

Priority: -- → P3
Attachment #9135002 - Attachment description: Bug 1623226 - WIP: Use the current pending exception stack to get the file-name/line etc. for uncaught exceptions → Bug 1623226 - Pass exception stack from AutoJSAPI::ReportException. r?baku
Attachment #9135274 - Attachment description: Bug 1623226 - WIP: Include mutedErrors in SavedFrame → Bug 1623226 - Include mutedErrors in SavedFrame
Blocks: 1626100
Pushed by evilpies@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/83f0b1c5399c
Include mutedErrors in SavedFrame r=jwalden
https://hg.mozilla.org/integration/autoland/rev/798079ba88f7
Use the current pending exception stack to get the file-name/line etc. for uncaught exceptions. r=jwalden
https://hg.mozilla.org/integration/autoland/rev/312da02ea6d6
Pass exception stack from AutoJSAPI::ReportException. r=baku
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/22549 for changes under testing/web-platform/tests
Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.
Upstream PR was closed without merging

Ah. It seems like the column we get is 2 instead of 1. Maybe we shouldn't be calling FixupColumnForDisplay ?

Flags: needinfo?(evilpies)
Pushed by evilpies@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/41494fa6d116
Include mutedErrors in SavedFrame r=jwalden
https://hg.mozilla.org/integration/autoland/rev/a9783d27bb78
Use the current pending exception stack to get the file-name/line etc. for uncaught exceptions. r=jwalden
https://hg.mozilla.org/integration/autoland/rev/ca98d4d3023d
Pass exception stack from AutoJSAPI::ReportException. r=baku
Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.
Regressions: 1626143
Status: NEW → RESOLVED
Closed: 10 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla76
Flags: needinfo?(evilpies)
Upstream PR merged by moz-wptsync-bot
You need to log in before you can comment on or make changes to this bug.