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.05k stars 167 forks source link

Error reporting for addModule should be consistent with import or at least meaningful #1846

Closed guest271314 closed 5 years ago

guest271314 commented 5 years ago

Describe the issue

It is not clear how errors derived from .addModule() are reported.

If errors do occur the error and message should be meaningful and related to the cause of the error.

Where Is It

https://webaudio.github.io/web-audio-api/#AudioWorklet-Sequence

  1. The promise for the [addModule()](https://drafts.css-houdini.org/worklets/#dom-worklet-addmodule) call is resolved.

The specification does not state what should occur if the Promise is rejected.

Additional Information

Consider a developer/user browses Enter Audio Worklet copy/pastes and tries this code after user action, for example, a click event on an element

HTML

<div class="element">hello</div>

JavaScript

    onload = () => {
      document.querySelector('.element').addEventListener("click", e => {
        const context = new AudioContext();
        /* The code in the main global scope. */
        context.audioWorklet.addModule('processors.js').then(() => {
          let node = new AudioWorkletNode(context, 'port-processor');
          node.port.onmessage = (event) => {
            // Handling data from the processor.
            console.log(event.data);
          };

          node.port.postMessage('Hello!');
        })
        .catch(e => {
          console.error(e);
          console.trace();
        });
      }, {once: true});
    }

and names the file "processor.js" as the code at the linked document does

/* "processor.js" file. */
class PortProcessor extends AudioWorkletProcessor {
  constructor() {
    super();
    this.port.onmessage = (event) => {
      // Handling data from the node.
      console.log(event.data);
    };

    this.port.postMessage('Hi!');
  }

  process(inputs, outputs, parameters) {
    // Do nothing, producing silent output.
    return true;
  }
}

registerProcessor('port-processor', PortProcessor);

where the code at /* The code in the main global scope. */ passes "processor s .js" to .addModule().

The error message reported at Chromium 72 is a DOMException.

**DOMException**
context.audioWorklet.addModule.then.catch.e @ (index):23
Promise.catch (async)
document.querySelector.addEventListener.e @ (index):22

If Worklet is modeled on module script

1.2. Code Idempotency

  • Code is loaded as a module script which resulting in the code being executed in strict mode code without a shared this. This prevents two different module scripts sharing state by referencing shared objects on the global scope.

the error should be consistent with the error that

import x from "./processors"

or

import("./processors")

throws

GET https://path/to/resource.moc/processors.js net::ERR_ABORTED 404

or

(index):8 GET https://path/to/resource.moc/processors.js 404
(anonymous) @ (index):8
TypeError: Failed to fetch dynamically imported module: 
https://path/to/resource.moc/processors.js

Related https://github.com/WebAudio/web-audio-api/issues/1282 https://github.com/WebAudio/web-audio-api/issues/1581

hoch commented 5 years ago

The rejected case of .addModule() is described here: https://drafts.css-houdini.org/worklets/#dom-worklet-addmodule

The Audio Worklet spec only cares about the case where module loading is successfully resolved. Perhaps I can make some clarification on that part, but the error reporting (with meaningful message) is implementation detail. I will follow up at crbug.com.

guest271314 commented 5 years ago

@hoch Will file an issue relevant to https://drafts.css-houdini.org/worklets/#dom-worklet-addmodule if necessary. In the case of trying the code at https://developers.google.com/web/updates/2017/12/audio-worklet (see https://github.com/google/WebFundamentals/issues/7462) the test was less than 50 lines of code, where the error was caused by a single typographical mismatch at the code at the document. If the code were several hundred or thousand lines of code how exactly would the user know that a single character (and what line of the code that character occurred) was the issue?

hoch commented 5 years ago

I completely agree. Thanks for raising the issue - we are aware of the lack of proper error handling/reporting, and I believe that's a fixable problem. But it's something we need to discuss at crbug.com (or bugzilla when FF ships Audio Worklet) because the spec does not mandate what kind of error message needs to be printed out.

guest271314 commented 5 years ago

@hoch https://github.com/w3c/css-houdini-drafts/issues/871

mdjp commented 5 years ago

Duplicate of https://github.com/WebAudio/web-audio-api-v2/issues/29 in V2

guest271314 commented 5 years ago

@mdjp How is this issue a duplicate of the linked issue?

karlt commented 5 years ago

@guest271314 the behavior of addModule() is governed by the worklets spec, and so promise rejection exceptions are specified there. Does https://github.com/w3c/css-houdini-drafts/issues/509 cover the issue you are raising here?

guest271314 commented 5 years ago

@karlt

@guest271314 the behavior of addModule() is governed by the worklets spec, and so promise rejection exceptions are specified there. Does w3c/css-houdini-drafts#509 cover the issue you are raising here?

No, not entirely.

It is not immediately clear if or when the "other features" take control of the Worklet.

The second sentence at the closed issue requesting a video Worklet appears to imply that Worklet is somehow related to CSS? Or, to request for a Worklet to be implemented for "other features" the reques needs to be made at the "other features" specification, though Worklet is still dependent on the controlling Houdini specification.

Web Audio API is not related to CSS whatsoever.

A Worklet might have MessagePort defined at one "other features" not another or be "stateless" or not, e.g., PaintWorklet.

At those "other features" where the a MessagePort is defined the "other features" do have control over whether or not to implement that individual feature, e.g., AudioWorkletNode, indicating that when decided upon by the "other features" said separate implementation can take control over any aspect of the Worklet.

Though evidently "other features" have absolutely no control over error handling and reporting?

At best there are mixed messages depending on which specification an issue is filed at as to the extent to which "other features" (non-CSS) have control over error handling and reporting.

Meaning any "other features" can implement an object defined at the globalThis scope though has no control over error handling or reporting for the object?

Or, is Worklet simply a generic model where any "other features" which opt-in to Worklet are absolutely dependent upon the underlying architecture and specification of Worklet even if an object can be defined at globalThis scope for the given Worklet implementation?

PaintWorklet, AudioWorklet, animation worklet, etc. are all inflexibly beholden to https://drafts.css-houdini.org/worklets/#dom-worklet-addmodule?

If that is the case that would be a unique occurrence from own novice front-end experience with implementations, as implementers deliberately disregard, remove, or add their own "features" to various implementations when they decide to. There are more than one example that can be listed for such occurrences by implementers.

https://github.com/w3c/css-houdini-drafts/issues/857#issuecomment-503195129

@tabatkins What is the appropriate venue to propose a specific type of Worklet be specified?

https://github.com/w3c/css-houdini-drafts/issues/857#issuecomment-503621090

None, because worklets are tools used by other features, not features in themselves. ^_^

But if you want to talk about potential new Houdini features, open an issue in this repo.

Proposal: videoWorklet [worklets-1] #905

Consider the capabilities and functionality of HTML <video> element - without having to load an entire HTML document to access the underlying Media Decoder, Media Encoder, Web Media Player C++ code implemented by browsers (at least Firefox and Chromium) in a Worklet.

https://github.com/w3c/css-houdini-drafts/issues/905#issuecomment-505962411

Ok, so it sounds like you're trying to solve a more general problem, of having the ability to manipulate video streams without needing to instantiate them in a video element.

In that case, it seems like you've already got this well-handled by the existing issues on HTML, and/or the other API proposals you listed. This isn't a Houdini issue; it's not about CSS at all.

(This is why I said that Worklets are not a feature in and of themselves; they're a tool used by other specs that need a specific kind of separate-from-the-main-document JS context.)

karlt commented 5 years ago

The behavior of the addModule() method is defined by the worklets spec. Consistent behavior with import() would be just as useful for other kinds of worklets because the fetching and parsing of scripts is the same.

Other specifications may define how additional methods behave.