Thrown AbortErrors are not DOMExceptions

[kanongil]

Version

v17.0.1

Platform

any

Subsystem

No response

What steps will reproduce the bug?

import { readFile } from 'fs/promises';

const ref = new DOMException('The operation was aborted', 'AbortError');
console.log('REF INSTANCE', ref instanceof DOMException);
console.log('REF OBJECT', ref);

const ab = new AbortController();
ab.abort();

try {
    await readFile('does_not_matter', { signal: ab.signal });
}
catch (err) {
    console.log('ERR INSTANCE', err instanceof DOMException);
    console.log('ERR OBJECT', err);
}

How often does it reproduce? Is there a required condition?

100%, including for all other native APIs that supports signals.

What is the expected behavior?

From the docs:

If a request is aborted the promise returned is rejected with an AbortError

There is no mention that this is a non-standard new DOMException(message, 'AbortError') error. As such, I expect 'REF' and 'ERR' logged objects to be the same (except the stack).

What do you see instead?

REF INSTANCE true
REF OBJECT DOMException [AbortError]: The operation was aborted
    at file://<snip>/fail.mjs:3:13
    at ModuleJob.run (node:internal/modules/esm/module_job:185:25)
    at async Promise.all (index 0)
    at async ESMLoader.import (node:internal/modules/esm/loader:281:24)
    at async loadESM (node:internal/process/esm_loader:88:5)
    at async handleMainPromise (node:internal/modules/run_main:65:12)
ERR INSTANCE false
ERR OBJECT AbortError: The operation was aborted
    at checkAborted (node:internal/fs/promises:373:11)
    at readFile (node:internal/fs/promises:845:3)
    at file://<snip>/fail.mjs:11:11
    at ModuleJob.run (node:internal/modules/esm/module_job:185:25)
    at async Promise.all (index 0)
    at async ESMLoader.import (node:internal/modules/esm/loader:281:24)
    at async loadESM (node:internal/process/esm_loader:88:5)
    at async handleMainPromise (node:internal/modules/run_main:65:12) {
  code: 'ABORT_ERR'
}

Additional information

This is only an issue in node 17+, since previous versions did not have a DOMException global.

[benjamingr]

Hey, this is by design, you can see the context and meeting in https://github.com/nodejs/node/issues/36084. Node.js does throw DOMExceptions for DOM APIs. For its own APIs Node.js prefers a simpler AbortError with the same .name property (which is the preferred way to check for AbortErrors according to the DOM spec). The reason for this is that Node.js vendors some of its modules (like readable-stream) and having to ship DOMException with them would be hard.

[kanongil]

@benjamingr That design does not appear to have been revised when node added support for native DOMException. This is causing extra confusion, and the root of this issue.

Essentially, this decision puts the onus on API consumers to handle the strange complexity, and on API documenters to clearly convey this, just so node can potentially simpler vendor readable-stream (which has not actually happened due to other complexities).

Given that there is no obvious central place to document this complexity, any API that uses a signal must clearly communicate this, which is currently failing.

I hope you will take a chance to reconsider this. You are prioritizing the efforts of a few core developers against the documentation team, and the entire developer base.

[benjamingr]

@benjamingr That design does not appear to have been revised when node added support for native DOMException. This is causing extra confusion, and the root of this issue.

Node has added DOMException before AbortSignal - the decision was to move away from DOMExceptions for non DOM APIs using AbortSignal and stick to .name (the reasoning is in the thread I linked to and in the meeting minutes).

(Node does vendor readable-stream as the readable-stream package on NPM btw - but it hasn't been updated)

Given that there is no obvious central place to document this complexity, any API that uses a signal must clearly communicate this, which is currently failing.

Would it help if the documentations clarified the AbortError interface we use like we do for AbortSignal and friends?

I hope you will take a chance to reconsider this. You are prioritizing the efforts of a few core developers against the documentation team, and the entire developer base.

I don't think that's an entirely fair or an accurate representation of how things transpired:

• AbortController/AbortSignal was added to the platform to enable cancellation scenarios.
• We iterated on them and added them to more APIS while soliciting responses from the community, library authors, WHATWG folks and other subject-matter-experts.
• When soliciting feedback from library vendors and core - the concern about DOMException being complicated and different was raised. There were no objections to changing it - you are in fact the first person to bring up this complaint in the ±year we've shipped this.
• The process was open and in the public, anyone from the community was welcome to participate and many were invited specifically so we collect user feedback. We've had a meeting about this in the summit (online, also open to everyone) and several other discussions.
• This decision was made considering the people who built AbortController/AbortSignal/AbortError in the DOM in the first place, people who worked on other AbortSignal APIs, the community, library authors and more.
• Things quieted down pretty quickly and we're getting positive feedback.

Also I want to point out that the decision isn't final, we can revisit anything here. We did not receive any pushback about this change from the documentation team nor developer base. We are happily willing to listen to feedback.

All of this is relatively new and improvement opportunities (in docs, APIs, debugging experience and more) are welcome. Contributions are welcome and Node.js in general and this area in particular is eager to receive them.

[kanongil]

Thanks for the thorough response, and good to know that there is still room for iterating the approach.

DOMException has only just been made public with node v17, so has only been there for a few weeks for API consumers. It was merged August 6'th in e4b1fb5e6422c1ff151234bb9de792d45dd88d87, and I don't see any considerations on how this impacts AbortError in any of the relevant PRs. Issue #39098 explicitly mentions the ability to do instanceof checks. #39176 can hardly have been a consideration almost a year ago in #36084.

Also, most feedback you will have gotten so far is likely not from the wider dev community, as many still support node v12, and are only just starting to look into using the features v14+ provides due to v12 EOL coming up.

FYI, I'm very exited about this new feature, which is why I want to see it done right. Here I think unifying on new DOMException(message, 'AbortError') for node native and modules targeting v17+ will make it simpler to use. Both as providers (those throwing) and consumers (those passing signal / calling abort()) that are likely to have to ignore the subsequent rejections. Besides making it simpler to use the feature, it also enables compatibility with browser runtimes.

This still leaves module that target v14/v16 and v17+ "in the dust", having to deal with extra complexity to handle two different error implementations. However, they are no worse of than the current situation, and will eventually be able to drop the legacy codepaths.

Given that there is no obvious central place to document this complexity, any API that uses a signal must clearly communicate this, which is currently failing.

Would it help if the documentations clarified the AbortError interface we use like we do for AbortSignal and friends?

Yes – this would be simpler to document if the internal AbortError was public (as discussed in #38361).

[benjamingr]

Thanks for the thorough response, and good to know that there is still room for iterating the approach.

Absolutely.

DOMException has only just been made public with node v17, so has only been there for a few weeks for API consumers. It was merged August 6'th in e4b1fb5, and I don't see any considerations on how this impacts AbortError in any of the relevant PRs. Issue #39098 explicitly mentions the ability to do instanceof checks.

It's true that DOMException has been made public in Node.js 17 but it has been in core for a few years now (since things like URL raise it). Note that a public DOMException is even worse for stuff like readable-stream that exists for compatibility across versions since people may expect instanceof checks to work (and they won't) - neither will instanceof checks work across vm contexts and other "realms".

In general it makes perfect sense to expose DOMException as well as AbortError but people should never rely on instanceof checks for errors. That's why there were proposals for stuff like Error.isError.

If you read the WHATWG fetch specification carefully (I am happy to dig up the discussions where we initially met to discuss AbortController) - you will see it makes no requirements on raising AbortError. In fact when I added an API myself (signal in addEventListener) there were no errors involved.

DOM APIs are encouraged but not required to reject with an AbortError DOMException - non DOM APIs have no such requirements. If you look at the spec example it encourages checking .name rather than instanceof (for the aforementioned reasons) that is quite intentional.

#39176 can hardly have been a consideration almost a year ago in #36084.

I actually recall seeing (but not having time to review) that and trusting reviewers since I think it's overall a good idea.

Also, most feedback you will have gotten so far is likely not from the wider dev community, as many still support node v12, and are only just starting to look into using the features v14+ provides due to v12 EOL coming up.

This was less "users approaching us" and more "us approaching users and asking what they think" and "us talking to people who have experimented with userland AbortSignals (like SDK authors) and asking them what feedback their users are giving. Gathering user and library maintainer feedback is always a big challenge in Node.js and always something we could use more of.

FYI, I'm very exited about this new feature, which is why I want to see it done right. Here I think unifying on new DOMException(message, 'AbortError') for node native and modules targeting v17+ will make it simpler to use. Both as providers (those throwing) and consumers (those passing signal / calling abort()) that are likely to have to ignore the subsequent rejections. Besides making it simpler to use the feature, it also enables compatibility with browser runtimes.

Note that we switched from DOMException - I think this is where it was added after the meeting and consensus.

Unless the concerns raised by @mcollina regarding this being hard for readable stream and other parts were addressed in his perspective - I don't see us switching back to DOMExceptions (which wouldn't break a lot of code probably since people are supposed to check .name but it might).

Yes – this would be simpler to document if the internal AbortError was public (as discussed in #38361).

Is that something you'd be interested in contributing? I know we've talked about this in the past but I am honestly not sure what it's blocked on and I am happy for us to start with a simpler flagged errors module and add more codes/errors.

How can I help you with this? (I can review, ping the TSC to discuss the module inclusion, try to get more people to look at this etc).

[mcollina]

I don't have much time right now for joining a conversation with this depth. From a quick skim @benjamingr summarizes this correctly. My position has not changed on this matter.

[kanongil]

We agree that it makes sense for user code to distinguish AbortError from regular errors, eg. to avoid logging it, right? Thus it makes sense that the error included in the rejection (when triggered) is clearly identifiable. As you say, this is currently done through checking the .name property, but it relies on APIs to reject using properly crafted errors.

While the node APIs can be aligned on what is thrown, it is anyones guess what third-party modules will throw. And since those modules might forward the signal to submodules, the same API call could be able to reject using two or more completely different abort errors, depending on when it is aborted. Ie. you can't even reliably test against how abort errors are emitted.

For a different take on this, node could expose helpers like this to enable tests for .aborted and throwing using a standardized AbortError, making it less likely that third party modules get it wrong. And probably also something for the rare cases that the 'abort' event is used. This can be done without exporting the AbortError class.

Btw, this issue is really about standardizing the rejected error, be it through always using DOMException or some other means. I'm naturally inclined towards DOMException since it better aligns with browsers, but if it is a clear no-go, then there are still other ways to improve the situation.

[benjamingr]

I think we are all very much in favor of standardizing third-party thrown exceptions and providing guidance to people authoring async APIs with cancellation. I think that's a good idea and I don't think that was contested.

I think that is mostly blocked on someone doing the work. If exposing errors is a problem (I think it can be done and think the block is just technical) I am fine with exposing it elsewhere.

[domenic]

I saw claims like this a few times:

If you look at the spec example it encourages checking .name rather than instanceof (for the aforementioned reasons) that is quite intentional.

I can state that is not intentional, and using .name is not the recommended way. That one example in the DOM spec is just an example. All coding styles are valid and OK on the web; instanceof, for example, is fine (if you're working within a single realm). As is checking err.code === DOMException.ABORT_ERR.

I do think it would be beneficial to the ecosystem if Node didn't create its own separate AbortError type, and instead used and encouraged the standard DOMException. I think as @kanongil points out the fact that there are two is going to cause a lot of ecosystem confusion, as e.g. code that wants to be web/Deno compatible uses DOMException, and code which doesn't care about such compatibility uses Node's one-off version, and so on.

[ljharb]

Why would it be a good thing for environments without a DOM to have or use a DOM exception? If it was meant to be a universal exception type then it seems it was unwisely named.

[domenic]

Yes, it's pretty easy to complain about names of things in the JavaScript ecosystem; as I'm sure you're aware we haven't named everything optimally in its 20+ year history.

[benjamingr]

@domenic to be clear the thing blocking DOMExceptions for AbortErrors in Node.js isn't the fact people don't like DOMExceptions (we already ship DOMExceptions anyway).

It's the fact DOMExceptions become a dependency of libraries (like readable-stream) that are explicitly designed to run in non-modern Node.js environments and porting DOMException there would be hard.

The current solution is a compromise that enabled us to adopt AbortSignals in all APIs while staying spec-compliant. Obviously we would be happier shipping less kinds of errors for the same thing :)

@ljharb

Why would it be a good thing for environments without a DOM to have or use a DOM exception?

It's a communications thing but IMO the DOMness of the exception is "this exception comes from a DOM API" rather than requiring a DOM tree.

[benjamingr]

Oh, and regarding:

I saw claims like this a few times: I can state that is not intentional, and using .name is not the recommended way. That one example in the DOM spec is just an example.

@domenic I think we got that feedback/best-practice from @jakearchibald both in the AbortError meeting regarding it and back when cancellable fetch was discussed since it inter-ops between realms and is guaranteed.

If there is other preferred guidance we are happy to look at alternatives (though it would have been great a year ago).

(Also note I am happy to ping you more on these issues but I know you're busy and remember you don't like direct pings so I've been limiting the amount of pings to cases spec-compliance was on the line)

(And of course the thing that blocked using DOMExceptions wasn't their name anyway it was the difficulty vendoring)

[ljharb]

@benjamingr "came from a DOM API" means we'd be expecting users to know the difference between specifications, something web browser representatives have repeatedly insisted they do not and should not understand.

I'm not saying the name of DOMException is the issue; as much as it is that AbortError inherits from it. Is it really too late for the web to fix that?

[domenic]

Yes, I agree developers should not have to know where the error came from. That's why I think Node.js should follow other ecosystems in using DOMException for abort errors instead of inventing its own one-off.

We have no plans to change how the web throws abort errors to use anything other than DOMException.

[Jamesernator]

I'm not saying the name of DOMException is the issue; as much as it is that AbortError inherits from it. Is it really too late for the web to fix that?

Actually AbortError doesn't inherit DOMException, rather there is just a single DOMException class such that every kind of DOM error is, just the .name/.code properties get set differently.

We have no plans to change how the web throws abort errors to use anything other than DOMException.

Do you think it would be viable to make AbortError an actual subclass of DOMException? This way people would be able to do err instanceof AbortError and new AbortError(message) as expected. We could even support existing uses of new DOMException("message", "AbortError") by having a custom [Symbol.hasInstance].

This would mean AbortError would be an actual global rather than the confusing and poorly named DOMException, but should not be an otherwise breaking change. (This could be done with other kinds of DOMException too, but I don't know if there would be as much demand as people wouldn't generally construct those themselves).

[domenic]

We have no plans to change how the web throws abort errors to use anything other than DOMException.

[ljharb]

That's clear, but I think the request is to perhaps create those plans.

[benjamingr]

@DerekNonGeneric this is not a confirmed bug - the fact this happens was a design decision that carefully considered the trade-offs. We can re-evaluate this further and see if we can stop vendoring readable-stream (or only vendor it in environments that have DOMExceptions) or we can explore DOM changes (though I'm not sure how we'd make a compelling case for browsers) or anything really.

Let's not "fix" this until there is consensus it should be fixed and how :)

[DerekNonGeneric]

My only argument was being in favor of code portability, but open to finding what other ways we would be able to achieve that.

[benjamingr]

@DerekNonGeneric I am not arguing for/against using DOMExceptions for AbortErrors right now - I am just asking not to change the behaviour without discussions. We started with DOMExceptions and moved away from them (see the discussion above :)), I suspect that if we refactor back people will not be happy.

In particular readable-stream would need to vendor DOMException inside readable-stream and would like to avoid that.

[Jamesernator]

That's clear, but I think the request is to perhaps create those plans.

Yes, there's plenty of improvements on the web for ergonomics of APIs all the time. I don't see why DOMException would not be eligible for such improvements.

We have no plans to change how the web throws abort errors to use anything other than DOMException.

And to be clear, I don't want to break existing code which is why my suggestion above made AbortError a subclass so that code that already uses DOMException still works as usual. However it would still offer the ergonomic benefits like new AbortError("aborted for reason") or err instanceof AbortError working as expected.

@DerekNonGeneric I am not arguing for/against using DOMExceptions for AbortErrors right now - I am just asking not to change the behaviour without discussions. We started with DOMExceptions and moved away from them (see the discussion above :)), I suspect that if we refactor back people will not be happy.

For this, something worth pointing out is that the spec for AbortController/AbortSignal has actually added quite a bit tighter integration with DOMException/AbortError recently. In particular there is a new property signal.reason that is set by default to an AbortError DOMException. i.e.:

const controller = new AbortController();
controller.abort();

console.log(controller.signal.reason); // DOMException: AbortError
``

This `.reason` property is intended to be [thrown from all APIs as part of its integration](https://github.com/whatwg/dom/issues/1030). i.e. If you're aborting from a signal it would be equivalent to this:

```js
if (signal.aborted) {
    throw signal.reason;
}

signal.addEventListener('abort', (event) => {
    reject(event.target.signal);
});

The .reason can be set to a custom value instead too though i.e.:

class MyCustomError extends Error { name="MyCustomError" }

const controller = new AbortController();
controller.abort(new MyCustomError());

console.log(signal.reason); // MyCustomError

I don't know if this throwing behaviour is something Node will adopt or not, but if it does this essentially removes control of what kind of error is thrown from the code utilizing an AbortSignal entirely instead deferring such decisions to whoever has the abort controller.

[kanongil]

signal.reason seems like a very sensible API, and has already been merged in https://github.com/whatwg/dom/pull/1027.

Using this in node would make the readable-stream issue moot, since the associated abort logic would no longer generate any error, just throw the provided signal.reason. This makes whether it is a DOMException, or a custom AbortError, up to the signal provider.

[benjamingr]

@kanongil If I understand correctly signal.reason doesn't replace the error does it?

@Jamesernator to explain: in order for the DOM to consider not using DOMExceptions it would have to be specifically interesting to the browser stakeholders. So you would need to present a compelling argument on how this would benefit browsers rather than a non-browser-related Node.js compatibility issue.

[benjamingr]

Hmm, .reason superceding the AbortError is actually pretty risky since it means consumers can raise arbitrary error subclasses "from the outside" and users can't catch cancellation with AbortErrors.

I think Node.js should still wrap errors with AbortError probably even if reason is passed.

[kanongil]

@benjamingr Yes, it should replace the generated error, as specified here.

[kanongil]

In most cases the user will also be the one calling .abort() on the AbortController, in which case it is not a problem, but a feature.

Eg. a test case runner that provides a signal property to the case logic can now call ac.abort(new CaseClosedError()) when the test logic fails, and properly ignore any subsequent rejections with that error.

[benjamingr]

@kanongil This sounds dangerous since it means users will be able to throw non-AbortErrors into controllers they own which means the contract for "what this method rejects with" changes from "Operational Errors + AbortError" to "Operational Errors + whatever whomever has access to the controller chooses to throw into the method".

@domenic can you clarify/confirm this is the intended behaviour?

For example can I pass a signal to a fetch and then get that fetch to reject with an arbitrary error object?

[kanongil]

@benjamingr Yeah, I guess it could cause an issue for code that wants to test for this, like:

try {
    await fetch(…, { signal });
}
catch (err) {
    if (err.name === 'AbortError') {
        throw err;
    }

    // else retry the fetch
}

It will have to be reworked:

try {
    await fetch(…, { signal });
}
catch (err) {
    if (signal.reason) {
        throw signal.reason;
    }

    // else retry the fetch
}

[benjamingr]

Hmm, this seems rather unfortunate. Intuitively I'd prefer it if it was only possible to abort with subclasses of DOMException AbortErrors even if that'd force Node.js to use DOMExceptions in the implementation.

[kanongil]

FYI, there are plans to signal some aborts using new DOMException("TimeoutError") as well: https://github.com/whatwg/dom/issues/951#issuecomment-922833719

[domenic]

can you clarify/confirm this is the intended behaviour?

For example can I pass a signal to a fetch and then get that fetch to reject with an arbitrary error object?

Yes, this is intended behavior. And indeed we are already using this in streams and soon in fetch.

[benjamingr]

@domenic and how are users expected to distinguish cancellation from other (operational) errors? Is there a best practice we should follow here?

[domenic]

In general if you or a library/platform feature signals a given abort reason in one part of your application, the rest of your application should be prepared to deal with it. I don't think there's anything particularly unusual about this in terms of how exceptions normally work; it's a two-sided contract between the thrower (in this case signaler) and the catcher (listener).

[Jamesernator]

@domenic and how are users expected to distinguish cancellation from other (operational) errors? Is there a best practice we should follow here?

In general I'd suggest just handling errors like you normally do, i.e. if someone wants to abort with some other kind of error, their intention is probably that you not treat that error as special if you don't know what it is. Ultimately whatever code is actually triggering the abort should be ready for its own error.

Like consider fetch as an example, if I were to create a controller and pass its signal to fetch("...someurl...", { signal: controller.signal }) then were to abort with .abort(new MyCustomError()) then given I create MyCustomError I should know how to handle it. Even if you have a delegated signal, i.e. you're forwarding a signal, if that signal were to have MyCustomError reason, you should just throw it further up the chain as you don't know how to handle it. Existing errors such as network failures or whatever would continue to be treated as such as per normal.

There is one weirdness and that's that you could throw fake errors into these things to masquerade as errors that thing could usually throw, in these cases either I'd just assume all errors are legitimate, as if they abort with a masqueraded error, they probably want you to execute code for that error (if you know how to handle it).

[kanongil]

@Jamesernator There is still the case where some intermediate logic (that you might not control) wants to handle abort errors in a special way, as in https://github.com/nodejs/node/issues/40692#issuecomment-972712108. However, this is not an issue for new code, only legacy code that could expect a regular abort error.

For compatibility the intermediate logic could handle it like this:

try {
    await fetch(…, { signal });
}
catch (err) {
    if (signal.aborted) {
        throw signal.reason ?? err;
    }

    // else retry the fetch
}

[kanongil]

Maybe node needs to change its guidelines to not test for err.name === 'AbortError' (like in stream), but instead just test signal.aborted?

[kanongil]

I took the liberty of creating a PR that removes all references to the internal AbortError and uses signal.aborted to detect if an error is from an abort: #40883

Let me know what you think?

[benjamingr]

@jasnell pointed out that .reason does not imply/suggest/guarantee that it is the exception thrown when a promise API is rejected.

So I think my current ask in this regard is really just "have a way to know when an error is cancellation" blessed by the platform. Node can ship this without WHATWG (with a static) but I think this is important for users elsewhere too so I am going to try and figure it out in the WHATWG/dom repo.

[jasnell]

I agree that we should be using DOMException(AbortError) to signal cancelations (or DOMException(TimeoutError) when AbortSignal.timeout() is finished. Having a separate AbortError that is specific to Node.js is confusing. Where Node.js currently throws (or rejects) with an AbortError, we'll need to take those APIs through a deprecation cycle to transition to DOMException(AbortError), which is a bit irritating, but that's our own issue.

@benjamingr, I still don't really know what you mean by "know when an error is cancellation".

[benjamingr]

@jasnell Node.js moved from DOMEXceptions for vendoring reasons (so that readable-stream would not need to vendor DOMExcepiton). If that concern has been resolved we can revert that decision.

[Jamesernator]

So I think my current ask in this regard is really just "have a way to know when an error is cancellation" blessed by the platform. Node can ship this without WHATWG (with a static) but I think this is important for users elsewhere too so I am going to try and figure it out in the WHATWG/dom repo.

What are the situations where you actually want to distinguish cancellation from other error kinds WHERE you don't control the AbortController?

@jasnell pointed out that .reason does not imply/suggest/guarantee that it is the exception thrown when a promise API is rejected.

I do agree that .reason not being guaranteed to be the thing thrown is weird, but that design falls out of the fact that AbortSignal was done in userland because TC39 never came to consensus on a design for cancellation to be distinct. Building on what JS provides, errors are the only mechanism by which things will bubble up the stack, so there's not really choices in this regard.

Also it's not really any different to the fact that an API can just outright ignore the AbortSignal and throw any error it wants at any time for any reason anyway, .reason is a bit nicer because it gives the person requesting the abort more control rather if the API accepting the AbortSignal is willing to accept a fairly simple implicit contract.

@jasnell Node.js moved from DOMEXceptions for vendoring reasons (so that readable-stream would not need to vendor DOMExcepiton). If that concern has been resolved we can revert that decision.

If readable-stream simply throws .reason then it's no longer constructing its own errors for aborts anyway, so it shouldn't need to depend on DOMException.

[Jamesernator]

So I think my current ask in this regard is really just "have a way to know when an error is cancellation" blessed by the platform. Node can ship this without WHATWG (with a static) but I think this is important for users elsewhere too so I am going to try and figure it out in the WHATWG/dom repo.

What are the situations where you actually want to distinguish cancellation from other error kinds WHERE you don't control the AbortController?

@jasnell pointed out that .reason does not imply/suggest/guarantee that it is the exception thrown when a promise API is rejected.

I do agree that .reason not being guaranteed to be the thing thrown is weird, but that design falls out of the fact that AbortSignal was done in userland because TC39 never came to consensus on a design for cancellation to be distinct. Building on what JS provides, errors are the only mechanism by which things will bubble up the stack, so there's not really choices in this regard.

Thinking about this for a minute, you actually can distinguish cancellations from other errors. This isn't something I'd recommend for general people to use (general users should just treat the errors by their kind, not how they were thrown), but if you really must you can do:

async function myCancelableOperation(abortSignal) {
    try {
        await someOtherOperation(abortSignal);
    } catch (error) {
        if (abortSignal.aborted && error === abortSignal.reason) {
            // The error was a cancellation
        } else {
            // Not a cancellation
        }
    }
}

[benjamingr]

Thinking about this for a minute, you actually can distinguish cancellations from other errors. This isn't something I'd recommend for general people to use (general users should just treat the errors by their kind, not how they were thrown), but if you really must you can do:

This only works if you have a reference to the signal.

Building on what JS provides, errors are the only mechanism by which things will bubble up the stack, so there's not really choices in this regard.

The ask is not for true third-state cancellation only to be able to distinguish it as was possible until now. I realize that we don't have to throw signal.reason and can keep reason AbortErrors in our code happily while giving signal.reason as metadata which should be fine.

What are the situations where you actually want to distinguish cancellation from other error kinds WHERE you don't control the AbortController?

See examples in whatwg repo. Basically almost any time a framework is involved?

[tilgovi]

If, instead of wrapping reason with new AbortError(..., { cause: signal.reason }), Node.js followed fetch in throwing reason itself, wouldn't error === signal.reason work to detect cancellation?

[tilgovi]

@benjamingr I think this is a better place to respond to your comment about fetch^1 than on the closed issue for that, but please direct me if there's a more constructive way I could engage with this issue.

I'd like to make the case that Node should switch to throwing reason in the APIs that use AbortSignal. I'm keeping an open mind to be convinced otherwise, though.

As a consumer of a signal that forwards it to another function, any code I write that looks like this

try {
  await forward({ signal });
} catch (err) {
  if (err.name === 'AbortError') { /* ... */ }
  throw err;
}

can just as easily be written like this

try {
  await forward({ signal });
} catch (err) {
  if (err === signal.reason) { /* ... */ }
  throw err;
}

I'd like to argue that the latter is more accurate. Instead of reacting to the fact that something was thrown that has a name property equal to AbortError, it's possible to know for sure that what happened was that the signal caused the operation to abort. Otherwise, it's possible some other thing was thrown that has that name, or some sub-operation of forward aborted for some other reason and forward didn't catch it, and so on.

Whether or not such a function should or should not propagate the reason by re-throwing it, and under what conditions, feels like it's not for anyone but the implementor to decide. I just argue that we should give them that power by propagating the reason in Node APIs.

Anyone creating an AbortController and passing its signal into Node APIs that accept it doesn't have to change their code. The check for err.name === 'AbortError' will continue to work. But for library authors that might consume and forward a signal, it should be possible to switch without affecting callers (unless they decide to throw something else).

The only place where this falls down is on v16, where AbortController.prototype.abort() without an argument doesn't set signal.reason. I hope we could bury that inconsistency or perhaps backport the default reason to v16.

[benjamingr]

I am currently both sick and on bereavement leave so I can't leave a proper full response but:

There are two major issues with this:

• I think breaking the way everyone checks errors everywhere is a huge breaking change and I am ver hesitant for Node.js to do it and break all our error guarantees here (it would certainly be semver-major).
• You really don't always have access to the AbortController/AbortSignal when doing these sort of checks - and even if you do there might be multiple signals (e.g. one tied to the process's life time, one to an http request's).

[tilgovi]

No rush on any fuller response, but I'll leave my reactions for when you return:

I think breaking the way everyone checks errors everywhere is a huge breaking change and I am ver hesitant for Node.js to do it and break all our error guarantees here (it would certainly be semver-major).

This is a proper concern, but I'm hoping it's mitigated by the fact that for any case where abort() is called without a reason argument the error handling could remain the same. I'd hope there isn't a lot of code in the wild passing reason yet, but I still sympathize with this concern. If it's done in a semver-major change, code will have to write if (error === signal.reason || error.cause === signal.reason) until Node.js 18 EOL. Unfortunate, but also understandable.

You really don't always have access to the AbortController/AbortSignal when doing these sort of checks - and even if you do there might be multiple signals (e.g. one tied to the process's life time, one to an http request's).

This is exactly the argument in favor of the change, I think. If one has multiple signals, one can tell them apart if a rejection surfaces an object that is one of the reasons of one of the signals. Otherwise, we're stuck guessing at which signal, if any, might be the cause. If one doesn't have the signal at all, checking properties of the error is still the best bet regardless, and properly re-throwing unexpected errors (if you catch them at all) is normal.

The use case I have in mind as I think about this is one where there is more than one concurrent, asynchronous call that has yet to resolve. One might want to abort them gracefully, e.g. in response to SIGTERM. But one could throw exceptions into them that cause them to reject with something other than an abort error, effectively causing them to exit less gracefully.

[benjamingr]

I think breaking the way everyone checks errors everywhere is a huge breaking change and I am ver hesitant for Node.js to do it and break all our error guarantees here (it would certainly be semver-major).

This is a proper concern, but I'm hoping it's mitigated by the fact that for any case where abort() is called without a reason argument the error handling could remain the same.

It would have been nice if it was but unfortunately this changes the API surface consumers have to deal with from "This API can reject with an AbortError" to "This API can reject with anything" and code surrounding it would now have to deal with other types of errors.

Since virtually all of Node's APIs supports this this is a big breaking change in errors. I am honestly not sure semver-major is enough for such a big breaking change in error handling. Note that single error-code changes are semver-major and this is a lot more drastic.

Node has a lot less lee-way in these sort of breaking changes.

This is exactly the argument in favor of the change, I think. If one has multiple signals, one can tell them apart if a rejection surfaces an object that is one of the reasons of one of the signals. Otherwise, we're stuck guessing at which signal, if any, might be the cause.

We have mitigated this by setting the .cause on the error with the .reason of the signal so you can do this today without increasing the API surface.

That said like with exceptions you typically shouldn't have to care which signal caused the abort only that the abort happened to clean up and propagate the error. So while this use case is possible I am not sure it's one people should reach out to often.

The use case I have in mind as I think about this is one where there is more than one concurrent, asynchronous call that has yet to resolve. One might want to abort them gracefully, e.g. in response to SIGTERM. But one could throw exceptions into them that cause them to reject with something other than an abort error, effectively causing them to exit less gracefully.

I acknowledge that communicating extra information or metadata is a totally valid use case. If the whatwg design forced AbortError as a base class or .name to be AbortError or another reliable method to not break existing code but attach metadata to it I'd be game. I'm not sure with the design they went with we can do better than .cause