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

DOMException(name=<Exception>) or instances of <Exception>? #1817

Closed
lgrahl opened this issue Mar 26, 2018 · 14 comments
Closed

DOMException(name=<Exception>) or instances of <Exception>? #1817

lgrahl opened this issue Mar 26, 2018 · 14 comments

Comments

@lgrahl
Copy link
Contributor

lgrahl commented Mar 26, 2018

There are a bunch of exceptions that should be raised in various circumstances defined by the spec. Are these of...

  1. type DOMException with .name set (to for example TypeError, OperationError, ...), or
  2. type TypeError, OperationError, etc.

Since OperationError doesn't seem a valid standalone exception class and we're using OperationError in the spec, I would assume 1. is correct.
But the WPTs use TypeError all the time and there are plenty of tests that pass with this assumption...

So, what's correct?

Note: Depending on the outcome of this we should file an issue for WPT and update adapter.

@nils-ohlmeier
Copy link

Don't forget that there is also RTCError with errorDetail. Would it be possible to unify these into one type only?

@gsnedders
Copy link

gsnedders commented Mar 26, 2018

WebRTC currently has "created OperationError" where "created" is a citation to WebIDL (first time I looked at this it just sent me to [WebIDL] in the references section and not the definition in WebIDL, weirdly?), where the "created" citation does say that it creates a DOMException.

I'd strongly recommend following what the DOM spec does, with wording like 'throw a "SyntaxError" DOMException', as this makes it unambiguous that a DOMException is being created/thrown.

@lgrahl
Copy link
Contributor Author

lgrahl commented Mar 26, 2018

So, if I understand WebIDL correctly, then everything is a DOMException unless it is one of the following so called simple exceptions that you have to detect by their name: Error, EvalError, RangeError, ReferenceError, TypeError, URIError? 😒

@gsnedders
Copy link

@lgrahl yes.

@aboba
Copy link
Contributor

aboba commented Apr 19, 2018

@lgrahl Is there a spec Issue that needs to be addressed?

@gsnedders
Copy link

Per what I said above,

I'd strongly recommend following what the DOM spec does, with wording like 'throw a "SyntaxError" DOMException', as this makes it unambiguous that a DOMException is being created/thrown.

Only an editorial change, but when that makes things much clearer.

@lgrahl
Copy link
Contributor Author

lgrahl commented Apr 19, 2018

@aboba No spec issue from my side. However, I agree with @gsnedders some wordsmithing could be applied. Would you like to create a PR for this, @gsnedders?

@jan-ivar
Copy link
Member

Seems a consistency issue. I already see this in the spec: "Reject p with a DOMException object whose name attribute has the value OperationError and abort these steps." - just need to use it consistently.

@jan-ivar
Copy link
Member

As to RTCError that is a new error class (type 2 above)

@gsnedders
Copy link

@lgrahl I'm unlikely to get around to it, FWIW.

@lgrahl
Copy link
Contributor Author

lgrahl commented Apr 23, 2018

Okay. FWIW, I can't work on this atm either.

@henbos
Copy link
Contributor

henbos commented Nov 26, 2019

In #2330 we propose using the old name ('OperationError'), but instanceof RTCError would still be true. Can we close this issue?

@henbos
Copy link
Contributor

henbos commented Nov 26, 2019

Or we still need to update WPT tests?

@dontcallmedom
Copy link
Member

the consistency issue seems to have been addressed. I have marked #2375 with the needs test label; closing that issue

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

No branches or pull requests

7 participants