Fire an ErrorEvent instead an Event

[bakulf]

https://html.spec.whatwg.org/multipage/workers.html#worker-processing-model-top

If the algorithm asynchronously completes with null, queue a task to fire an event named error at worker, and abort these steps. Otherwise, continue the rest of these steps after the algorithm's asynchronous completion, with script being the asynchronous completion value.

Why are we firing a normal event and not an ErrorEvent?
With an ErrorEvent we can specify a message and this can be useful.

[zcorpan]

What are the different cases where error would fire here? Just if fetching failed for some reason? Note that we can't expose reason for fetch failure to JS in general for security reasons (but browser devtools is OK I think).

[bakulf]

There are some WPT tests where we use ErrorEvent when, in theory, we should have Error. For instance:

. workers/constructors/Worker/same-origin.html
. workers/constructors/SharedWorker/same-origin.html

Are they wrong? Or am I missing something?

[zcorpan]

Hmm. They look wrong per spec. But if more browsers fire ErrorEvent here then we should probably change the spec...

[bzbarsky]

Just if fetching failed for some reason?

Pretty much, yes. Gecko code has a codepath here for "URL parse failure", but I'm not sure that's really reachable.

So maybe there's nothing useful that can be communicated here...

[zcorpan]

The tests also allow either an exception or an error event, which seems bogus.

[zcorpan]

The spec's first step for Worker constructor says

The user agent may throw a "SecurityError" DOMException and abort these steps if the request violates a policy decision (e.g. if the user agent is configured to not allow the page to start dedicated workers).

but it's unclear to me it is a good idea to have these tests pass if a SecurityError is raised. In Chromium it seems SecurityError is raised for cross-origin URLs, but that is not really per spec (since the URL has not been parsed yet in the spec).

[zcorpan]

git blame points to

web-platform-tests/wpt@98fdc49

https://bugzilla.mozilla.org/show_bug.cgi?id=1218433

[bzbarsky]

Mmm. Yes, looks like the Firefox code there is not following the spec either, in that things like same-origin and "no mixed content" and "is this page even allowed to run workers" are all checked synchronously before the constructor returns, and lead to exceptions on failure.

[hiroshige-g]

As for the async Error event (aside from the sync SecurityError),

• The spec is currently at Step 14 of https://html.spec.whatwg.org/multipage/workers.html#run-a-worker.
  So Event is used for fetch errors and parse errors.
• Test for parse error case: https://wpt.fyi/results/workers/dedicated-worker-parse-error-failure.html?label=experimental&label=master&aligned
• A previous discussion on whether ErrorEvent or Event here is at Add WPTs for parse error handling in SharedWorkers web-platform-tests/wpt#22185.

However, if the error is propagated from the WorkerGlobalScope (exceptions are not handled in WorkerGlobalScope.onerror), ErrorEvent is used (https://html.spec.whatwg.org/multipage/workers.html#runtime-script-errors-2).

The current browser behavior is:

	Chrome	Firefox	Safari
fetch error (MIME type), classic	Event	Event	Event
fetch error (MIME type), module	Event	Event	Event
parse error, classic	ErrorEvent (to be fixed by https://crbug.com/1071345)	ErrorEvent	ErrorEvent
parse error, module	Event (fixed by https://crbug.com/1071345)	N/A	ErrorEvent

[allstarschh]

Hi, @domenic
When a worker's script has an error to rethrow, according to the spec, it should fire an Event named error to the worker (instead of firing an ErrorEvent).
Can you confirm that the spec is correct?

Specifically, in run a worker

14. In both cases, let onComplete given script be the following steps:

    1. If script is null or if script's error to rethrow is non-null, then:

       1. Queue a global task on the DOM manipulation task source given worker's relevant global object to fire an event named error at worker.

       2. Run the environment discarding steps for inside settings.

       3. Abort these steps.

Because from WPT, some test testing with Event failed, whereas some test testing with ErrorEvent is passed among all browsers.

/workers/dedicated-worker-parse-error-failure.html, which is testing Event, is failed among all browsers
https://wpt.fyi/results/workers/dedicated-worker-parse-error-failure.html?label=experimental&label=master&aligned
https://github.com/web-platform-tests/wpt/blob/master/workers/dedicated-worker-parse-error-failure.html
https://github.com/web-platform-tests/wpt/blob/master/workers/support/check-error-arguments.js#L3

There is another test,
/workers/constructors/Worker/AbstractWorker.onerror.html
which is passed in all browsers.
https://wpt.fyi/results/workers/constructors/Worker/AbstractWorker.onerror.html?label=experimental&label=master&aligned
But the test is still testing with ErrorEvent.
https://github.com/web-platform-tests/wpt/blob/master/workers/constructors/Worker/AbstractWorker.onerror.html#L16

I've checked the commit history of AbstractWorker.onerror.html
https://github.com/web-platform-tests/wpt/commits/master/workers/constructors/Worker/AbstractWorker.onerror.html
The latest change for Event can be traced back to Jun, 2015
According to the spec back then

http://web.archive.org/web/20150604031622/https://html.spec.whatwg.org/multipage/workers.html#run-a-worker
6. If the script was not compiled successfully, let the code entry-point be a no-op script, and act as if a corresponding uncaught script error had occurred.

http://web.archive.org/web/20150604031622/https://html.spec.whatwg.org/multipage/workers.html#runtime-script-errors-2

http://web.archive.org/web/20150604100312/https://html.spec.whatwg.org/multipage/webappapis.html#report-the-error
7. Let event be a new trusted ErrorEvent object ...

So the test AbstractWorker.onerror.html looks like it was testing with the older spec, but not the current spec ?

And the commit to check if the script has an error to rethrow, is from
c747ce0

Although the fix was originally to fix SharedWorker, but I assume it also fixed DedicatedWorker?

Thanks

[domenic]

I cannot confirm the spec is correct. As attested by the fact that this issue is still open, there is a long history of discussion about what the correct type of event is to fire here.

[domenic]

After reviewing the exact location at which you inserted the note in #9595, I think the current spec is correct. There's no way an ErrorEvent would make any sense, because the thrown error object only exists inside the WorkerGlobalScope's (unner) realm, not inside the Worker's (outer) realm. So there would be nothing we could populate errorEvent.error with.