search

whatwg / html

HTML Standard
https://html.spec.whatwg.org/multipage/
Other
8.13k stars 2.67k forks source link

Spec for worker error reporting is broken #1607

Open bzbarsky opened 8 years ago

bzbarsky commented 8 years ago

With the recent changes to the script execution model and exception reporting, error reporting in workers is now broken, I believe. For example, consider an event listener running in a worker. We land in https://dom.spec.whatwg.org/#concept-event-listener-inner-invoke and the callback throws an exception. The spec says to "report the exception" which links to https://html.spec.whatwg.org/multipage/webappapis.html#report-the-exception ... but that's the document specific exception handling mechanism. The correct one for workers is at https://html.spec.whatwg.org/multipage/workers.html#runtime-script-errors-2

In particular, per the letter of the spec as written right now, an exception in an event listener in a worker will get reported as an "error" event on the worker's global scope but not on the worker itself or the rest of the processing model described in https://html.spec.whatwg.org/multipage/workers.html#runtime-script-errors-2. This is broken.

Similarly, when running the main worker script https://html.spec.whatwg.org/multipage/workers.html#run-a-worker step 21 calls into https://html.spec.whatwg.org/multipage/webappapis.html#run-a-classic-script which in step 9 substep 3 goes to https://html.spec.whatwg.org/multipage/webappapis.html#report-the-exception which is once again the wrong place to end up.

importScripts has the same issue, because it too lands in https://html.spec.whatwg.org/multipage/webappapis.html#run-a-classic-script

// cc @domenic @annevk

domenic commented 8 years ago

I guess it will be easiest to fix this if I also fix https://github.com/whatwg/html/issues/958 at the same time

foolip commented 6 years ago

Adding the interop label, because in https://github.com/w3c/web-platform-tests/issues/4884 this was the cause of some confusion, as a harness error only happened in Firefox where a error event was fired on window, even though the script error occurred in Chrome as well.

https://wpt.fyi/workers/interfaces/WorkerGlobalScope/onerror/propagate-to-window-onerror.html is an existing test for this.

foolip commented 6 years ago

I've manually run propagate-to-window-onerror.html in Edge 16 and it passes there, so Chrome is the outlier.

bzbarsky commented 6 years ago

@domenic @annevk It would be really good to get this fixed. The spec now matches neither implementations nor web compat nor the actual intent of the spec...

foolip commented 6 years ago

https://bugs.chromium.org/p/chromium/issues/detail?id=590219 is the Chromium bug for this, filed by none other than @bzbarsky.

bzbarsky commented 6 years ago

Note that this issue covers a lot more than that bug. Chrome fires an error event on the worker for example, which the current spec doesn't have happening.

foolip commented 6 years ago

Yeah, I think fixing that bug isn't necessarily blocked on this issue either, we already have a failing test, even if perhaps one can't conclude it should work from the spec at this point.