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

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P3)

Description

[Karl Tomlinson (back Feb 2 :karlt)]

./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.

Comment 1

[Tom Schuster [:evilpie]]

Attachment: Bug 1623226 - Pass exception stack from AutoJSAPI::ReportException. r?baku

Comment 2

[Tom Schuster [:evilpie]]

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.

Comment 3

[Release mgmt bot [:sylvestre / :calixte / :marco for bugbug]]

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

Comment 4

[Tom Schuster [:evilpie]]

Attachment: Bug 1623226 - Include mutedErrors in SavedFrame

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.

Comment 5

[Tom Schuster [:evilpie]]

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.

Comment 6

[Tom Schuster [:evilpie]]

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 ...

Comment 7

[Tom Schuster [:evilpie]]

Attachment: Bug 1623226 - Use the current pending exception stack to get the file-name/line etc. for uncaught exceptions. r?jwalden

Depends on D67953

Comment 8

[Pulsebot]

Pushed by [e-mail]:
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

Comment 9

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/22549 for changes under testing/web-platform/tests

Comment 10

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.

Comment 11

[Dorel Luca [:dluca]]

Backed out 3 changesets (bug 1623226) for Devtools failures in browser/browser_webconsole_stubs_page_error.js. CLOSED TREE

Log:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=295489424&repo=autoland&lineNumber=7187

Push with failures:
https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&revision=312da02ea6d6aef4ae98390fd2939543c3cd6b64

Backout:
https://hg.mozilla.org/integration/autoland/rev/ce00073cc7e381802f95f1cfa08030ecc66e78a3

Comment 12

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Upstream PR was closed without merging

Comment 13

[Tom Schuster [:evilpie]]

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

Comment 14

[Pulsebot]

Pushed by [e-mail]:
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

Comment 15

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.

Comment 16

[Dorel Luca [:dluca]]

Also caused WPT failures:

Log:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=295493105&repo=autoland&lineNumber=37147

Treeherder:
https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&fromchange=4ba3f4a39170e1625509b553ff60b2bd1bdaf2de&selectedJob=295503598

Tom: Can you please take a look at this failures?

Comment 17

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/41494fa6d116
https://hg.mozilla.org/mozilla-central/rev/a9783d27bb78
https://hg.mozilla.org/mozilla-central/rev/ca98d4d3023d

Comment 18

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot