Fix abortions between retries (#433)

Co-authored-by: Sindre Sorhus <sindresorhus@gmail.com>

package.json

@@ -55,6 +55,7 @@
 		"@types/body-parser": "^1.19.2",
 		"@types/busboy": "^0.3.1",
 		"@types/express": "^4.17.14",
+		"@types/node": "^18.11.7",
 		"@types/node-fetch": "^2.6.2",
 		"abort-controller": "^3.0.0",
 		"ava": "^4.3.3",

source/core/Ky.ts

@@ -5,7 +5,8 @@ import type {Input, InternalOptions, NormalizedOptions, Options, SearchParamsIni
 import {ResponsePromise} from '../types/ResponsePromise.js';
 import {deepMerge, mergeHeaders} from '../utils/merge.js';
 import {normalizeRequestMethod, normalizeRetryOptions} from '../utils/normalize.js';
-import {delay, timeout, TimeoutOptions} from '../utils/time.js';
+import timeout, {TimeoutOptions} from '../utils/timeout.js';
+import delay from '../utils/delay.js';
 import {ObjectEntries} from '../utils/types.js';
 import {
 	maxSafeTimeout,
@@ -153,8 +154,10 @@ export class Ky {
 		if (supportsAbortController) {
 			this.abortController = new globalThis.AbortController();
 			if (this._options.signal) {
+				const originalSignal = this._options.signal;
+
 				this._options.signal.addEventListener('abort', () => {
-					this.abortController!.abort();
+					this.abortController!.abort(originalSignal.reason);
 				});
 			}
 
@@ -247,7 +250,7 @@ export class Ky {
 		} catch (error) {
 			const ms = Math.min(this._calculateRetryDelay(error), maxSafeTimeout);
 			if (ms !== 0 && this._retryCount > 0) {
-				await delay(ms);
+				await delay(ms, {signal: this._options.signal});
 
 				for (const hook of this._options.hooks.beforeRetry) {
 					// eslint-disable-next-line no-await-in-loop

source/errors/DOMException.ts

@@ -0,0 +1,8 @@
+const DOMException = globalThis.DOMException ?? Error;
+
+export default DOMException;
+
+// When targeting Node.js 18, use `signal.throwIfAborted()` (https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/throwIfAborted)
+export function composeAbortError(signal?: AbortSignal) {
+	return new DOMException(signal?.reason ?? 'The operation was aborted.');
+}

source/utils/delay.ts

@@ -0,0 +1,34 @@
+// https://github.com/sindresorhus/delay/tree/ab98ae8dfcb38e1593286c94d934e70d14a4e111
+
+import {composeAbortError} from '../errors/DOMException.js';
+import {InternalOptions} from '../types/options.js';
+
+export interface DelayOptions {
+	signal?: InternalOptions['signal'];
+}
+
+export default async function delay(
+	ms: number,
+	{signal}: DelayOptions,
+): Promise<void> {
+	return new Promise((resolve, reject) => {
+		if (signal) {
+			if (signal.aborted) {
+				reject(composeAbortError(signal));
+				return;
+			}
+
+			signal.addEventListener('abort', handleAbort, {once: true});
+		}
+
+		function handleAbort() {
+			reject(composeAbortError(signal!));
+			clearTimeout(timeoutId);
+		}
+
+		const timeoutId = setTimeout(() => {
+			signal?.removeEventListener('abort', handleAbort);
+			resolve();
+		}, ms);
+	});
+}

source/utils/time.ts → source/utils/timeout.ts

@@ -6,12 +6,12 @@ export type TimeoutOptions = {
 };
 
 // `Promise.race()` workaround (#91)
-export const timeout = async (
+export default async function timeout(
 	request: Request,
 	abortController: AbortController | undefined,
 	options: TimeoutOptions,
-): Promise<Response> =>
-	new Promise((resolve, reject) => {
+): Promise<Response> {
+	return new Promise((resolve, reject) => {
 		const timeoutId = setTimeout(() => {
 			if (abortController) {
 				abortController.abort();
@@ -28,7 +28,4 @@ export const timeout = async (
 				clearTimeout(timeoutId);
 			});
 	});
-
-export const delay = async (ms: number) => new Promise(resolve => {
-	setTimeout(resolve, ms);
-});
+}

test/browser.ts

@@ -83,10 +83,12 @@ test('aborting a request', withPage, async (t: ExecutionContext, page: Page) =>
 	const error = await page.evaluate(async (url: string) => {
 		const controller = new AbortController();
 		const request = window.ky(`${url}/test`, {signal: controller.signal}).text();
-		controller.abort();
+		controller.abort('🦄');
 		return request.catch(error_ => error_.toString());
 	}, server.url);
-	t.is(error, 'AbortError: Failed to execute \'fetch\' on \'Window\': The user aborted a request.');
+
+	// TODO: When targeting Node.js 18, also assert that the error is a DOMException
+	t.is(error.split(': ')[1], '🦄');
 
 	await server.close();
 });

test/main.ts

@@ -552,7 +552,7 @@ test('ky.extend()', async t => {
 	await server.close();
 });
 
-test('throws AbortError when aborted by user', async t => {
+test('throws DOMException when aborted by user', async t => {
 	const server = await createHttpTestServer();
 	// eslint-disable-next-line @typescript-eslint/no-empty-function
 	server.get('/', () => {});
@@ -562,7 +562,8 @@ test('throws AbortError when aborted by user', async t => {
 	const response = ky(server.url, {signal});
 	abortController.abort();
 
-	await t.throwsAsync(response, {name: 'AbortError'});
+	const {name} = (await t.throwsAsync(response))!;
+	t.true(['DOMException', 'Error'].includes(name), `Expected DOMException or Error, got ${name}`);
 });
 
 test('supports Request instance as input', async t => {