Bug 218284 - Error event not captured when error originates in module
Summary: Error event not captured when error originates in module
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: Safari 14
Hardware: Mac All
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-10-28 07:25 PDT by Andreas
Modified: 2022-08-04 08:24 PDT (History)
13 users (show)

See Also:


Attachments
Not all errors (in module) are captured by the listener (1.10 KB, text/html)
2020-10-28 07:25 PDT, Andreas
no flags Details
Patch (62.09 KB, patch)
2022-01-28 15:48 PST, Asumu Takikawa
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (64.61 KB, patch)
2022-01-28 17:35 PST, Asumu Takikawa
no flags Details | Formatted Diff | Diff
Patch (67.43 KB, patch)
2022-01-28 22:49 PST, Asumu Takikawa
no flags Details | Formatted Diff | Diff
Patch (71.87 KB, patch)
2022-01-29 10:02 PST, Asumu Takikawa
no flags Details | Formatted Diff | Diff
Patch (72.10 KB, patch)
2022-01-31 10:06 PST, Asumu Takikawa
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Safari only captures 3 of the 4 error-events (vs FF that correctly captures al 4) (670.59 KB, image/png)
2022-05-14 10:46 PDT, Andreas
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Andreas 2020-10-28 07:25:45 PDT
Created attachment 412527 [details]
Not all errors (in module) are captured by the listener

Some syntax errors are not captured by the event listener when originated in a module.

Syntax errors inside a module in situations like:
	function a() {  // missing closing bracket

won’t get captured by the listener.
While other errors like:
	const a = 1
	a = 2

will get captured, even when generated inside the module.
This works as expected in FF and Chrome.

In the attachment a demo with 4 examples.
Comment 1 Radar WebKit Bug Importer 2020-10-28 10:50:33 PDT
<rdar://problem/70770789>
Comment 2 Asumu Takikawa 2022-01-28 15:48:53 PST
Created attachment 450283 [details]
Patch
Comment 3 Asumu Takikawa 2022-01-28 17:35:33 PST
Created attachment 450295 [details]
Patch
Comment 4 Asumu Takikawa 2022-01-28 22:49:33 PST
Created attachment 450317 [details]
Patch
Comment 5 Asumu Takikawa 2022-01-29 10:02:29 PST
Created attachment 450326 [details]
Patch
Comment 6 Asumu Takikawa 2022-01-29 18:24:13 PST
I think the attached patch will finally be green on CI soon. This patch solves a more general problem than this bug, but it should also ensure the error event gets captured in the original test case too.

The more general problem is that the handling of when errors should get dispatched for module scripts in script elements currently doesn't quite follow the HTML spec. The patches fixes this.

Depending on the error, it gets handled in different ways. If there is a fetch error, the script is null in the spec and gets dispatched to the script element's error handler (https://html.spec.whatwg.org/#execute-the-script-block, step 3).

But if there's a load error, it should not get dispatched. Instead, it won't get thrown until "run a module script" (https://html.spec.whatwg.org/#run-a-module-script) step 7. It doesn't dispatch to the script element, but to the global object.

The patch implements this behavior and changes various tests and test expectations to match this.

It also ensures that when an error is reported from a module script, it is never muted (see https://html.spec.whatwg.org/#report-the-error step 6, the "muted errors" flag is never set for modules) and so the error message gets through to the handler.

This patch should also close https://bugs.webkit.org/show_bug.cgi?id=182177 (because the messages are routed to stderr and expectations changed) and https://bugs.webkit.org/show_bug.cgi?id=158192 (at least for module scripts, I don't know if other scripts need changes for this).

There may need to be more changes made for workers, but I can make a follow-up patch. Another limitation is that the ErrorEvent's "filename" property is blank right now for some errors that get newly reported now.
Comment 7 Yusuke Suzuki 2022-01-30 22:38:34 PST
Comment on attachment 450326 [details]
Patch

Nice! r=me
Comment 8 EWS 2022-01-31 01:29:54 PST
Patch 450326 does not build
Comment 9 Asumu Takikawa 2022-01-31 10:06:25 PST
Created attachment 450415 [details]
Patch
Comment 10 EWS 2022-01-31 11:05:44 PST
Committed r288820 (246594@main): <https://commits.webkit.org/246594@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 450415 [details].
Comment 11 Andreas 2022-05-14 10:46:30 PDT
Created attachment 459357 [details]
Safari only captures 3 of the 4 error-events (vs FF that correctly captures al 4)

The fix still doesn't seem to work in Safari Version 15.4 (15613.1.17.1.13, 15613):
only 3 of the 4 errors are captured by the events in the example file.
 
Attached a screenshot of the console in Safari and FF where the events are correctly captured.