1558786 - Rejected promise in Worker don't produce "uncaught exception" message in the console

Status: RESOLVED FIXED

(Core :: DOM: Workers, defect, P2)

Description

[u591120]

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/75.0.3770.80 Safari/537.36

Steps to reproduce:

Paste the following code in the console or save it to a file and run it:

  let workerScript = `
  self.onmessage = async () => {
    console.log("started!");
    throw new Error("woops");
    self.postMessage("hi!");
  }`;
  let workerScriptUrl = URL.createObjectURL(new Blob([workerScript], {type:"text/plain"}));
  let worker = new Worker(workerScriptUrl, {type:"module"});
  worker.onmessage = (e) => {
    console.log("worker said:", e.data);
  };
  worker.postMessage({});

Actual results:

Notice that there are no errors shown in the console. It simply logs "started!" and that's it. Remove "async" from the self.onmessage hander and notice that the error now shows properly.

Expected results:

The errors that occur in a webworker should obviously be shown in the browser's console, just like the console.logs.

Note that it's not specific to the self.onmessage function. Any async function inside the worker script "swallows" errors.

Tested on two different versions:

• Windows 7, Firefox 65
• Ubuntu 16.04, Firefox 67
  and both give the same behaviour.

Comment 1

[Tom S [:evilpie]]

This seems like a serious issue with more and more people using async functions / promises.
I debugged this a little bit and we do end up in Promise::ReportRejectedPromise and NS_DispatchToMainThread the error event. So we must not be reporting errors on that path.

Comment 2

[Tom S [:evilpie]]

Attachment: Bug 1558786 - Report rejected promises from workers to the right window. r?baku

Comment 3

[Tom S [:evilpie]]

Hey Karl! Baku seems unresponsive, so maybe you can help with this? Promise::ReportRejectedPromise is never invoked when testing with Promise.reject(new Error("foobar")) in worklet_exception.js.

Comment 4

[Karl Tomlinson (:karlt)]

Do you have a testcase for workers?
If so, is the uncaught rejection reported only after the worker thread terminates?

FlushRejections runnables are dispatched to the main thread, where they will not find uncaught rejections from the worker or worklet CycleCollectedJSContext.

PromiseDebugging::FlushUncaughtRejections() is called when a worker thread will terminate.

For worklets, I expect it should be sufficient for FlushRejections::DispatchNeeded() to dispatch to the current thread.
That would be appropriate for workers also.

This looks like a regression from https://hg.mozilla.org/mozilla-central/rev/279446fd94839af9e8f8ae25894f424f0d049909#l1.31

Comment 5

[Tom S [:evilpie]]

(In reply to Karl Tomlinson (:karlt) from comment #4)

Do you have a testcase for workers?

http://tomschuster.name/tests/worker-promise.html

If so, is the uncaught rejection reported only after the worker thread terminates?

I adjusted my testcase to test for this. You are correct, even with my patch we don't report the error until we terminate the worker.
Would your proposal also fix that? I have to admit I actually haven't read most of the promise or worker code.

Comment 6

[Tom S [:evilpie]]

Yay. NS_DispatchToCurrentThread makes the error message immediately appear for workers and worklets. \o/ So now I just have to get some tests going.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ad5d9d96fdfe
Report rejected promises from workers to the right window. r=baku

Comment 8

[Tom S [:evilpie]]

Attachment: Bug 1558786 - Test console message for promise rejection in Worker/Worklet. r?karlt

Comment 9

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/ad5d9d96fdfe

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/50bf3ff60730
Test console message for promise rejection in Worker/Worklet. r=baku

Comment 11

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/50bf3ff60730