Promise-returning operations should return Promise.reject on invalid argument

[danyao]

If an interface defines a Promise-returning operation with required arguments, the generated wrapper throws TypeError if insufficient number of arguments are provided. I believe the expected behavior is to return a Promise.reject that throws.

E.g.

interface Foo {
  Promise<string> greet(string name);
};

Generates:

Foo.prototype.greet = function greet(name) {
  ...
  if (arguments.length < 1) {
    throw new TypeError(
      "Failed to execute 'greet' on 'Foo': " + "1 argument required, but only " + arguments.length + " present."
    );  // [*]
  }

  ...
}

[*] should be:

return Promise.reject(new TypeError("<error message>"));

I couldn't find exact wording in WebIDL spec about this. However, the return-rejected-promise behavior is what Web Platform Test's idlharness.js enforces.

[domenic]

FYI https://help.github.com/articles/creating-and-highlighting-code-blocks/ for putting code in GitHub. I'll edit your post to do that :)

[danyao]

Thanks! Will do next time.

[TimothyGu]

Web IDL reference: https://heycam.github.io/webidl/#dfn-create-operation-function step 2:

...
And then, if an exception E was thrown:

1. If op has a return type that is a promise type, then return ! Call(%Promise_reject%, %Promise%, «E»).
2. Otherwise, end these steps and allow the exception to propagate.

Likewise we should fix this for attribute getters and setters, which has a similar clause.

[domenic]

This is blocking the Streams Standard reference implementation from migrating to webidl2js. If anyone is excited about taking this I would very much appreciate it. Otherwise I'll try to find time soon!

[Sebmaster]

We could probably implement this rather easily by just marking the functions async if they have a Promise return type nowadays since our node requirements are high enough, yeah?

[domenic]

True, I think that would probably do it!

[ExE-Boss]

The thing is, WebIDL creates a rejected Promise synchronously in that case, whereas async functions only begin executing as a macrotask.

This is also why #172 and #194 use try‑catch instead of async function, because there the user callback would otherwise begin executing asynchronously, which can potentially be detected.