WebAudio / web-audio-api

The Web Audio API v1.0, developed by the W3C Audio WG
https://webaudio.github.io/web-audio-api/
Other
1.04k stars 165 forks source link

Should AudioWorkletGlobalScope.registerProcessor() really throw for invalid parameter descriptor values? #2534

Closed cdumez closed 11 months ago

cdumez commented 1 year ago

Describe the issue

Should AudioWorkletGlobalScope.registerProcessor() really throw for invalid parameter descriptor values? According to the specification, it should:

WebKit currently implements this. However, it seems Chrome doesn't (I haven't verified but I have been told Firefox doesn't either). Looking at the Blink code I see the following comment:

// TODO(crbug.com/1078546): The steps 7.3.3 ~ 7.3.6 are missing.

The inconsistent behavior between WebKit and other engines led to the following bug report against WebKit:

The bug reporter also found it confusing that Worklet.addModule() didn't throw even though the worklet script threw an exception but this is the behavior expected by the tests:

See https://wpt.live/worklets/audio-worklet-import.https.html Subtest: Importing a script which throws should still resolve the given promise.

Not sure what to do here? Should I align WebKit with other browser engines? In which case, we should probably update the specification to match the behavior of browsers. Or should we try to get some buy-in for other browser engines so they align with the specification?

cdumez commented 1 year ago

cc @hoch

chrisguttandin commented 1 year ago

I think Firefox implements those checks, too. I made a little example which fails as expected in Firefox and Safari.

https://stackblitz.com/edit/js-l8bprb?file=index.js

Chrome is the only browser which doesn't throw an error. But the AudioParam somehow ends up in an impossible state. My personal opinion is that what Chrome is currently doing is far more confusing as the error thrown by Firefox and Safari.

It would be great if the error could be thrown already when calling addModule() but I guess that behavior is inherited from here: https://html.spec.whatwg.org/multipage/worklets.html#dom-worklet-addmodule. However in step 4.2. the algorithm mentions something called error to rethrow but I couldn't find any description of how that could be set to true. Maybe doing that would be an option.

cdumez commented 1 year ago

Oh, if Firefox implements these checks then it'd be good to get Blink on board too so we have interoperability.

For https://html.spec.whatwg.org/multipage/worklets.html#dom-worklet-addmodule, You mention error to rethrow at step 6.4.2. However, I think this is too early, the actual module execution only happens at step 6.4.4.

hoch commented 1 year ago

Thanks for bringing to my attention. I created a bug to track this: http://crbug.com/1409936

https://webaudio.github.io/web-audio-api/#AudioWorkletGlobalScope-methods

The step 7.6 handles this specific problem and Chrome is missing the check. Re: the timing of this check - I agree with @cdumez that the script execution is later in the process.

Perhaps adding AudioWorklet.onerror might be a better way to solve this so it can catch the exception from AWGS itself. We also have an open issue about adding onmessage and postMessage to AudioWorklet/AWGS anyway.

guest271314 commented 1 year ago

For https://html.spec.whatwg.org/multipage/worklets.html#dom-worklet-addmodule, You mention error to rethrow at step 6.4.2. However, I think this is too early, the actual module execution only happens at step 6.4.4.

You can use this https://gist.github.com/lukaslihotzki/b50ccb61ff3a44b48fc4d5ed7e54303f to demonstrate where and how you think addModule() should throw, per current specification or otherwise.

hoch commented 11 months ago

Closing this issue because we open a bug agains Chrome.