webidl icon indicating copy to clipboard operation
webidl copied to clipboard

add [[ErrorData]] slot to DOMExceptions

Open ljharb opened this issue 1 year ago • 8 comments

This is an attempt to fix https://github.com/tc39/proposal-is-error/issues/9.

The intention is to make DOM Exceptions true subclasses of Error.

It's possible this PR needs additional changes to simplify/undo #732 - I'm happy to make whatever changes are appropriate.

  • [ ] At least two implementers are interested (and none opposed):
  • [ ] Tests are written and can be reviewed and commented upon at:
  • [ ] Implementation bugs are filed:
    • Chromium: …
    • Gecko: …
    • WebKit: …
    • Deno: …
    • Node.js: …
    • webidl2.js: …
    • widlparser: …
  • [ ] MDN issue is filed: …
  • [x] The top of this comment includes a clear commit message to use.

This is part of a stage 2 TC39 proposal for Error.isError. When it advances to stage 2.7, I'll file the appropriate implementation bugs.


Preview | Diff

ljharb avatar Jul 18 '24 04:07 ljharb

ping @domenic @annevk, this proposal is on the agenda for advancement next plenary, and i'd love to know that this direction is acceptable :-)

ljharb avatar Aug 27 '24 21:08 ljharb

Seems fine directionally. In general we want DOMException to behave as if its constructor had a super() call and I think this aligns with that.

Editorially I wonder if putting it as a step in https://webidl.spec.whatwg.org/#internally-create-a-new-object-implementing-the-interface would be better. (E.g. modifying step 4.)

I also don't understand the relationship to #732 (serializability).

domenic avatar Aug 27 '24 23:08 domenic

How would you suggest I modify step 4?

The relationship to #732, as I understand it (but may indeed understand incorrectly) is that that PR was only necessary because DOMExceptions aren't proper Errors - but by making them proper Errors, then serialization of DOMExceptions should be automatic now?

ljharb avatar Aug 28 '24 00:08 ljharb

How would you suggest I modify step 4?

By adding [[ErrorData]] to the argument list of MakeBasicObject, if the inclusive inherited interfaces of interface includes DOMException.

but by making them proper Errors, then serialization of DOMExceptions should be automatic now?

No, it's not automatic, and it should not be.

The relevant step is step 17 of https://html.spec.whatwg.org/#structuredserializeinternal. Thankfully, it's guarded by "and value is not a platform object", which means DOMException will properly go down to step 19 instead.

If we took the generic step 17 path for DOMException, we would lose the name information, and would look at the public message property instead of the internal message.

domenic avatar Aug 28 '24 00:08 domenic

ah, gotcha. ok cool, i'll make those changes to step 4, thanks!

ljharb avatar Aug 28 '24 01:08 ljharb

hmm - looking at the MakeBasicObject call, that seems like it's for all platform objects, not just DOMExceptions. Would something in https://webidl.spec.whatwg.org/#js-creating-throwing-exceptions make more sense?

If the MakeBasicObject call is correct, then how would I modify that to only add the slot for DOMExceptions and not for other things?

ljharb avatar Aug 28 '24 05:08 ljharb

I'd put the list passed to MakeBasicObject in a variable and then conditionally append [[ErrorData]] to it.

annevk avatar Aug 28 '24 07:08 annevk

Thanks, updated! I left the original note I added as informative, but can remove it if preferred.

ljharb avatar Aug 28 '24 15:08 ljharb

This proposal is now stage 3. What are the next steps to getting this PR merged?

ljharb avatar Dec 09 '24 19:12 ljharb

Implementer interest is the biggest checkbox in the OP that isn't checked. Maybe you can gather that in TC39? Ideally an explicit statement that the people implementing Error.isError() will also make sure the WPTs you've linked pass for DOMException.

domenic avatar Dec 10 '24 02:12 domenic

For Chrome I'll make sure Error.isError returns true for DOMExceptions.

syg avatar Dec 10 '24 16:12 syg

For Firefox, we have https://bugzilla.mozilla.org/show_bug.cgi?id=1936385 filed, and the implementation looks straightforward, so we're also in support.

dminor avatar Dec 11 '24 14:12 dminor

done, if i got the n/a's right :-) thanks!

ljharb avatar Dec 12 '24 05:12 ljharb

For Deno, I have implemented it so that it returns true when V8 implements Error.isError.

petamoriken avatar Dec 13 '24 19:12 petamoriken