Use of Get in registerProcessor does not seem to handle abrupt completions

[bzbarsky]

https://webaudio.github.io/web-audio-api/#dom-audioworkletglobalscope-registerprocessor has:

Let prototype be the result of Get(O=processorCtor, P="prototype").

which can return an abrupt completion instead of an object, but this code seems to assume that that can't happen.

Similar for:

IsCallable(argument=Get(O=prototype, P="process"))

That said, maybe the idea is that abrupt completions should just be propagated out? That would normally be indicated with explicit ? annotations, I'd think... @domenic is that how it's currently set up to work for situations like this, where IDL steps explicitly invoke ES operations?

Is there test coverage for these cases?

[padenot]

That said, maybe the idea is that abrupt completions should just be propagated out?

I think for now it's supposed to call onprocessorerror, but this needs to be spelled out. @hoch, do you agree?

I think this is tested in https://github.com/web-platform-tests/wpt/blob/master/webaudio/the-audio-api/the-audioworklet-interface/audioworkletnode-onerror.https.html#L34

[hoch]

The problem here is also tied to the lack of proper error handling channel in AudioWorkletGlobalScope: https://github.com/WebAudio/web-audio-api/issues/1984

We can't use AudioWorkletNode.onprocessorerror, because these errors are being thrown in the registration process so it's before the construction of an AudioWorkletNode. Perhaps we should add .onerror to AudioWorklet.

[hoch]

For V1, we don't have a plan to address this issue. The WorkletGlobalScope does not have a proper way of handling exceptions, so the issue should be fixed there not in Web Audio API.

Also registerProcessor() algorithm starts with a sentence that says:

If an exception is thrown in any step, abort the remaining steps.

So the exception will be thrown, but it will not be delivered to the control thread.

As we discussed, processorerror is bound to an AWN instance and registerProcessor() runs on the global scope, not an individual node.

[hoch]

Some ideas from TPAC:

1. AudioWorklet.onerror event handler
2. AudioWorklet.addModule().catch()

[domenic]

Somehow I missed the ping here... It seems discussions have overtaken me, but here's a response to the OP, in case it's helpful.

That said, maybe the idea is that abrupt completions should just be propagated out? That would normally be indicated with explicit ? annotations, I'd think... @domenic is that how it's currently set up to work for situations like this, where IDL steps explicitly invoke ES operations?

Yes, usually with a note like

This section uses the terminology and typographic conventions from the JavaScript specification. [JAVASCRIPT]

e.g. in https://html.spec.whatwg.org/#safe-passing-of-structured-data

[padenot]

https://drafts.css-houdini.org/worklets/#fetch-and-invoke-a-worklet-script, addModule has some error handling.

[padenot]

Virtual F2F:

• We should check if those operations have succeeded, and then follow what the worklets text says to do (reject with the error).

[hoch]

2023 TPAC Audio WG Discussion: There was a confusion in the meeting; checking process in registerProcessor() is not necessary. it’s valid for an AudioWorkletProcessor not to have a process method defined. (see this PR)). The problem in question is how to propagate any error happened in registerProcess() to the main thread.

The proposal is to add onerror handler under AudioWorklet:

partial interface AudioWorklet {
  attribute EventHandler onerror;
}

Since the TPAC discussion was misaligned, the WG will discuss it in the regular teleconference.

[domenic]

Note that AudioWorklet, and thus Worklet, do not inherit from EventTarget right now, so cannot fire events. However, adding such an inheritance link (which would be an edit to the base Worklet spec in HTML) seems very reasonable.

An interesting further question is whether maybe we should support this general error propagation for all worklets. I'd slightly prefer that, but on the other hand I don't want to block the web audio use case where it's most important.

[hoch]

Yes. I believe the lack of proper error propagation in worklets has been a problem. Your second point is aligned with what I am thinking, but I guess the horizontal review (e.g. TAG) will probably trigger a similar suggestion anyway.

[chrisguttandin]

Maybe there is a chance to combine the global error handler with the global MessagePort. The global MessagePort is already in the spec but as far as I can tell it's not implemented anywhere. Maybe it's okay to change it again.

If the AudioWorklet would get a new error event the code to listen to global messages and to listen to global errors would look like this:

audioContext.audioWorklet.port.onmessage = () => { /* */ }; 
audioContext.audioWorklet.onerror = () => { /* */ }; 

To really listen for all errors one would need to add another error handler to the port.

audioContext.audioWorklet.port.onmessageerror = () => { /* */ }; 

What about dispatching all events on the AudioWorklet instead?

audioContext.audioWorklet.onmessage = () => { /* */ }; 
audioContext.audioWorklet.onmessageerror = () => { /* */ }; 
audioContext.audioWorklet.onerror = () => { /* */ }; 

This would also align the API of an AudioWorklet with the one of a regular Web Worker. To make this fully the same the global MessagePort on the AudioWorkletGlobalScope could be replaced with a postMessage() function similar to the one available in a normal Web Worker.

[hoch]

I second @chrisguttandin's idea above.