Skip to content

Add DOMException #1040

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Add DOMException #1040

wants to merge 3 commits into from

Conversation

bptato
Copy link
Contributor

@bptato bptato commented Apr 30, 2025

Implemented separately from the other errors because it is defined in terms of WebIDL, where members of an interface are getters on their prototype.

See the difference between
JSON.stringify(Object.getOwnPropertyDescriptors(new TypeError())) vs JSON.stringify(Object.getOwnPropertyDescriptors(new DOMException())).

(Required for btoa/atob and structuredClone; ref. #16, #1032)

Implemented separately from the other errors because it is defined in
terms of WebIDL, where members of an interface are getters on their
prototype.

See the difference between
`JSON.stringify(Object.getOwnPropertyDescriptors(new TypeError()))` vs
`JSON.stringify(Object.getOwnPropertyDescriptors(new DOMException()))`.
@@ -285,10 +285,10 @@ function bjson_test_bytecode()
function bjson_test_fuzz()
{
var corpus = [
"FBAAAAAABGA=",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: this was complaining because of the bytecode version, but I don't know how the corpus was generated so I just incremented it manually...

@bptato bptato marked this pull request as draft April 30, 2025 16:49
@bptato
Copy link
Contributor Author

bptato commented Apr 30, 2025

It's still missing a stack trace when created with the constructor...

The standard doesn't specify where to put "stack".
This follows node instead of browsers because it's more straightforward
to implement it this way.
@bptato
Copy link
Contributor Author

bptato commented Apr 30, 2025

I was confused for a minute because browsers set stack as a getter on error prototypes, but we set it as a property on individual Error objects. I ended up following our existing practice.
(stack itself is non-standard; the spec only says that if regular Errors have it, DOMException should also have it.
I don't know if the difference matters anywhere in practice.)

@bptato bptato marked this pull request as ready for review April 30, 2025 17:51
@saghul
Copy link
Contributor

saghul commented Apr 30, 2025

Perhaps you could use Js_NewError and extract it?

@saghul
Copy link
Contributor

saghul commented Apr 30, 2025

I was confused for a minute because browsers set stack as a getter on error prototypes, but we set it as a property on individual Error objects. I ended up following our existing practice.

(stack itself is non-standard; the spec only says that if regular Errors have it, DOMException should also have it.

I don't know if the difference matters anywhere in practice.)

I think that should be fine.

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

Successfully merging this pull request may close these issues.

2 participants