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

Audio worklet error handling #2267

Closed
annevk opened this issue Oct 19, 2020 · 7 comments
Closed

Audio worklet error handling #2267

annevk opened this issue Oct 19, 2020 · 7 comments
Labels
Needs Discussion The issue needs more discussion before it can be fixed.

Comments

@annevk
Copy link

annevk commented Oct 19, 2020

Closure of whatwg/html#2611 made me realize that audio worklets have no error handling yet and since they can communicate this might be useful?

The one weirdness here is that this would be the first place where the class does not inherit from EventTarget. (The other solution would be to add that to all worklets, but that would expose the global quite a bit more.)

cc @domenic

@annevk annevk added Needs Discussion The issue needs more discussion before it can be fixed. Untriaged labels Oct 19, 2020
@domenic
Copy link
Contributor

domenic commented Oct 19, 2020

I think they communicate errors out via onprocessorerror.

@padenot
Copy link
Member

padenot commented Oct 29, 2020

Yes, this is how this is set up.

When an error happens in an AudioWorkletGlobalScope, a task is queued to fire an event on the main thread, because we don't want to do this on the real-time thread, in line with what low-latency audio API generally do.

@padenot padenot closed this as completed Oct 29, 2020
@annevk
Copy link
Author

annevk commented Nov 20, 2020

How does this works if the script itself throws? As far as I can tell from https://html.spec.whatwg.org/#dom-worklet-addmodule it invokes https://html.spec.whatwg.org/#run-a-module-script which invokes https://html.spec.whatwg.org/#report-the-exception which invokes https://html.spec.whatwg.org/#report-the-error which terminates at step 8 because the target presumably doesn't implement EventTarget. How would the error end up in onprocessorerror?

@padenot
Copy link
Member

padenot commented Nov 20, 2020

I guess it doesn't. HTML needs a spec hook so that we can get this and do our error handling.

@padenot padenot reopened this Nov 20, 2020
@karlt
Copy link
Contributor

karlt commented Nov 25, 2020

processorerror communicates errors from invocation of an AudioWorkletProcessorConstructor or of the process() method. It is dispatched on an AudioWorkletNode. It is not intended for errors during addModule(), at which point there usually is no AudioWorkletNode.

The concern of an error reporting mechanism for paintWorklet exposing paint implementation specifics doesn't extend to the "Run a module script" script execution, so perhaps a worklet-level solution could still be an option.

Perhaps an exception thrown during addModule() could somehow cause the promise to be rejected, but I don't know if or how this would extend to the first exception in a triggered microtask or to the first unhandled promise rejection.

If there is any non-determinism (e.g. Math.random()), then the same script may run differently when creating another worklet global scope, in which case exceptions could not be covered by any promise.

@annevk
Copy link
Author

annevk commented Nov 30, 2020

I guess if these are explicitly not to be handled, then all might be in order after all and you should feel free to close this. And then if someone wanted to get at those errors at a future point we could consider how to design something for that use case then?

@rtoy
Copy link
Member

rtoy commented Dec 3, 2020

Teleconf: Based on the comments #2267 (comment) and #2267 (comment), we will close the event.

If this is wrong, please let us know and re-open this issue or file a new one.

@rtoy rtoy closed this as completed Dec 3, 2020
@hoch hoch removed the Untriaged label Aug 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Discussion The issue needs more discussion before it can be fixed.
Projects
None yet
Development

No branches or pull requests

6 participants