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

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

Closed
guest271314 opened this issue Mar 27, 2019 · 9 comments
Assignees

Comments

@guest271314
Copy link
Contributor

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 #1282
#1581

@hoch
Copy link
Member

hoch commented Mar 28, 2019

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.

@hoch hoch self-assigned this Mar 28, 2019
@guest271314

This comment was marked as off-topic.

@hoch
Copy link
Member

hoch commented Mar 28, 2019

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

This comment was marked as off-topic.

@mdjp
Copy link
Member

mdjp commented Sep 17, 2019

@mdjp mdjp closed this as completed Sep 17, 2019
@guest271314

This comment was marked as off-topic.

@karlt
Copy link
Contributor

karlt commented Sep 17, 2019

@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?

@guest271314

This comment was marked as off-topic.

@karlt
Copy link
Contributor

karlt commented Sep 17, 2019

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants