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.

Sign up for GitHub

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

Support AbortController #2020

Merged
merged 20 commits into from
Jul 24, 2022
Merged

Support AbortController #2020

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
831d8b6
Add signal option and related tests
jopemachine Mar 29, 2022
452435a
Fix CI Error
jopemachine Mar 30, 2022
6bdd99a
Add initial rejection when already aborted
jopemachine Apr 27, 2022
9451678
Got stream should be aborted instead
szmarczak May 3, 2022
723b960
Restore logic and fix tests
jopemachine May 12, 2022
63f7b08
Revert "Restore logic and fix tests"
jopemachine May 13, 2022
b18f1fe
Simplify the codes
jopemachine May 13, 2022
f5a877b
Remove improper comment
jopemachine May 13, 2022
4ef257a
Move if statement's position to top
jopemachine May 13, 2022
f7d5cd5
Revert "Remove improper comment"
jopemachine May 14, 2022
b65e512
Revert "Simplify the codes"
jopemachine May 14, 2022
94dacf7
Fix tests
jopemachine May 14, 2022
0e6f539
Add `AbortError`
jopemachine Jul 11, 2022
83aef43
Implement throwing native DOMException when possible
jopemachine Jul 12, 2022
5e8a6ec
Revert "Implement throwing native DOMException when possible"
jopemachine Jul 22, 2022
cddec2d
Revert comment style
jopemachine Jul 22, 2022
67d12d4
Remove unnecessary asterisk from a comment
szmarczak Jul 22, 2022
710d1f0
Merge branch 'main' into support-abort-controller
szmarczak Jul 22, 2022
7406560
Reflect feedbacks
jopemachine Jul 23, 2022
94b5ed0
Update 2-options.md
sindresorhus Jul 24, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions documentation/8-errors.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,3 +97,9 @@ When the request is aborted with `promise.cancel()`.
**Code: `ERR_RETRYING`**

Always triggers a new retry when thrown.

### `AbortError`

**Code: `ERR_ABORTED`**

When the request is aborted with [AbortController.abort()](https://developer.mozilla.org/en-US/docs/Web/API/AbortController/abort).
10 changes: 10 additions & 0 deletions source/core/errors.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,3 +170,13 @@ export class RetryError extends RequestError {
this.code = 'ERR_RETRYING';
}
}

/**
An error to be thrown when the request is aborted by AbortController.
*/
export class AbortError extends RequestError {
constructor(request: Request, error?: Error) {
super('This operation was aborted.', {...error, code: 'ERR_ABORTED'}, request);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of cloning error, please set this.code like in errors above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since error (abortsignal.reason) is optional here, it seems passing error into the second argument of the constructor cause compile error.

// Compile error occurs

// Argument of type 'ErrnoException | undefined' is not assignable to parameter of type 'Partial<ErrnoException & { code?: string | undefined; }>'.
// Type 'undefined' is not assignable to type 'Partial<ErrnoException & { code?: string | undefined; }>'
super('This operation was aborted.', error, request);
this.code = 'ERR_ABORTED';

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see, thanks for clarifying. In this case you can just skip this. Like so:

super('This operation was aborted.', {}, request);
this.name = 'AbortError';
this.code = 'ERR_ABORTED';

The reason is user accessible via controller.signal.reason, so I don't think we need to expose it for now. That might be for #1953

this.name = 'AbortError';
}
}
7 changes: 6 additions & 1 deletion source/core/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import {
TimeoutError,
UploadError,
CacheError,
AbortError,
} from './errors.js';
import type {PlainResponse} from './response.js';
import type {PromiseCookieJar, NativeRequestOptions, RetryOptions} from './options.js';
Expand Down Expand Up @@ -235,8 +236,12 @@ export default class Request extends Duplex implements RequestEvents<Request> {
return;
}

if (this.options.signal?.aborted) {
this.destroy(new AbortError(this, (this.options.signal as any).reason));
}

this.options.signal?.addEventListener('abort', () => {
this.destroy(new Error('This operation was aborted.'));
this.destroy(new AbortError(this, (this.options.signal as any).reason));
});

// Important! If you replace `body` in a handler with another stream, make sure it's readable first.
Expand Down
2 changes: 1 addition & 1 deletion source/core/options.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1488,7 +1488,7 @@ export default class Options {
/**
You can abort the `request` using [`AbortController`](https://developer.mozilla.org/en-US/docs/Web/API/AbortController).

**Requires Node.js 16 or later.*
__Requires Node.js 16 or later.__
szmarczak marked this conversation as resolved.
Show resolved Hide resolved

@example
```
Expand Down
7 changes: 7 additions & 0 deletions test/abort.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ if (globalThis.AbortController !== undefined) {
});

await t.throwsAsync(gotPromise, {
code: 'ERR_ABORTED',
message: 'This operation was aborted.',
});

Expand Down Expand Up @@ -115,6 +116,7 @@ if (globalThis.AbortController !== undefined) {
});

await t.throwsAsync(gotPromise, {
code: 'ERR_ABORTED',
message: 'This operation was aborted.',
});

Expand All @@ -141,6 +143,7 @@ if (globalThis.AbortController !== undefined) {
});

await t.throwsAsync(gotPromise, {
code: 'ERR_ABORTED',
message: 'This operation was aborted.',
});
await t.notThrowsAsync(promise, 'Request finished instead of aborting.');
Expand All @@ -165,6 +168,7 @@ if (globalThis.AbortController !== undefined) {
});

await t.throwsAsync(gotPromise, {
code: 'ERR_ABORTED',
message: 'This operation was aborted.',
});
await t.notThrowsAsync(promise, 'Request finished instead of aborting.');
Expand All @@ -189,6 +193,7 @@ if (globalThis.AbortController !== undefined) {
controller.abort();

await t.throwsAsync(gotPromise, {
code: 'ERR_ABORTED',
message: 'This operation was aborted.',
});
await t.notThrowsAsync(promise, 'Request finished instead of aborting.');
Expand Down Expand Up @@ -242,6 +247,7 @@ if (globalThis.AbortController !== undefined) {
}, 400);

await t.throwsAsync(promise, {
code: 'ERR_ABORTED',
message: 'This operation was aborted.',
});
});
Expand All @@ -261,6 +267,7 @@ if (globalThis.AbortController !== undefined) {
controller.abort();

await t.throwsAsync(promise, {
code: 'ERR_ABORTED',
message: 'This operation was aborted.',
});
});
Expand Down