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

doc: DOMException missing from “Errors” API reference docs page #40789

Open
1 task done
DerekNonGeneric opened this issue Nov 11, 2021 · 12 comments
Open
1 task done
Labels
abortcontroller Issues and PRs related to the AbortController API assert Issues and PRs related to the assert subsystem. discuss Issues opened for discussions and feedbacks. doc Issues and PRs related to the documentations. errors Issues and PRs related to JavaScript errors originated in Node.js core.

Comments

@DerekNonGeneric
Copy link
Contributor

📗 API Reference Docs Problem

  • Platform: Linux 5.10.47-linuxkit x86_64 GNU/Linux
  • Subsystem: errors

Location

Section of the site where the content exists

Affected URL(s):

Description

Concise explanation of the problem

DOMException, which was made globally available in v17.0.0, is not amongst the runtime errors listed in our “Errors” API reference docs.

It is, however, correctly listed on the “Globals” page.

By referencing the original (“Globals”) location from a brand new section dedicated to DOMException on our “Errors” page, I am thinking that we may be able to have it listed on both pages similar to how we list AssertionError on both /api/errors.html and /api/assert.html. Thoughts?

P.S. Unsure of how relevant this may be (perhaps wrt the potential misclassification of AbortError raised in #40692), but this seems worth mentioning in this issue…

Along w/ structuredClone (spec link) — which was also made globally available in v17.0.0 — we appear to have added our first (?) couple of objects to be classified as “platform objects” according to Web IDL (at least that’s mine and @domenic’s current interpret of the spec, anyway). Perhaps it may be worth explicitly identifying them as such in our documentation?

Any additional information/suggestions would be appreciated. It seems like it could simply be a minor oversight unless I am mistaken.

/cc @aduh95 @targos @Trott @BridgeAR @addaleax @mcollina @benjamingr @jasnell (et al. who may have more context)


  • I would like to work on this issue and
    submit a pull request.
@DerekNonGeneric DerekNonGeneric added assert Issues and PRs related to the assert subsystem. doc Issues and PRs related to the documentations. discuss Issues opened for discussions and feedbacks. errors Issues and PRs related to JavaScript errors originated in Node.js core. abortcontroller Issues and PRs related to the AbortController API labels Nov 11, 2021
@benjamingr
Copy link
Member

Yes, this is a good catch, we need to document DOMException and link to MDN (and probably the spec) as well as AbortError

@the-spyke
Copy link

What is the actual reason behind throwing a DOMException instead of a natural Error('ABORT_ERR')? Will all Node errors become browser errors?

@aduh95
Copy link
Contributor

aduh95 commented Feb 1, 2023

It makes it easier to write code that works on both browsers and Node.js – so if you want to check if the caught exception you just get is an AbortError, you can use if (err?.name === 'AbortError') and you know it will work on whatever runtime your code is running.
The alternative would be for each runtime to define its own way of handling abortion, and the devs would have to maintain separate branches for each runtime, but that's a pattern that the ecosystem is trying to move away from.

instead of a natural Error('ABORT_ERR')

FWIW none of Node.js errors is using this pattern.

Will all Node errors become browser errors?

Note sure what you mean here, Node.js will adopt the same APIs as browsers wherever it makes sense to do so (including errors), but both browsers and Node.js are already using the same Error class defined in ECMAScript spec, so in that sense it's already using browser errors.

@the-spyke
Copy link

@aduh95 There is a page with Node errors codes and ABORT_ERR is here. And to my understanding error codes were the Node's approach not the checking constructor name.

@aduh95
Copy link
Contributor

aduh95 commented Feb 1, 2023

FWIW we do use an AbortError class for all the none DOM APIs that interact with AbortController, and instances of that class have a .name property of AbortError and a .code property of ABORT_ERR, which is what the page you linked to refer to.

For DOM APIs (e.g. fetch. webstreams, etc.), we do follow a spec that defines a DOMException class and require abort errors to be instances of that class, so code using those APIs can be runtime agnostic.

@the-spyke
Copy link

the-spyke commented Feb 1, 2023

@aduh95 From the docs

All JavaScript and system errors raised by Node.js inherit from, or are instances of, 
the standard JavaScript <Error> class and are guaranteed to provide at least the 
properties available on that class.

Which doesn't fully apply here, but good to know that instanceof still works. Also, the code is a number, not a string. Is there a place to import this ABORT_ERR constant and how to create such error by yourself?

const { equal } = require('node:assert');
const { isNativeError } = require('node:util/types');

const ac = new AbortController();

ac.abort();

try {
  ac.signal.throwIfAborted();
} catch (err) {
  console.log(isNativeError(err)); // false
  console.log(err instanceof Error); // true
  console.log(isNativeError(new Error())); // true
  console.log(typeof err.code, err.code); // number 20
}

try {
  equal(1, 2);
} catch (err) {
  console.log(typeof err.code, err.code); // string ERR_ASSERTION
}

@aduh95
Copy link
Contributor

aduh95 commented Feb 1, 2023

Note that when calling ac.abort() with no arguments / with undefined, it will use a DOMException by default as defined by https://dom.spec.whatwg.org/#ref-for-dom-abortcontroller-abortcontroller%E2%91%A0.

Node.js specific APIs that deal with aborted signals will use a native error with code ABORT_ERR:

$ node -e 'events.once(global, "event", {signal:AbortSignal.abort()}).catch(e=>console.log(e.code, require("node:util/types").isNativeError(e)))'
ABORT_ERR true

@the-spyke
Copy link

@aduh95 Which means I should check for both 20 and ABORT_ERR as I can't know how a 3rd party library may abort the signal. And this is 20 too, so this global event throws a different error based on the original reason:

try {
  AbortSignal.abort().throwIfAborted();
} catch (err) {
  console.log(typeof err.code, err.code); // number 20
}

@aduh95
Copy link
Contributor

aduh95 commented Feb 1, 2023

Or you could check err?.name === 'AbortError'.

so this global event throws a different error based on the original reason

It's not a global event, I've used global and "event" as placeholders but it could be anything. The thing throwing is events.once. Sorry if that was confusing, I'm using an empty object and an empty string below, hopefully it's less confusing.

Note that AbortSignal.abort() is also using a DOMException by default (https://dom.spec.whatwg.org/#interface-AbortSignal). signal.throwIfAborted() is throwing whatever reason, it doesn't have to be an AbortError:

try {
  AbortSignal.abort(null).throwIfAborted();
} catch (err) {
  console.log(err); // null
}
try {
  AbortSignal.abort('reason').throwIfAborted();
} catch (err) {
  console.log(err); // "reason"
}
// Using an aborted signal with the string "reason" as the reason:
require('node:events').once({}, '', {signal:AbortSignal.abort('reason')})
  .catch(err => {
    console.log(err.code); // ABORT_ERR
    console.log(typeof err.cause, err.cause); // string reason
  });
// Using an aborted signal no explicit reason:
require('node:events').once({}, '', {signal:AbortSignal.abort()})
  .catch(err => {
    console.log(err.code); // ABORT_ERR
    console.log(typeof err.cause); // object
    console.log(err.cause?.code); // number 20
    console.log(err.cause?.name); // AbortError
  })

@the-spyke
Copy link

the-spyke commented Feb 1, 2023

@aduh95 I see that Node wraps DOM's AbortError into Node's AbortError. If I manually call ac.abort(message) it doesn't happen obviously. In this case I should pass an error instance. Is there a way to import Node's AbortError constructor? I see it is not defined in the global scope. If I just rename an Error, it is slightly different:

Error [AbortError]: This operation was aborted
  code: 'ABORT_ERR'
vs.
AbortError: The operation was aborted
  code: 'ABORT_ERR'

Thanks for the help!

Also Node's message says The operation was aborted while DOM says This operation was aborted. Notice the vs. this :)

@aduh95
Copy link
Contributor

aduh95 commented Feb 1, 2023

Node's message says The operation was aborted while DOM says This operation was aborted. Notice the vs. this :)

Good catch :) AFAIK the spec doesn't have any requirement for the error message, so I don't think it's a problem if those differ. FWIW it seems that all runtimes differ:

  • Node.js: This operation was aborted
  • Chromium browsers: signal is aborted without reason
  • Firefox: The operation was aborted.
  • Safari: The operation was aborted.
  • Deno: The signal has been aborted

@aduh95
Copy link
Contributor

aduh95 commented Feb 1, 2023

Is there a way to import Node's AbortError constructor?

There's no practical way. We wouldn't want to expose something like that in a Node.js specific way, we would want to either have it added to the ECMAScript spec, or at least have the WinterCG discuss how it can be exposed in a way that maximize code portability.
I think your custom class is your best bet for the time being, and in general any solution that ensures that checking err?.name === 'AbortError' gives the expected result, everything should be fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
abortcontroller Issues and PRs related to the AbortController API assert Issues and PRs related to the assert subsystem. discuss Issues opened for discussions and feedbacks. doc Issues and PRs related to the documentations. errors Issues and PRs related to JavaScript errors originated in Node.js core.
Projects
None yet
Development

No branches or pull requests

4 participants