Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork1.1k
supportAbortSignal.reason#1775
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Conversation
In enviroments that implement it.When an `AbortSignal#reason` has a value, i.e. in nodejs >= v17.2.0,it is added on the returned `AbortError`. `AbortError#reason` would be`undefined` otherwise.Contary to the specification, when `AbortController#abort` is called,with or without a reason, `fetch` _does not_ throw the`AbortSignal#reason`.
| * AbortError interface for cancelled requests | ||
| */ | ||
| exportclassAbortErrorextendsFetchBaseError{ | ||
| constructor(message,type='aborted'){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Is this a breaking change?
AbortError is exported publicly from the package...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Yeah, wasn't aware the constructor was actually exported. It just seemed odd that thetype would be explicitly given, i.e. what else could it be.
We basically need to thread thereason into the thrownAbortError. To not break its constructor, the only sane option would be to extend it:
// no change to existing `AbortError`classAbortWithReasonErrorextendsAbortError{constructor(message,reason){super(message);this.reason=reason;}}
In enviroments that implement it.
When an
AbortSignal#reasonhas a value, i.e. in nodejs >= v17.2.0, it is added on the returnedAbortError.AbortError#reasonwould beundefinedotherwise.Contrary to the specification, when
AbortController#abortis called, with or without a reason,fetchdoes not throw theAbortSignal#reason. This is to allow more fine grained error handling. Had thereasonwas thrown, the general information that an abort happened is lost and it becomes sole responsibility of the reason provider.Purpose
Allow detecting thereason fetch was aborted.
Changes
AbortErrorto have fieldreason. It's constructor takes this additional argument.AbortSignal#reasonis given when constructingAbortErrorinstance. Could possibly beundefinedin environments/libraries/polyfills that do not support it.Additional information
The actual value (even existence) of
AbortSignal#reasonis external to this library. We make no assumptions about it. We simply forward it.This goes against#1519. There is no inherent reason to follow the specifications to the dot.
node-fetchis for node, specifically, not the web. TBD.