From 831d8b68c2ff5f396e81a4a16fdbdc2b42d3e0e8 Mon Sep 17 00:00:00 2001 From: Gyubong Lee Date: Tue, 29 Mar 2022 19:27:11 +0900 Subject: [PATCH 01/19] Add signal option and related tests --- documentation/2-options.md | 23 ++++ source/as-promise/index.ts | 15 ++- source/core/options.ts | 33 +++++ source/create.ts | 20 ++- test/abort.ts | 265 +++++++++++++++++++++++++++++++++++++ 5 files changed, 353 insertions(+), 3 deletions(-) create mode 100644 test/abort.ts diff --git a/documentation/2-options.md b/documentation/2-options.md index f5e10669a..55adc525b 100644 --- a/documentation/2-options.md +++ b/documentation/2-options.md @@ -209,6 +209,29 @@ await got('https://httpbin.org/anything'); #### **Note:** > - If you're passing an absolute URL as `url`, you need to set `prefixUrl` to an empty string. + +### `signal` + +**Type: [`AbortSignal`](https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal)** + +You can abort the `request` using [`AbortController`](https://developer.mozilla.org/en-US/docs/Web/API/AbortController). + +*Requires Node.js 16 or later.* + +```js +import got from 'got'; + +const abortController = new AbortController(); + +const request = got('https://httpbin.org/anything', { + signal: abortController.signal +}); + +setTimeout(() => { + abortController.abort(); +}, 100); +``` + ### `method` **Type: `string`**\ diff --git a/source/as-promise/index.ts b/source/as-promise/index.ts index 95ce06dd2..8b658bd78 100644 --- a/source/as-promise/index.ts +++ b/source/as-promise/index.ts @@ -22,13 +22,26 @@ const proxiedRequestEvents = [ 'downloadProgress', ]; -export default function asPromise(firstRequest?: Request): CancelableRequest { +export default function asPromise(firstRequest?: Request, signal?: AbortSignal): CancelableRequest { let globalRequest: Request; let globalResponse: Response; let normalizedOptions: Options; const emitter = new EventEmitter(); const promise = new PCancelable((resolve, reject, onCancel) => { + if (signal) { + signal.addEventListener('abort', () => { + globalRequest.destroy(); + + // To do: remove below if statement when targets node is 17.3.0 or higher. + if ((signal as any).reason) { + reject((signal as any).reason); + } else { + reject(new DOMException('This operation was aborted', 'AbortError')); + } + }); + } + onCancel(() => { globalRequest.destroy(); }); diff --git a/source/core/options.ts b/source/core/options.ts index 33073636d..65655844c 100644 --- a/source/core/options.ts +++ b/source/core/options.ts @@ -827,6 +827,7 @@ const defaultInternals: Options['_internals'] = { }, setHost: true, maxHeaderSize: undefined, + signal: undefined, }; const cloneInternals = (internals: typeof defaultInternals) => { @@ -1484,6 +1485,36 @@ 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.* + + @example + ``` + import got from 'got'; + + const abortController = new AbortController(); + + const request = got('https://httpbin.org/anything', { + signal: abortController.signal + }); + + setTimeout(() => { + abortController.abort(); + }, 100); + ``` + */ + get signal(): AbortSignal | undefined { + return this._internals.signal; + } + + set signal(value: AbortSignal | undefined) { + assert.object(value); + + this._internals.signal = value; + } + /** Ignore invalid cookies instead of throwing an error. Only useful when the `cookieJar` option has been set. Not recommended. @@ -2410,6 +2441,7 @@ export default class Options { headers: internals.headers, createConnection: internals.createConnection, timeout: internals.http2 ? getHttp2TimeoutOption(internals) : undefined, + signal: internals.signal, // HTTP/2 options h2session: internals.h2session, @@ -2473,5 +2505,6 @@ export default class Options { Object.freeze(options.retry.methods); Object.freeze(options.retry.statusCodes); Object.freeze(options.context); + Object.freeze(options.signal); } } diff --git a/source/create.ts b/source/create.ts index 376fda30f..584a57725 100644 --- a/source/create.ts +++ b/source/create.ts @@ -34,6 +34,20 @@ const aliases: readonly HTTPAlias[] = [ 'delete', ]; +const getSignal = (url: string | URL | OptionsInit | undefined, options?: OptionsInit): AbortSignal | undefined => { + let signal; + + if (typeof url === 'object' && 'signal' in (url as OptionsInit)) { + signal = (url as OptionsInit).signal; + } + + if (options?.signal) { + signal = options.signal; + } + + return signal; +}; + const create = (defaults: InstanceDefaults): Got => { defaults = { options: new Options(undefined, undefined, defaults.options), @@ -52,6 +66,8 @@ const create = (defaults: InstanceDefaults): Got => { const request = new Request(url, options, defaultOptions); let promise: CancelableRequest | undefined; + const signal = getSignal(url, options); + const lastHandler = (normalized: Options): GotReturn => { // Note: `options` is `undefined` when `new Options(...)` fails request.options = normalized; @@ -63,7 +79,7 @@ const create = (defaults: InstanceDefaults): Got => { } if (!promise) { - promise = asPromise(request); + promise = asPromise(request, signal); } return promise; @@ -77,7 +93,7 @@ const create = (defaults: InstanceDefaults): Got => { if (is.promise(result) && !request.options.isStream) { if (!promise) { - promise = asPromise(request); + promise = asPromise(request, signal); } if (result !== promise) { diff --git a/test/abort.ts b/test/abort.ts new file mode 100644 index 000000000..a4e0ea1d6 --- /dev/null +++ b/test/abort.ts @@ -0,0 +1,265 @@ +import process from 'process'; +import {EventEmitter} from 'events'; +import stream, {Readable as ReadableStream} from 'stream'; +import test from 'ava'; +import delay from 'delay'; +import {pEvent} from 'p-event'; +import {Handler} from 'express'; +import got from '../source/index.js'; +import slowDataStream from './helpers/slow-data-stream.js'; +import {GlobalClock} from './helpers/types.js'; +import {ExtendedHttpTestServer} from './helpers/create-http-test-server.js'; +import withServer, {withServerAndFakeTimers} from './helpers/with-server.js'; + +const prepareServer = (server: ExtendedHttpTestServer, clock: GlobalClock): {emitter: EventEmitter; promise: Promise} => { + const emitter = new EventEmitter(); + + const promise = new Promise((resolve, reject) => { + server.all('/abort', async (request, response) => { + emitter.emit('connection'); + + request.once('aborted', resolve); + response.once('finish', reject.bind(null, new Error('Request finished instead of aborting.'))); + + try { + await pEvent(request, 'end'); + } catch { + // Node.js 15.0.0 throws AND emits `aborted` + } + + response.end(); + }); + + server.get('/redirect', (_request, response) => { + response.writeHead(302, { + location: `${server.url}/abort`, + }); + response.end(); + + emitter.emit('sentRedirect'); + + clock.tick(3000); + resolve(); + }); + }); + + return {emitter, promise}; +}; + +const downloadHandler = (clock?: GlobalClock): Handler => (_request, response) => { + response.writeHead(200, { + 'transfer-encoding': 'chunked', + }); + + response.flushHeaders(); + + stream.pipeline( + slowDataStream(clock), + response, + () => { + response.end(); + }, + ); +}; + +if (globalThis.AbortController !== undefined) { + test.serial('does not retry after abort', withServerAndFakeTimers, async (t, server, got, clock) => { + const {emitter, promise} = prepareServer(server, clock); + const controller = new AbortController(); + + const gotPromise = got('redirect', { + signal: controller.signal, + retry: { + calculateDelay() { + t.fail('Makes a new try after abort'); + return 0; + }, + }, + }); + + emitter.once('sentRedirect', () => { + controller.abort(); + }); + + await t.throwsAsync(gotPromise, { + instanceOf: DOMException, + }); + + await t.notThrowsAsync(promise, 'Request finished instead of aborting.'); + }); + + test.serial('abort request timeouts', withServer, async (t, server, got) => { + server.get('/', () => {}); + + const controller = new AbortController(); + + const gotPromise = got({ + signal: controller.signal, + timeout: { + request: 10, + }, + retry: { + calculateDelay({computedValue}) { + process.nextTick(() => { + controller.abort(); + }); + + if (computedValue) { + return 20; + } + + return 0; + }, + limit: 1, + }, + }); + + await t.throwsAsync(gotPromise, { + instanceOf: DOMException, + }); + + // Wait for unhandled errors + await delay(40); + }); + + test.serial('aborts in-progress request', withServerAndFakeTimers, async (t, server, got, clock) => { + const {emitter, promise} = prepareServer(server, clock); + + const controller = new AbortController(); + + const body = new ReadableStream({ + read() {}, + }); + body.push('1'); + + const gotPromise = got.post('abort', {body, signal: controller.signal}); + + // Wait for the connection to be established before canceling + emitter.once('connection', () => { + controller.abort(); + body.push(null); + }); + + await t.throwsAsync(gotPromise, { + instanceOf: DOMException, + }); + await t.notThrowsAsync(promise, 'Request finished instead of aborting.'); + }); + + test.serial('aborts in-progress request with timeout', withServerAndFakeTimers, async (t, server, got, clock) => { + const {emitter, promise} = prepareServer(server, clock); + + const controller = new AbortController(); + + const body = new ReadableStream({ + read() {}, + }); + body.push('1'); + + const gotPromise = got.post('abort', {body, timeout: {request: 10_000}, signal: controller.signal}); + + // Wait for the connection to be established before canceling + emitter.once('connection', () => { + controller.abort(); + body.push(null); + }); + + await t.throwsAsync(gotPromise, { + instanceOf: DOMException, + }); + await t.notThrowsAsync(promise, 'Request finished instead of aborting.'); + }); + + test.serial('abort immediately', withServerAndFakeTimers, async (t, server, got, clock) => { + const controller = new AbortController(); + + const promise = new Promise((resolve, reject) => { + // We won't get an abort or even a connection + // We assume no request within 1000ms equals a (client side) aborted request + server.get('/abort', (_request, response) => { + response.once('finish', reject.bind(global, new Error('Request finished instead of aborting.'))); + response.end(); + }); + + clock.tick(1000); + resolve(); + }); + + const gotPromise = got('abort', {signal: controller.signal}); + controller.abort(); + + await t.throwsAsync(gotPromise); + await t.notThrowsAsync(promise, 'Request finished instead of aborting.'); + }); + + test('recover from abort using abortable promise attribute', async t => { + // Canceled before connection started + const controller = new AbortController(); + + const p = got('http://example.com', {signal: controller.signal}); + const recover = p.catch((error: Error) => { + if (controller.signal.aborted) { + return; + } + + throw error; + }); + + controller.abort(); + + await t.notThrowsAsync(recover); + }); + + test('recover from abort using error instance', async t => { + const controller = new AbortController(); + + const p = got('http://example.com', {signal: controller.signal}); + const recover = p.catch((error: Error) => { + if (error instanceof DOMException) { + return; + } + + throw error; + }); + + controller.abort(); + + await t.notThrowsAsync(recover); + }); + + // TODO: Use `fakeTimers` here + test.serial('throws on incomplete (aborted) response', withServer, async (t, server, got) => { + server.get('/', downloadHandler()); + + const controller = new AbortController(); + + const promise = got('', {signal: controller.signal}); + + setTimeout(() => { + controller.abort(); + }, 400); + + await t.throwsAsync(promise, { + instanceOf: DOMException, + }); + }); + + test('throws when aborting cached request', withServer, async (t, server, got) => { + server.get('/', (_request, response) => { + response.setHeader('Cache-Control', 'public, max-age=60'); + response.end(Date.now().toString()); + }); + + const cache = new Map(); + + await got({cache}); + + const controller = new AbortController(); + const promise = got({cache, signal: controller.signal}); + controller.abort(); + + await t.throwsAsync(promise, { + instanceOf: DOMException, + }); + }); +} From 452435aef9f663e8674dc80b7216dcf71f518a5c Mon Sep 17 00:00:00 2001 From: Gyubong Lee Date: Wed, 30 Mar 2022 22:12:50 +0900 Subject: [PATCH 02/19] Fix CI Error --- source/as-promise/index.ts | 17 +++++++++++------ source/core/errors.ts | 12 ++++++++++++ test/abort.ts | 18 ++++++++++-------- 3 files changed, 33 insertions(+), 14 deletions(-) diff --git a/source/as-promise/index.ts b/source/as-promise/index.ts index 8b658bd78..b331aff7f 100644 --- a/source/as-promise/index.ts +++ b/source/as-promise/index.ts @@ -5,6 +5,7 @@ import { RequestError, HTTPError, RetryError, + AbortError, } from '../core/errors.js'; import Request from '../core/index.js'; import {parseBody, isResponseOk} from '../core/response.js'; @@ -22,6 +23,11 @@ const proxiedRequestEvents = [ 'downloadProgress', ]; +// eslint-disable-next-line @typescript-eslint/naming-convention +const getDOMException = (errorMessage: string) => globalThis.DOMException === undefined + ? new AbortError(errorMessage) + : new DOMException(errorMessage); + export default function asPromise(firstRequest?: Request, signal?: AbortSignal): CancelableRequest { let globalRequest: Request; let globalResponse: Response; @@ -33,12 +39,11 @@ export default function asPromise(firstRequest?: Request, signal?: AbortSigna signal.addEventListener('abort', () => { globalRequest.destroy(); - // To do: remove below if statement when targets node is 17.3.0 or higher. - if ((signal as any).reason) { - reject((signal as any).reason); - } else { - reject(new DOMException('This operation was aborted', 'AbortError')); - } + const reason = (signal as any).reason === undefined + ? getDOMException('This operation was aborted.') + : (signal as any).reason; + + reject(reason instanceof Error ? reason : getDOMException(reason)); }); } diff --git a/source/core/errors.ts b/source/core/errors.ts index fc48166f2..b7277d989 100644 --- a/source/core/errors.ts +++ b/source/core/errors.ts @@ -170,3 +170,15 @@ export class RetryError extends RequestError { this.code = 'ERR_RETRYING'; } } + +/** +An error to be thrown when the request is aborted by AbortController. +DOMException is thrown instead of this Error when DOMException is available. +*/ +export class AbortError extends Error { + constructor(message: string) { + super(); + this.name = 'AbortError'; + this.message = message; + } +} diff --git a/test/abort.ts b/test/abort.ts index a4e0ea1d6..1a100b799 100644 --- a/test/abort.ts +++ b/test/abort.ts @@ -82,7 +82,7 @@ if (globalThis.AbortController !== undefined) { }); await t.throwsAsync(gotPromise, { - instanceOf: DOMException, + name: 'AbortError', }); await t.notThrowsAsync(promise, 'Request finished instead of aborting.'); @@ -115,7 +115,7 @@ if (globalThis.AbortController !== undefined) { }); await t.throwsAsync(gotPromise, { - instanceOf: DOMException, + name: 'AbortError', }); // Wait for unhandled errors @@ -141,7 +141,7 @@ if (globalThis.AbortController !== undefined) { }); await t.throwsAsync(gotPromise, { - instanceOf: DOMException, + name: 'AbortError', }); await t.notThrowsAsync(promise, 'Request finished instead of aborting.'); }); @@ -165,7 +165,7 @@ if (globalThis.AbortController !== undefined) { }); await t.throwsAsync(gotPromise, { - instanceOf: DOMException, + name: 'AbortError', }); await t.notThrowsAsync(promise, 'Request finished instead of aborting.'); }); @@ -188,7 +188,9 @@ if (globalThis.AbortController !== undefined) { const gotPromise = got('abort', {signal: controller.signal}); controller.abort(); - await t.throwsAsync(gotPromise); + await t.throwsAsync(gotPromise, { + name: 'AbortError', + }); await t.notThrowsAsync(promise, 'Request finished instead of aborting.'); }); @@ -215,7 +217,7 @@ if (globalThis.AbortController !== undefined) { const p = got('http://example.com', {signal: controller.signal}); const recover = p.catch((error: Error) => { - if (error instanceof DOMException) { + if (error.name === 'AbortError') { return; } @@ -240,7 +242,7 @@ if (globalThis.AbortController !== undefined) { }, 400); await t.throwsAsync(promise, { - instanceOf: DOMException, + name: 'AbortError', }); }); @@ -259,7 +261,7 @@ if (globalThis.AbortController !== undefined) { controller.abort(); await t.throwsAsync(promise, { - instanceOf: DOMException, + name: 'AbortError', }); }); } From 6bdd99af5a070288f24165c81b7c2e3b2ece860f Mon Sep 17 00:00:00 2001 From: Gyubong Lee Date: Wed, 27 Apr 2022 10:44:53 +0900 Subject: [PATCH 03/19] Add initial rejection when already aborted --- source/as-promise/index.ts | 21 +++++++++++++++------ test/abort.ts | 2 +- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/source/as-promise/index.ts b/source/as-promise/index.ts index b331aff7f..5ae67be6c 100644 --- a/source/as-promise/index.ts +++ b/source/as-promise/index.ts @@ -28,6 +28,15 @@ const getDOMException = (errorMessage: string) => globalThis.DOMException === un ? new AbortError(errorMessage) : new DOMException(errorMessage); +const getAbortedReason = (signal: AbortSignal) => { + // To do: Remove below any castings when '@types/node' targets 18 + const reason = (signal as any).reason === undefined + ? getDOMException('This operation was aborted.') + : (signal as any).reason; + + return reason instanceof Error ? reason : getDOMException(reason); +}; + export default function asPromise(firstRequest?: Request, signal?: AbortSignal): CancelableRequest { let globalRequest: Request; let globalResponse: Response; @@ -36,14 +45,14 @@ export default function asPromise(firstRequest?: Request, signal?: AbortSigna const promise = new PCancelable((resolve, reject, onCancel) => { if (signal) { - signal.addEventListener('abort', () => { + if (signal.aborted) { globalRequest.destroy(); + reject(getAbortedReason(signal)); + } - const reason = (signal as any).reason === undefined - ? getDOMException('This operation was aborted.') - : (signal as any).reason; - - reject(reason instanceof Error ? reason : getDOMException(reason)); + signal.addEventListener('abort', () => { + globalRequest.destroy(); + reject(getAbortedReason(signal)); }); } diff --git a/test/abort.ts b/test/abort.ts index 1a100b799..7fef94863 100644 --- a/test/abort.ts +++ b/test/abort.ts @@ -195,7 +195,7 @@ if (globalThis.AbortController !== undefined) { }); test('recover from abort using abortable promise attribute', async t => { - // Canceled before connection started + // Abort before connection started const controller = new AbortController(); const p = got('http://example.com', {signal: controller.signal}); From 945167860552e5a2236af2a8e572108e721540ac Mon Sep 17 00:00:00 2001 From: Szymon Marczak <36894700+szmarczak@users.noreply.github.com> Date: Tue, 3 May 2022 15:27:56 +0200 Subject: [PATCH 04/19] Got stream should be aborted instead --- source/as-promise/index.ts | 29 +---------------------------- source/core/errors.ts | 12 ------------ source/core/index.ts | 4 ++++ source/core/options.ts | 1 - source/create.ts | 20 ++------------------ 5 files changed, 7 insertions(+), 59 deletions(-) diff --git a/source/as-promise/index.ts b/source/as-promise/index.ts index 5ae67be6c..95ce06dd2 100644 --- a/source/as-promise/index.ts +++ b/source/as-promise/index.ts @@ -5,7 +5,6 @@ import { RequestError, HTTPError, RetryError, - AbortError, } from '../core/errors.js'; import Request from '../core/index.js'; import {parseBody, isResponseOk} from '../core/response.js'; @@ -23,39 +22,13 @@ const proxiedRequestEvents = [ 'downloadProgress', ]; -// eslint-disable-next-line @typescript-eslint/naming-convention -const getDOMException = (errorMessage: string) => globalThis.DOMException === undefined - ? new AbortError(errorMessage) - : new DOMException(errorMessage); - -const getAbortedReason = (signal: AbortSignal) => { - // To do: Remove below any castings when '@types/node' targets 18 - const reason = (signal as any).reason === undefined - ? getDOMException('This operation was aborted.') - : (signal as any).reason; - - return reason instanceof Error ? reason : getDOMException(reason); -}; - -export default function asPromise(firstRequest?: Request, signal?: AbortSignal): CancelableRequest { +export default function asPromise(firstRequest?: Request): CancelableRequest { let globalRequest: Request; let globalResponse: Response; let normalizedOptions: Options; const emitter = new EventEmitter(); const promise = new PCancelable((resolve, reject, onCancel) => { - if (signal) { - if (signal.aborted) { - globalRequest.destroy(); - reject(getAbortedReason(signal)); - } - - signal.addEventListener('abort', () => { - globalRequest.destroy(); - reject(getAbortedReason(signal)); - }); - } - onCancel(() => { globalRequest.destroy(); }); diff --git a/source/core/errors.ts b/source/core/errors.ts index b7277d989..fc48166f2 100644 --- a/source/core/errors.ts +++ b/source/core/errors.ts @@ -170,15 +170,3 @@ export class RetryError extends RequestError { this.code = 'ERR_RETRYING'; } } - -/** -An error to be thrown when the request is aborted by AbortController. -DOMException is thrown instead of this Error when DOMException is available. -*/ -export class AbortError extends Error { - constructor(message: string) { - super(); - this.name = 'AbortError'; - this.message = message; - } -} diff --git a/source/core/index.ts b/source/core/index.ts index 1d913c265..3e1a35114 100644 --- a/source/core/index.ts +++ b/source/core/index.ts @@ -235,6 +235,10 @@ export default class Request extends Duplex implements RequestEvents { return; } + this.options.signal?.addEventListener('abort', () => { + this.destroy(new Error('This operation was aborted.')); + }); + // Important! If you replace `body` in a handler with another stream, make sure it's readable first. // The below is run only once. const {body} = this.options; diff --git a/source/core/options.ts b/source/core/options.ts index 65655844c..989c9df57 100644 --- a/source/core/options.ts +++ b/source/core/options.ts @@ -2441,7 +2441,6 @@ export default class Options { headers: internals.headers, createConnection: internals.createConnection, timeout: internals.http2 ? getHttp2TimeoutOption(internals) : undefined, - signal: internals.signal, // HTTP/2 options h2session: internals.h2session, diff --git a/source/create.ts b/source/create.ts index 584a57725..376fda30f 100644 --- a/source/create.ts +++ b/source/create.ts @@ -34,20 +34,6 @@ const aliases: readonly HTTPAlias[] = [ 'delete', ]; -const getSignal = (url: string | URL | OptionsInit | undefined, options?: OptionsInit): AbortSignal | undefined => { - let signal; - - if (typeof url === 'object' && 'signal' in (url as OptionsInit)) { - signal = (url as OptionsInit).signal; - } - - if (options?.signal) { - signal = options.signal; - } - - return signal; -}; - const create = (defaults: InstanceDefaults): Got => { defaults = { options: new Options(undefined, undefined, defaults.options), @@ -66,8 +52,6 @@ const create = (defaults: InstanceDefaults): Got => { const request = new Request(url, options, defaultOptions); let promise: CancelableRequest | undefined; - const signal = getSignal(url, options); - const lastHandler = (normalized: Options): GotReturn => { // Note: `options` is `undefined` when `new Options(...)` fails request.options = normalized; @@ -79,7 +63,7 @@ const create = (defaults: InstanceDefaults): Got => { } if (!promise) { - promise = asPromise(request, signal); + promise = asPromise(request); } return promise; @@ -93,7 +77,7 @@ const create = (defaults: InstanceDefaults): Got => { if (is.promise(result) && !request.options.isStream) { if (!promise) { - promise = asPromise(request, signal); + promise = asPromise(request); } if (result !== promise) { From 723b96030f245837dc37dff2ed878bca274a5f6e Mon Sep 17 00:00:00 2001 From: Gyubong Lee Date: Thu, 12 May 2022 12:19:43 +0900 Subject: [PATCH 05/19] Restore logic and fix tests --- source/as-promise/index.ts | 27 ++++++++++++++++++++++++++- source/core/errors.ts | 12 ++++++++++++ source/core/index.ts | 3 ++- source/create.ts | 20 ++++++++++++++++++-- 4 files changed, 58 insertions(+), 4 deletions(-) diff --git a/source/as-promise/index.ts b/source/as-promise/index.ts index 95ce06dd2..de236e544 100644 --- a/source/as-promise/index.ts +++ b/source/as-promise/index.ts @@ -5,6 +5,7 @@ import { RequestError, HTTPError, RetryError, + AbortError, } from '../core/errors.js'; import Request from '../core/index.js'; import {parseBody, isResponseOk} from '../core/response.js'; @@ -22,13 +23,37 @@ const proxiedRequestEvents = [ 'downloadProgress', ]; -export default function asPromise(firstRequest?: Request): CancelableRequest { +// eslint-disable-next-line @typescript-eslint/naming-convention +const getDOMException = (errorMessage: string) => globalThis.DOMException === undefined + ? new AbortError(errorMessage) + : new DOMException(errorMessage); + +const getAbortedReason = (signal: AbortSignal) => { + // To do: Remove below any castings when '@types/node' targets 18 + const reason = (signal as any).reason === undefined + ? getDOMException('This operation was aborted.') + : (signal as any).reason; + + return reason instanceof Error ? reason : getDOMException(reason); +}; + +export default function asPromise(firstRequest?: Request, signal?: AbortSignal): CancelableRequest { let globalRequest: Request; let globalResponse: Response; let normalizedOptions: Options; const emitter = new EventEmitter(); const promise = new PCancelable((resolve, reject, onCancel) => { + if (signal) { + if (signal.aborted) { + reject(getAbortedReason(signal)); + } + + signal.addEventListener('abort', () => { + reject(getAbortedReason(signal)); + }); + } + onCancel(() => { globalRequest.destroy(); }); diff --git a/source/core/errors.ts b/source/core/errors.ts index fc48166f2..b7277d989 100644 --- a/source/core/errors.ts +++ b/source/core/errors.ts @@ -170,3 +170,15 @@ export class RetryError extends RequestError { this.code = 'ERR_RETRYING'; } } + +/** +An error to be thrown when the request is aborted by AbortController. +DOMException is thrown instead of this Error when DOMException is available. +*/ +export class AbortError extends Error { + constructor(message: string) { + super(); + this.name = 'AbortError'; + this.message = message; + } +} diff --git a/source/core/index.ts b/source/core/index.ts index 3e1a35114..972401958 100644 --- a/source/core/index.ts +++ b/source/core/index.ts @@ -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'; @@ -236,7 +237,7 @@ export default class Request extends Duplex implements RequestEvents { } this.options.signal?.addEventListener('abort', () => { - this.destroy(new Error('This operation was aborted.')); + this.destroy(new AbortError('This operation was aborted.')); }); // Important! If you replace `body` in a handler with another stream, make sure it's readable first. diff --git a/source/create.ts b/source/create.ts index 376fda30f..f5386e00a 100644 --- a/source/create.ts +++ b/source/create.ts @@ -25,6 +25,20 @@ const delay = async (ms: number) => new Promise(resolve => { const isGotInstance = (value: Got | ExtendOptions): value is Got => is.function_(value); +const getSignal = (url: string | URL | OptionsInit | undefined, options?: OptionsInit): AbortSignal | undefined => { + let signal; + + if (typeof url === 'object' && 'signal' in (url as OptionsInit)) { + signal = (url as OptionsInit).signal; + } + + if (options?.signal) { + signal = options.signal; + } + + return signal; +}; + const aliases: readonly HTTPAlias[] = [ 'get', 'post', @@ -52,6 +66,8 @@ const create = (defaults: InstanceDefaults): Got => { const request = new Request(url, options, defaultOptions); let promise: CancelableRequest | undefined; + const signal = getSignal(url, options); + const lastHandler = (normalized: Options): GotReturn => { // Note: `options` is `undefined` when `new Options(...)` fails request.options = normalized; @@ -63,7 +79,7 @@ const create = (defaults: InstanceDefaults): Got => { } if (!promise) { - promise = asPromise(request); + promise = asPromise(request, signal); } return promise; @@ -77,7 +93,7 @@ const create = (defaults: InstanceDefaults): Got => { if (is.promise(result) && !request.options.isStream) { if (!promise) { - promise = asPromise(request); + promise = asPromise(request, signal); } if (result !== promise) { From 63f7b08ff9636cb2f1bccc5e3429ef5299d92957 Mon Sep 17 00:00:00 2001 From: Gyubong Lee Date: Fri, 13 May 2022 10:47:32 +0900 Subject: [PATCH 06/19] Revert "Restore logic and fix tests" This reverts commit 723b96030f245837dc37dff2ed878bca274a5f6e. --- source/as-promise/index.ts | 27 +-------------------------- source/core/errors.ts | 12 ------------ source/core/index.ts | 3 +-- source/create.ts | 20 ++------------------ 4 files changed, 4 insertions(+), 58 deletions(-) diff --git a/source/as-promise/index.ts b/source/as-promise/index.ts index de236e544..95ce06dd2 100644 --- a/source/as-promise/index.ts +++ b/source/as-promise/index.ts @@ -5,7 +5,6 @@ import { RequestError, HTTPError, RetryError, - AbortError, } from '../core/errors.js'; import Request from '../core/index.js'; import {parseBody, isResponseOk} from '../core/response.js'; @@ -23,37 +22,13 @@ const proxiedRequestEvents = [ 'downloadProgress', ]; -// eslint-disable-next-line @typescript-eslint/naming-convention -const getDOMException = (errorMessage: string) => globalThis.DOMException === undefined - ? new AbortError(errorMessage) - : new DOMException(errorMessage); - -const getAbortedReason = (signal: AbortSignal) => { - // To do: Remove below any castings when '@types/node' targets 18 - const reason = (signal as any).reason === undefined - ? getDOMException('This operation was aborted.') - : (signal as any).reason; - - return reason instanceof Error ? reason : getDOMException(reason); -}; - -export default function asPromise(firstRequest?: Request, signal?: AbortSignal): CancelableRequest { +export default function asPromise(firstRequest?: Request): CancelableRequest { let globalRequest: Request; let globalResponse: Response; let normalizedOptions: Options; const emitter = new EventEmitter(); const promise = new PCancelable((resolve, reject, onCancel) => { - if (signal) { - if (signal.aborted) { - reject(getAbortedReason(signal)); - } - - signal.addEventListener('abort', () => { - reject(getAbortedReason(signal)); - }); - } - onCancel(() => { globalRequest.destroy(); }); diff --git a/source/core/errors.ts b/source/core/errors.ts index b7277d989..fc48166f2 100644 --- a/source/core/errors.ts +++ b/source/core/errors.ts @@ -170,15 +170,3 @@ export class RetryError extends RequestError { this.code = 'ERR_RETRYING'; } } - -/** -An error to be thrown when the request is aborted by AbortController. -DOMException is thrown instead of this Error when DOMException is available. -*/ -export class AbortError extends Error { - constructor(message: string) { - super(); - this.name = 'AbortError'; - this.message = message; - } -} diff --git a/source/core/index.ts b/source/core/index.ts index 972401958..3e1a35114 100644 --- a/source/core/index.ts +++ b/source/core/index.ts @@ -31,7 +31,6 @@ import { TimeoutError, UploadError, CacheError, - AbortError, } from './errors.js'; import type {PlainResponse} from './response.js'; import type {PromiseCookieJar, NativeRequestOptions, RetryOptions} from './options.js'; @@ -237,7 +236,7 @@ export default class Request extends Duplex implements RequestEvents { } this.options.signal?.addEventListener('abort', () => { - this.destroy(new AbortError('This operation was aborted.')); + this.destroy(new Error('This operation was aborted.')); }); // Important! If you replace `body` in a handler with another stream, make sure it's readable first. diff --git a/source/create.ts b/source/create.ts index f5386e00a..376fda30f 100644 --- a/source/create.ts +++ b/source/create.ts @@ -25,20 +25,6 @@ const delay = async (ms: number) => new Promise(resolve => { const isGotInstance = (value: Got | ExtendOptions): value is Got => is.function_(value); -const getSignal = (url: string | URL | OptionsInit | undefined, options?: OptionsInit): AbortSignal | undefined => { - let signal; - - if (typeof url === 'object' && 'signal' in (url as OptionsInit)) { - signal = (url as OptionsInit).signal; - } - - if (options?.signal) { - signal = options.signal; - } - - return signal; -}; - const aliases: readonly HTTPAlias[] = [ 'get', 'post', @@ -66,8 +52,6 @@ const create = (defaults: InstanceDefaults): Got => { const request = new Request(url, options, defaultOptions); let promise: CancelableRequest | undefined; - const signal = getSignal(url, options); - const lastHandler = (normalized: Options): GotReturn => { // Note: `options` is `undefined` when `new Options(...)` fails request.options = normalized; @@ -79,7 +63,7 @@ const create = (defaults: InstanceDefaults): Got => { } if (!promise) { - promise = asPromise(request, signal); + promise = asPromise(request); } return promise; @@ -93,7 +77,7 @@ const create = (defaults: InstanceDefaults): Got => { if (is.promise(result) && !request.options.isStream) { if (!promise) { - promise = asPromise(request, signal); + promise = asPromise(request); } if (result !== promise) { From b18f1fe29958c160a06dc3f534d87c0b48ac0e05 Mon Sep 17 00:00:00 2001 From: Gyubong Lee Date: Fri, 13 May 2022 12:00:23 +0900 Subject: [PATCH 07/19] Simplify the codes --- source/as-promise/index.ts | 6 ++++++ source/core/errors.ts | 13 +++++++++++++ source/core/index.ts | 3 ++- 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/source/as-promise/index.ts b/source/as-promise/index.ts index 95ce06dd2..769cd16e7 100644 --- a/source/as-promise/index.ts +++ b/source/as-promise/index.ts @@ -29,6 +29,12 @@ export default function asPromise(firstRequest?: Request): CancelableRequest< const emitter = new EventEmitter(); const promise = new PCancelable((resolve, reject, onCancel) => { + firstRequest?.on('error', (error: RequestError) => { + if (error.code === 'ERR_GOR_ABORT_ERROR') { + reject(error); + } + }); + onCancel(() => { globalRequest.destroy(); }); diff --git a/source/core/errors.ts b/source/core/errors.ts index fc48166f2..ff347a1a3 100644 --- a/source/core/errors.ts +++ b/source/core/errors.ts @@ -170,3 +170,16 @@ export class RetryError extends RequestError { this.code = 'ERR_RETRYING'; } } + +/** +An error to be thrown when the request is aborted by AbortController. +DOMException is thrown instead of this Error when DOMException is available. +*/ +export class AbortError extends RequestError { + constructor(request: Request, message: string) { + super(message, {}, request); + this.name = 'AbortError'; + this.code = 'ERR_GOR_ABORT_ERROR'; + this.message = message; + } +} diff --git a/source/core/index.ts b/source/core/index.ts index 3e1a35114..c011e0d20 100644 --- a/source/core/index.ts +++ b/source/core/index.ts @@ -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'; @@ -236,7 +237,7 @@ export default class Request extends Duplex implements RequestEvents { } this.options.signal?.addEventListener('abort', () => { - this.destroy(new Error('This operation was aborted.')); + this.destroy(new AbortError(this, 'This operation was aborted.')); }); // Important! If you replace `body` in a handler with another stream, make sure it's readable first. From f5a877bf8738b0d16ef9725b72e2e425ef5a4ab1 Mon Sep 17 00:00:00 2001 From: Gyubong Lee Date: Fri, 13 May 2022 12:06:33 +0900 Subject: [PATCH 08/19] Remove improper comment --- source/core/errors.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/source/core/errors.ts b/source/core/errors.ts index ff347a1a3..0376acfe3 100644 --- a/source/core/errors.ts +++ b/source/core/errors.ts @@ -173,7 +173,6 @@ export class RetryError extends RequestError { /** An error to be thrown when the request is aborted by AbortController. -DOMException is thrown instead of this Error when DOMException is available. */ export class AbortError extends RequestError { constructor(request: Request, message: string) { From 4ef257aa556c84bd2560d580e318ef337a60d175 Mon Sep 17 00:00:00 2001 From: Gyubong Lee Date: Fri, 13 May 2022 12:17:26 +0900 Subject: [PATCH 09/19] Move if statement's position to top --- test/abort.ts | 78 +++++++++++++++++++++++++-------------------------- 1 file changed, 39 insertions(+), 39 deletions(-) diff --git a/test/abort.ts b/test/abort.ts index 7fef94863..f61a2078f 100644 --- a/test/abort.ts +++ b/test/abort.ts @@ -11,58 +11,58 @@ import {GlobalClock} from './helpers/types.js'; import {ExtendedHttpTestServer} from './helpers/create-http-test-server.js'; import withServer, {withServerAndFakeTimers} from './helpers/with-server.js'; -const prepareServer = (server: ExtendedHttpTestServer, clock: GlobalClock): {emitter: EventEmitter; promise: Promise} => { - const emitter = new EventEmitter(); - - const promise = new Promise((resolve, reject) => { - server.all('/abort', async (request, response) => { - emitter.emit('connection'); +if (globalThis.AbortController !== undefined) { + const prepareServer = (server: ExtendedHttpTestServer, clock: GlobalClock): {emitter: EventEmitter; promise: Promise} => { + const emitter = new EventEmitter(); - request.once('aborted', resolve); - response.once('finish', reject.bind(null, new Error('Request finished instead of aborting.'))); + const promise = new Promise((resolve, reject) => { + server.all('/abort', async (request, response) => { + emitter.emit('connection'); - try { - await pEvent(request, 'end'); - } catch { - // Node.js 15.0.0 throws AND emits `aborted` - } + request.once('aborted', resolve); + response.once('finish', reject.bind(null, new Error('Request finished instead of aborting.'))); - response.end(); - }); + try { + await pEvent(request, 'end'); + } catch { + // Node.js 15.0.0 throws AND emits `aborted` + } - server.get('/redirect', (_request, response) => { - response.writeHead(302, { - location: `${server.url}/abort`, + response.end(); }); - response.end(); - emitter.emit('sentRedirect'); + server.get('/redirect', (_request, response) => { + response.writeHead(302, { + location: `${server.url}/abort`, + }); + response.end(); - clock.tick(3000); - resolve(); + emitter.emit('sentRedirect'); + + clock.tick(3000); + resolve(); + }); }); - }); - return {emitter, promise}; -}; + return {emitter, promise}; + }; -const downloadHandler = (clock?: GlobalClock): Handler => (_request, response) => { - response.writeHead(200, { - 'transfer-encoding': 'chunked', - }); + const downloadHandler = (clock?: GlobalClock): Handler => (_request, response) => { + response.writeHead(200, { + 'transfer-encoding': 'chunked', + }); - response.flushHeaders(); + response.flushHeaders(); - stream.pipeline( - slowDataStream(clock), - response, - () => { - response.end(); - }, - ); -}; + stream.pipeline( + slowDataStream(clock), + response, + () => { + response.end(); + }, + ); + }; -if (globalThis.AbortController !== undefined) { test.serial('does not retry after abort', withServerAndFakeTimers, async (t, server, got, clock) => { const {emitter, promise} = prepareServer(server, clock); const controller = new AbortController(); From f7d5cd5aeda697ae46bf5d03defb811c94d4bfa2 Mon Sep 17 00:00:00 2001 From: Gyubong Lee Date: Sat, 14 May 2022 15:43:10 +0900 Subject: [PATCH 10/19] Revert "Remove improper comment" This reverts commit f5a877bf8738b0d16ef9725b72e2e425ef5a4ab1. --- source/core/errors.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/source/core/errors.ts b/source/core/errors.ts index 0376acfe3..ff347a1a3 100644 --- a/source/core/errors.ts +++ b/source/core/errors.ts @@ -173,6 +173,7 @@ export class RetryError extends RequestError { /** An error to be thrown when the request is aborted by AbortController. +DOMException is thrown instead of this Error when DOMException is available. */ export class AbortError extends RequestError { constructor(request: Request, message: string) { From b65e512e06a07a5233b54cd34e4fc1ca3467fdcd Mon Sep 17 00:00:00 2001 From: Gyubong Lee Date: Sat, 14 May 2022 15:43:12 +0900 Subject: [PATCH 11/19] Revert "Simplify the codes" This reverts commit b18f1fe29958c160a06dc3f534d87c0b48ac0e05. --- source/as-promise/index.ts | 6 ------ source/core/errors.ts | 13 ------------- source/core/index.ts | 3 +-- 3 files changed, 1 insertion(+), 21 deletions(-) diff --git a/source/as-promise/index.ts b/source/as-promise/index.ts index 769cd16e7..95ce06dd2 100644 --- a/source/as-promise/index.ts +++ b/source/as-promise/index.ts @@ -29,12 +29,6 @@ export default function asPromise(firstRequest?: Request): CancelableRequest< const emitter = new EventEmitter(); const promise = new PCancelable((resolve, reject, onCancel) => { - firstRequest?.on('error', (error: RequestError) => { - if (error.code === 'ERR_GOR_ABORT_ERROR') { - reject(error); - } - }); - onCancel(() => { globalRequest.destroy(); }); diff --git a/source/core/errors.ts b/source/core/errors.ts index ff347a1a3..fc48166f2 100644 --- a/source/core/errors.ts +++ b/source/core/errors.ts @@ -170,16 +170,3 @@ export class RetryError extends RequestError { this.code = 'ERR_RETRYING'; } } - -/** -An error to be thrown when the request is aborted by AbortController. -DOMException is thrown instead of this Error when DOMException is available. -*/ -export class AbortError extends RequestError { - constructor(request: Request, message: string) { - super(message, {}, request); - this.name = 'AbortError'; - this.code = 'ERR_GOR_ABORT_ERROR'; - this.message = message; - } -} diff --git a/source/core/index.ts b/source/core/index.ts index c011e0d20..3e1a35114 100644 --- a/source/core/index.ts +++ b/source/core/index.ts @@ -31,7 +31,6 @@ import { TimeoutError, UploadError, CacheError, - AbortError, } from './errors.js'; import type {PlainResponse} from './response.js'; import type {PromiseCookieJar, NativeRequestOptions, RetryOptions} from './options.js'; @@ -237,7 +236,7 @@ export default class Request extends Duplex implements RequestEvents { } this.options.signal?.addEventListener('abort', () => { - this.destroy(new AbortError(this, 'This operation was aborted.')); + this.destroy(new Error('This operation was aborted.')); }); // Important! If you replace `body` in a handler with another stream, make sure it's readable first. From 94dacf7ff9fac29c7da52d28b22ed5e4504942c8 Mon Sep 17 00:00:00 2001 From: Gyubong Lee Date: Sat, 14 May 2022 15:56:41 +0900 Subject: [PATCH 12/19] Fix tests --- test/abort.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/test/abort.ts b/test/abort.ts index f61a2078f..3eb798a56 100644 --- a/test/abort.ts +++ b/test/abort.ts @@ -82,7 +82,7 @@ if (globalThis.AbortController !== undefined) { }); await t.throwsAsync(gotPromise, { - name: 'AbortError', + message: 'This operation was aborted.', }); await t.notThrowsAsync(promise, 'Request finished instead of aborting.'); @@ -115,7 +115,7 @@ if (globalThis.AbortController !== undefined) { }); await t.throwsAsync(gotPromise, { - name: 'AbortError', + message: 'This operation was aborted.', }); // Wait for unhandled errors @@ -141,7 +141,7 @@ if (globalThis.AbortController !== undefined) { }); await t.throwsAsync(gotPromise, { - name: 'AbortError', + message: 'This operation was aborted.', }); await t.notThrowsAsync(promise, 'Request finished instead of aborting.'); }); @@ -165,7 +165,7 @@ if (globalThis.AbortController !== undefined) { }); await t.throwsAsync(gotPromise, { - name: 'AbortError', + message: 'This operation was aborted.', }); await t.notThrowsAsync(promise, 'Request finished instead of aborting.'); }); @@ -189,7 +189,7 @@ if (globalThis.AbortController !== undefined) { controller.abort(); await t.throwsAsync(gotPromise, { - name: 'AbortError', + message: 'This operation was aborted.', }); await t.notThrowsAsync(promise, 'Request finished instead of aborting.'); }); @@ -217,7 +217,7 @@ if (globalThis.AbortController !== undefined) { const p = got('http://example.com', {signal: controller.signal}); const recover = p.catch((error: Error) => { - if (error.name === 'AbortError') { + if (error.message === 'This operation was aborted.') { return; } @@ -242,7 +242,7 @@ if (globalThis.AbortController !== undefined) { }, 400); await t.throwsAsync(promise, { - name: 'AbortError', + message: 'This operation was aborted.', }); }); @@ -261,7 +261,7 @@ if (globalThis.AbortController !== undefined) { controller.abort(); await t.throwsAsync(promise, { - name: 'AbortError', + message: 'This operation was aborted.', }); }); } From 0e6f53999fd6e6c2c1ffd398f0f065690dff2d2b Mon Sep 17 00:00:00 2001 From: Gyubong Lee Date: Mon, 11 Jul 2022 20:25:14 +0900 Subject: [PATCH 13/19] Add `AbortError` --- documentation/8-errors.md | 6 ++++++ source/core/errors.ts | 10 ++++++++++ source/core/index.ts | 7 ++++++- source/core/options.ts | 2 +- test/abort.ts | 7 +++++++ 5 files changed, 30 insertions(+), 2 deletions(-) diff --git a/documentation/8-errors.md b/documentation/8-errors.md index e32f44283..2cb97aa7d 100644 --- a/documentation/8-errors.md +++ b/documentation/8-errors.md @@ -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). diff --git a/source/core/errors.ts b/source/core/errors.ts index fc48166f2..56c19e68f 100644 --- a/source/core/errors.ts +++ b/source/core/errors.ts @@ -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); + this.name = 'AbortError'; + } +} diff --git a/source/core/index.ts b/source/core/index.ts index 3e1a35114..871a44186 100644 --- a/source/core/index.ts +++ b/source/core/index.ts @@ -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'; @@ -235,8 +236,12 @@ export default class Request extends Duplex implements RequestEvents { 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. diff --git a/source/core/options.ts b/source/core/options.ts index 989c9df57..7dc5ada1b 100644 --- a/source/core/options.ts +++ b/source/core/options.ts @@ -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.__ @example ``` diff --git a/test/abort.ts b/test/abort.ts index 3eb798a56..3312b090f 100644 --- a/test/abort.ts +++ b/test/abort.ts @@ -82,6 +82,7 @@ if (globalThis.AbortController !== undefined) { }); await t.throwsAsync(gotPromise, { + code: 'ERR_ABORTED', message: 'This operation was aborted.', }); @@ -115,6 +116,7 @@ if (globalThis.AbortController !== undefined) { }); await t.throwsAsync(gotPromise, { + code: 'ERR_ABORTED', message: 'This operation was aborted.', }); @@ -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.'); @@ -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.'); @@ -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.'); @@ -242,6 +247,7 @@ if (globalThis.AbortController !== undefined) { }, 400); await t.throwsAsync(promise, { + code: 'ERR_ABORTED', message: 'This operation was aborted.', }); }); @@ -261,6 +267,7 @@ if (globalThis.AbortController !== undefined) { controller.abort(); await t.throwsAsync(promise, { + code: 'ERR_ABORTED', message: 'This operation was aborted.', }); }); From 83aef439171dad9a67f2c3bf614cbd690d03a0b2 Mon Sep 17 00:00:00 2001 From: Gyubong Lee Date: Tue, 12 Jul 2022 12:23:29 +0900 Subject: [PATCH 14/19] Implement throwing native DOMException when possible --- documentation/8-errors.md | 2 -- source/core/errors.ts | 10 ---------- source/core/index.ts | 24 +++++++++++++++++++++--- test/abort.ts | 7 ------- 4 files changed, 21 insertions(+), 22 deletions(-) diff --git a/documentation/8-errors.md b/documentation/8-errors.md index 2cb97aa7d..f938995bb 100644 --- a/documentation/8-errors.md +++ b/documentation/8-errors.md @@ -100,6 +100,4 @@ 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). diff --git a/source/core/errors.ts b/source/core/errors.ts index 56c19e68f..fc48166f2 100644 --- a/source/core/errors.ts +++ b/source/core/errors.ts @@ -170,13 +170,3 @@ 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); - this.name = 'AbortError'; - } -} diff --git a/source/core/index.ts b/source/core/index.ts index 871a44186..cb06e7907 100644 --- a/source/core/index.ts +++ b/source/core/index.ts @@ -31,7 +31,6 @@ import { TimeoutError, UploadError, CacheError, - AbortError, } from './errors.js'; import type {PlainResponse} from './response.js'; import type {PromiseCookieJar, NativeRequestOptions, RetryOptions} from './options.js'; @@ -137,6 +136,25 @@ type UrlType = ConstructorParameters[0]; type OptionsType = ConstructorParameters[1]; type DefaultsType = ConstructorParameters[2]; +/** +TODO: Remove AbortError and just throw DOMException when targeting Node 18. +*/ +// eslint-disable-next-line @typescript-eslint/naming-convention +const getDOMException = (errorMessage: string) => globalThis.DOMException === undefined + ? new Error(errorMessage) + : new DOMException(errorMessage); + +/** +TODO: Remove below function and just 'reject(signal.reason)' when targeting Node 18. +*/ +const getAbortedReason = (signal: AbortSignal) => { + const reason = (signal as any).reason === undefined + ? getDOMException('This operation was aborted.') + : (signal as any).reason; + + return reason instanceof Error ? reason : getDOMException(reason); +}; + export default class Request extends Duplex implements RequestEvents { override ['constructor']: typeof Request; @@ -237,11 +255,11 @@ export default class Request extends Duplex implements RequestEvents { } if (this.options.signal?.aborted) { - this.destroy(new AbortError(this, (this.options.signal as any).reason)); + this.destroy(getAbortedReason(this.options.signal)); } this.options.signal?.addEventListener('abort', () => { - this.destroy(new AbortError(this, (this.options.signal as any).reason)); + this.destroy(getAbortedReason(this.options.signal!)); }); // Important! If you replace `body` in a handler with another stream, make sure it's readable first. diff --git a/test/abort.ts b/test/abort.ts index 3312b090f..3eb798a56 100644 --- a/test/abort.ts +++ b/test/abort.ts @@ -82,7 +82,6 @@ if (globalThis.AbortController !== undefined) { }); await t.throwsAsync(gotPromise, { - code: 'ERR_ABORTED', message: 'This operation was aborted.', }); @@ -116,7 +115,6 @@ if (globalThis.AbortController !== undefined) { }); await t.throwsAsync(gotPromise, { - code: 'ERR_ABORTED', message: 'This operation was aborted.', }); @@ -143,7 +141,6 @@ 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.'); @@ -168,7 +165,6 @@ 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.'); @@ -193,7 +189,6 @@ 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.'); @@ -247,7 +242,6 @@ if (globalThis.AbortController !== undefined) { }, 400); await t.throwsAsync(promise, { - code: 'ERR_ABORTED', message: 'This operation was aborted.', }); }); @@ -267,7 +261,6 @@ if (globalThis.AbortController !== undefined) { controller.abort(); await t.throwsAsync(promise, { - code: 'ERR_ABORTED', message: 'This operation was aborted.', }); }); From 5e8a6ec47ef25727bc4aee79989939147e20474f Mon Sep 17 00:00:00 2001 From: Gyubong Lee Date: Fri, 22 Jul 2022 12:22:07 +0900 Subject: [PATCH 15/19] Revert "Implement throwing native DOMException when possible" This reverts commit 83aef439171dad9a67f2c3bf614cbd690d03a0b2. --- documentation/8-errors.md | 2 ++ source/core/errors.ts | 10 ++++++++++ source/core/index.ts | 24 +++--------------------- test/abort.ts | 7 +++++++ 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/documentation/8-errors.md b/documentation/8-errors.md index f938995bb..2cb97aa7d 100644 --- a/documentation/8-errors.md +++ b/documentation/8-errors.md @@ -100,4 +100,6 @@ 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). diff --git a/source/core/errors.ts b/source/core/errors.ts index fc48166f2..56c19e68f 100644 --- a/source/core/errors.ts +++ b/source/core/errors.ts @@ -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); + this.name = 'AbortError'; + } +} diff --git a/source/core/index.ts b/source/core/index.ts index cb06e7907..871a44186 100644 --- a/source/core/index.ts +++ b/source/core/index.ts @@ -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'; @@ -136,25 +137,6 @@ type UrlType = ConstructorParameters[0]; type OptionsType = ConstructorParameters[1]; type DefaultsType = ConstructorParameters[2]; -/** -TODO: Remove AbortError and just throw DOMException when targeting Node 18. -*/ -// eslint-disable-next-line @typescript-eslint/naming-convention -const getDOMException = (errorMessage: string) => globalThis.DOMException === undefined - ? new Error(errorMessage) - : new DOMException(errorMessage); - -/** -TODO: Remove below function and just 'reject(signal.reason)' when targeting Node 18. -*/ -const getAbortedReason = (signal: AbortSignal) => { - const reason = (signal as any).reason === undefined - ? getDOMException('This operation was aborted.') - : (signal as any).reason; - - return reason instanceof Error ? reason : getDOMException(reason); -}; - export default class Request extends Duplex implements RequestEvents { override ['constructor']: typeof Request; @@ -255,11 +237,11 @@ export default class Request extends Duplex implements RequestEvents { } if (this.options.signal?.aborted) { - this.destroy(getAbortedReason(this.options.signal)); + this.destroy(new AbortError(this, (this.options.signal as any).reason)); } this.options.signal?.addEventListener('abort', () => { - this.destroy(getAbortedReason(this.options.signal!)); + 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. diff --git a/test/abort.ts b/test/abort.ts index 3eb798a56..3312b090f 100644 --- a/test/abort.ts +++ b/test/abort.ts @@ -82,6 +82,7 @@ if (globalThis.AbortController !== undefined) { }); await t.throwsAsync(gotPromise, { + code: 'ERR_ABORTED', message: 'This operation was aborted.', }); @@ -115,6 +116,7 @@ if (globalThis.AbortController !== undefined) { }); await t.throwsAsync(gotPromise, { + code: 'ERR_ABORTED', message: 'This operation was aborted.', }); @@ -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.'); @@ -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.'); @@ -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.'); @@ -242,6 +247,7 @@ if (globalThis.AbortController !== undefined) { }, 400); await t.throwsAsync(promise, { + code: 'ERR_ABORTED', message: 'This operation was aborted.', }); }); @@ -261,6 +267,7 @@ if (globalThis.AbortController !== undefined) { controller.abort(); await t.throwsAsync(promise, { + code: 'ERR_ABORTED', message: 'This operation was aborted.', }); }); From cddec2da3ca2d375a56808e4e5cabd000d1d8139 Mon Sep 17 00:00:00 2001 From: Gyubong Lee Date: Fri, 22 Jul 2022 12:32:07 +0900 Subject: [PATCH 16/19] Revert comment style --- source/core/options.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/core/options.ts b/source/core/options.ts index 7dc5ada1b..989c9df57 100644 --- a/source/core/options.ts +++ b/source/core/options.ts @@ -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.* @example ``` From 67d12d4c8b5b6188fdaed2740a5c44d11e7c3c4e Mon Sep 17 00:00:00 2001 From: Szymon Marczak <36894700+szmarczak@users.noreply.github.com> Date: Fri, 22 Jul 2022 16:18:08 +0200 Subject: [PATCH 17/19] Remove unnecessary asterisk from a comment --- source/core/options.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/core/options.ts b/source/core/options.ts index 989c9df57..423287643 100644 --- a/source/core/options.ts +++ b/source/core/options.ts @@ -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.* @example ``` From 7406560bc0fa98e6e1fc14a6218fb7ace9d81a6c Mon Sep 17 00:00:00 2001 From: Gyubong Lee Date: Sat, 23 Jul 2022 14:05:03 +0900 Subject: [PATCH 18/19] Reflect feedbacks --- source/core/errors.ts | 5 +++-- source/core/index.ts | 4 ++-- source/core/options.ts | 6 ++++-- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/source/core/errors.ts b/source/core/errors.ts index 56c19e68f..d344517da 100644 --- a/source/core/errors.ts +++ b/source/core/errors.ts @@ -175,8 +175,9 @@ export class RetryError extends RequestError { 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); + constructor(request: Request) { + super('This operation was aborted.', {}, request); + this.code = 'ERR_ABORTED'; this.name = 'AbortError'; } } diff --git a/source/core/index.ts b/source/core/index.ts index 987fa7eec..01a12128e 100644 --- a/source/core/index.ts +++ b/source/core/index.ts @@ -243,11 +243,11 @@ export default class Request extends Duplex implements RequestEvents { } if (this.options.signal?.aborted) { - this.destroy(new AbortError(this, (this.options.signal as any).reason)); + this.destroy(new AbortError(this)); } this.options.signal?.addEventListener('abort', () => { - this.destroy(new AbortError(this, (this.options.signal as any).reason)); + this.destroy(new AbortError(this)); }); // Important! If you replace `body` in a handler with another stream, make sure it's readable first. diff --git a/source/core/options.ts b/source/core/options.ts index 8ddf97150..38191f38e 100644 --- a/source/core/options.ts +++ b/source/core/options.ts @@ -1510,11 +1510,13 @@ export default class Options { }, 100); ``` */ - get signal(): AbortSignal | undefined { + // TODO: Replace `any` with `AbortSignal` when targeting Node 16. + get signal(): any | undefined { return this._internals.signal; } - set signal(value: AbortSignal | undefined) { + // TODO: Replace `any` with `AbortSignal` when targeting Node 16. + set signal(value: any | undefined) { assert.object(value); this._internals.signal = value; From 94b5ed067cdcacbb1ad330537ec47626fb7c83aa Mon Sep 17 00:00:00 2001 From: Sindre Sorhus Date: Sun, 24 Jul 2022 17:17:30 +0200 Subject: [PATCH 19/19] Update 2-options.md --- documentation/2-options.md | 1 - 1 file changed, 1 deletion(-) diff --git a/documentation/2-options.md b/documentation/2-options.md index c8f99deb8..ab7b24c49 100644 --- a/documentation/2-options.md +++ b/documentation/2-options.md @@ -209,7 +209,6 @@ await got('https://httpbin.org/anything'); #### **Note:** > - If you're passing an absolute URL as `url`, you need to set `prefixUrl` to an empty string. - ### `signal` **Type: [`AbortSignal`](https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal)**