Bug 218284

Summary: Error event not captured when error originates in module
Product: WebKit Reporter: Andreas <andreas>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Severity: Normal CC: andreas, asumu, bburg, cdumez, esprehn+autocc, ews-watchlist, fpizlo, hi, kangil.han, keith_miller, mkwst, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: Safari 14   
Hardware: Mac   
OS: All   
See Also: https://bugs.webkit.org/show_bug.cgi?id=243542
Description Flags
Not all errors (in module) are captured by the listener
ews-feeder: commit-queue-
ews-feeder: commit-queue-
Safari only captures 3 of the 4 error-events (vs FF that correctly captures al 4) none

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
Comment 2 Asumu Takikawa 2022-01-28 15:48:53 PST
Created attachment 450283 [details]
Comment 3 Asumu Takikawa 2022-01-28 17:35:33 PST
Created attachment 450295 [details]
Comment 4 Asumu Takikawa 2022-01-28 22:49:33 PST
Created attachment 450317 [details]
Comment 5 Asumu Takikawa 2022-01-29 10:02:29 PST
Created attachment 450326 [details]
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]

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