webidl icon indicating copy to clipboard operation
webidl copied to clipboard

Normative: Add DOMException cause

Open legendecas opened this issue 2 years ago • 41 comments

Add cause support to DOMException.

Fixes https://github.com/whatwg/webidl/issues/969

  • [x] At least two implementers are interested (and none opposed):
    • Node.js
    • Gecko
    • WebKit
  • [ ] Tests are written and can be reviewed and commented upon at:
    • https://github.com/web-platform-tests/wpt/pull/40881
  • [ ] Implementation bugs are filed:
    • Chrome: …
    • Firefox: …
    • Safari: …
    • Deno: …
    • Node.js: …
    • webidl2.js: …
    • widlparser: …

(See WHATWG Working Mode: Changes for more details.)


Preview | Diff

legendecas avatar Aug 13 '22 03:08 legendecas

@annevk Thank you for the suggestions! Updated.

legendecas avatar Sep 01 '22 09:09 legendecas

This looks fine to me, but I'd also like it to be consistent with the other DOMException properties (with a getter). I also think the serialization should include this value.

petervanderbeken avatar Sep 05 '22 09:09 petervanderbeken

+1 to make it an accessor property to be consistent. It's great to have an IDL attribute just like 'name', 'message', and 'code'.

yuki3 avatar Sep 06 '22 06:09 yuki3

As far as implementor support is concerned, I will implement support in Cloudflare Workers also.

jasnell avatar Sep 06 '22 21:09 jasnell

As people seem to be more comfortable with being consistent with IDL attributes on the cause, I've updated the text to define the cause as an IDL attribute on DOMException.

Also, I've updated the steps to include the cause in serialization/deserialization.

legendecas avatar Sep 07 '22 09:09 legendecas

I'm confused - 'cause' in error is very explicitly designed to determine if an error has a cause or not.

Making it an accessor on the prototype breaks this highly intentional design decision in ecma262. It's meant to be an own property that is completely absent by default.

ljharb avatar Sep 07 '22 15:09 ljharb

I was in favor of the accessor for consistency with message (and stack in SpiderMonkey) + the unusualness of Web IDL constructs defining own data properties, but @ljharb’s explanation changed my mind. More reliable API contracts for “whatever value was just thrown at me” is worth a lot more. DOMException is already a lil freaky anyway.

Also tho I sincerely hope nobody ends up needing to spot the distinction between "cause" in new Error("", { cause: undefined }) and "cause" in new Error, if they did need to and couldn’t, that’d be worse. Given cause would typically be a value the cause-bearing-error thrower is just passing along from possibly unrelated code, this might be your only signal that something more spooky’s going down.

bathos avatar Sep 07 '22 16:09 bathos

Yeah, I think that's a decision we just won't support on the web platform. It seems fine to not support undefined causes as distinct from absent causes. I gave this feedback back when the proposal was in TC39 and it was rejected. But for IDL and the web platform generally we'll use the web-platform-standard idioms of accessors on the prototype and undefined by default.

domenic avatar Sep 08 '22 01:09 domenic

Google reps were perfectly able to prevent advancing the language proposal - it seems pretty unfortunate to just callously choose to ignore the result of a standards process just because you don’t like the result.

ljharb avatar Sep 08 '22 01:09 ljharb

We're not ignoring the result of the standards process. The standards process you're discussing was about Error. We have our own standards process on the web platform for DOMException.

domenic avatar Sep 08 '22 02:09 domenic

… which inherits from Error, and thus creates user confusion when it deviates from it.

ljharb avatar Sep 08 '22 02:09 ljharb

We discussed this today on the WinterCG call. Most of the attendees did not have strong opinions one way or the other with regard to cause as an accessor or cause as a data property. However, those that did have strong opinions were in favor of matching the behavior specified by TC-39 for Error objects.

Specifically from the Cloudflare Workers point of view, because we are aligned with WebIDL, it is easier for us to implement cause using an accessor, and in our typical use case we would treat a missing cause and a cause that is always undefined as being semantically equivalent, however there is a valid argument to make that a cause that is explicitly undefined vs. a cause that just was never provided are very different things and should be treated as such. Unless there's a strong technical argument to make in favor of the accessor option beyond "that's what we do for name and message so let's keep it consistent", I'd prefer that cause on DOMException was defined as an own data property.

jasnell avatar Sep 08 '22 18:09 jasnell

In the WinterCG meeting it was also mentioned that making cause a property in the prototype might go against the priority of constituencies, since the cause property in Error instances can be tested for using "cause" in error, and authors would expect that to also work for DOMException. This would override any implementability or spec concerns.

andreubotella avatar Sep 08 '22 18:09 andreubotella

Thanks for the feedback! Those sound like reasons server-side runtimes might want to deviate from what we do in browsers, but in browsers we'll be following web platform patterns and using a prototype accessor.

domenic avatar Sep 08 '22 22:09 domenic

For workers, we'll implement cause as a data property, and I will be encouraging Node.js to do the same. If users ask why browsers work differently, I'll refer them to this thread.

Do browsers intend to diverge from the language spec for cause in any other way we should be concerned about?

jasnell avatar Sep 08 '22 22:09 jasnell

The priority of constituencies should be why browsers do a data property as well - "webIDL" is irrelevant compared to user expectations, as I understand it, and it's not like the web has no other own data properties, even on Errors (stack).

ljharb avatar Sep 08 '22 22:09 ljharb

Browsers intend to follow the Web IDL spec, which will specify cause as an accessor property like the other 10,000+ web platform object properties. This is not a deviation from Ecma-262 because Ecma-262 specifies Error, not DOMException.

domenic avatar Sep 08 '22 22:09 domenic

stack is a web platform object data property, it's just not on a web platform object.

Web IDL isn't for web developers, it's for web spec authors. Why is following Web IDL more important than following the pattern that the core JS error objects follow?

ljharb avatar Sep 08 '22 22:09 ljharb

Following the patterns of the web platform is more important for web platform objects, than following the pattern of (Chrome and Safari's implementation of) stack.

domenic avatar Sep 08 '22 22:09 domenic

Ok, so you expect web users to understand the difference between language objects and web platform objects? That’s great, i was under the impression that browsers’ position was that there wasn’t a distinction in the majority of user’s minds.

ljharb avatar Sep 08 '22 23:09 ljharb

No, that is not what I said.

domenic avatar Sep 09 '22 00:09 domenic

I’m confused then - it seems like either users understand the difference between web and language, and the consistency of web platform objects matters to them more than consistency with the language - or, it’s all just JavaScript, and consistency solely within the web platform is irrelevant compared to consistency with the union of the web and the language (and non-web environments, as well), since it’s all one big bucket.

If I’m making a false dichotomy I’d love to understand more about how.

ljharb avatar Sep 09 '22 00:09 ljharb

Just curious, do we have a clue that "cause" in err is already a well established pattern in the field?

saschanaz avatar Sep 09 '22 20:09 saschanaz

@saschanaz nope, i think cause is new enough that there are no established patterns - but the only other alternatives are Object.hasOwn(err, 'cause') - which wouldn't work with this accessor approach either - or the incomplete approach of typeof err.cause !== 'undefined', which fails to distinguish between a causeless error and an undefined cause error.

ljharb avatar Sep 09 '22 20:09 ljharb

Thanks for the quick answer! That was my impression too.

which fails to distinguish between a causeless error and an undefined cause error.

Sounds like it, but does it / should it practically matter? I think Web IDL generally doesn't try hard to distinguish between explicit undefined and nonexistence (e.g. dictionaries treat explicit undefined as nonexistent), and I believe nobody complained with it.

saschanaz avatar Sep 09 '22 21:09 saschanaz

I think Web IDL generally doesn't try hard to distinguish between explicit undefined and nonexistence (e.g. dictionaries treat explicit undefined as nonexistent), and I believe nobody complained with it.

Whether property absence & property access evaluating to undefined are semantically equivalent or distinct is a characteristic of an API contract, not of properties/objects inherently, and I’d figure the decision would typically be determined by the value space that needs to be expressed. In the Error cause contract (and a few other places in ECMA-262), property absence is used to express a +1 language value space: “all ES values or none”. It’s the least painful mechanism the language has for expressing this empty notion: a “third null” (cursed lol — not-a-value NaV?) couldn’t do the same because when reified as a (public) ES value, it’s “an ES value” and you’re back at square one. The next best thing is pretty gross: a second property (causeIsEmpty: true) for the extra information bit.

Given Web IDL dictionaries don’t need to express this meta value space, it makes sense that they make both cases equivalent and that no one would complain about it; it’s not preventing them from expressing any Web IDL values/not-a-values. I think it seems not far off from "0" / 0 / -0 being semantically equivalent when the dictionary member type is unsigned long: smaller value space = more conversion & conflation desirable? Or at least more is possible, I should say.

The contract in this case though is that of a non-Web IDL API from a user POV, the API of the (hypothetical, but seemingly intended) “super class” (and its other subclasses, notably?). I’m not sure the question of whether the semantic distinction should have been made ought to determine what’s done here. Perhaps the empty/undefined distinction might never help anybody debug anything, but I’m pretty sure cause-always-present will create bugs itself. People will end up using this check regardless and writing tests for whether some catch logic behaves right for each native/platform Error subclass would be unusual.

While I don’t have a strong opinion on whether the distinction was worth making, I haven’t understood why it’d be so off-the-table to — in intuitive, not literal terms — “just pass it to super()”. Is it a pain for implementors?


Aside re: “Web IDL generally doesn't try hard to distinguish between explicit undefined and nonexistence,” I’ve often wished it would try even less hard :)

Chromium console showing history.pushState(1) throwing due to arity — the undefined/absence distinction — while history.pushState(1, undefined) succeeds

bathos avatar Sep 10 '22 00:09 bathos

I’ve often wished it would try even less hard :)

Somewhat off-topic, but please file an issue on that against whatwg/html? Seems worth fixing one way or another.

annevk avatar Sep 10 '22 16:09 annevk

Possibly notable:

console.log("cause" in new WebAssembly.CompileError("xxx", {}));
console.log("cause" in new WebAssembly.CompileError("xxx", { cause: 1 }));

already prints false and true in both Chromium & FF.

bathos avatar Sep 19 '22 05:09 bathos

https://webassembly.github.io/spec/js-api/#error-objects

Note: It is not currently possible to define this behavior using Web IDL.

(WebAssembly being partially defined in Web IDL, but not actually being implemented in terms of Web IDL apparently has a couple of other twists to it.)

annevk avatar Sep 19 '22 15:09 annevk

@bathos

“just pass it to super()”. Is it a pain for implementors?

Conceptually, no, unfortunately the reality is a bit different. For those of us who implement DOMException in C++ using v8's APIs -- which do not provide any mechanism at the C++ level to invoke the Error constructor easily -- we can't simply "pass it to super()"... but that's an implementation problem.

The key issue here for me is that DOMException taking the getter/setter route when all other standardized Error types use the own property route means that users will be forced to either special case DOMException in error handling logic or otherwise be forced to always handle things the way DOMException does, eroding the language level guarantees.

For example,

try {
  doSomething();
} catch (err) {
  // It's now completely ambiguous what 'cause' in err means.

  // I either have to...

  if (err instanceof DOMException) {
    // 'cause' in err is meaningless.
  } else {
    // 'cause' in err has semantic value
  }

  // or always assume 'cause' in error is meaningless.
}

This difference does not just apply to non-browser runtimes. Browser developers will face this same issue. As the other runtimes also implement this, the fact that DOMException does it differently means that developers in general will not be able to rely on it.

Now, that said it's absolutely true that JavaScript allows you to throw literally anything so it can be argued that the mechanism is already unreliable, but since DOMException extends from Error, it really ought to follow the language specified contract for Error or it otherwise should not claim to be an Error.

const ex = new DOMException();
ex instanceof Error;  // true

jasnell avatar Sep 22 '22 22:09 jasnell