Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Spec for worker error reporting is broken #1607

Open
bzbarsky opened this issue Jul 28, 2016 · 7 comments
Open

Spec for worker error reporting is broken #1607

bzbarsky opened this issue Jul 28, 2016 · 7 comments
Assignees
Labels
interop Implementations are not interoperable with each other topic: error reporting topic: multiple globals topic: workers

Comments

@bzbarsky
Copy link
Contributor

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
Copy link
Member

domenic commented Aug 11, 2016

I guess it will be easiest to fix this if I also fix #958 at the same time

@foolip
Copy link
Member

foolip commented Feb 27, 2018

Adding the interop label, because in web-platform-tests/wpt#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 foolip added the interop Implementations are not interoperable with each other label Feb 27, 2018
@foolip
Copy link
Member

foolip commented Feb 27, 2018

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

@bzbarsky
Copy link
Contributor Author

@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
Copy link
Member

foolip commented Feb 27, 2018

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

@bzbarsky
Copy link
Contributor Author

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
Copy link
Member

foolip commented Feb 27, 2018

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
interop Implementations are not interoperable with each other topic: error reporting topic: multiple globals topic: workers
Development

No branches or pull requests

3 participants