Fix timeout to apply to total operation including retries

The `timeout` option now correctly applies to the entire operation including all retry attempts, as documented. Previously, each retry attempt received its own independent timeout.

Fixes #784

Author: sindresorhus

source/core/Ky.ts

@@ -1,6 +1,7 @@
 import {HTTPError} from '../errors/HTTPError.js';
 import {NonError} from '../errors/NonError.js';
 import {ForceRetryError} from '../errors/ForceRetryError.js';
+import {TimeoutError} from '../errors/TimeoutError.js';
 import type {
 	Input,
 	InternalOptions,
@@ -53,6 +54,11 @@ export class Ky {
 				throw new RangeError(`The \`timeout\` option cannot be greater than ${maxSafeTimeout}`);
 			}
 
+			// Track start time for total timeout across retries
+			if (ky.#startTime === undefined && typeof ky.#options.timeout === 'number') {
+				ky.#startTime = Date.now();
+			}
+
 			// Delay the fetch so that body method shortcuts can set the Accept header
 			await Promise.resolve();
 			// Before using ky.request, _fetch clones it and saves the clone for future retries to use.
@@ -210,6 +216,7 @@ export class Ky {
 	#originalRequest?: Request;
 	readonly #userProvidedAbortSignal?: AbortSignal;
 	#cachedNormalizedOptions: NormalizedOptions | undefined;
+	#startTime?: number;
 
 	// eslint-disable-next-line complexity
 	constructor(input: Input, options: Options = {}) {
@@ -328,6 +335,11 @@ export class Ky {
 		return Math.min(backoffLimit, jitteredDelay);
 	}
 
+	#clampRetryDelayToMax(retryDelay: number): number {
+		const {maxRetryAfter} = this.#options.retry;
+		return maxRetryAfter === undefined ? retryDelay : Math.min(maxRetryAfter, retryDelay);
+	}
+
 	async #calculateRetryDelay(error: unknown) {
 		this.#retryCount++;
 
@@ -389,9 +401,14 @@ export class Ky {
 					after -= Date.now();
 				}
 
-				const max = this.#options.retry.maxRetryAfter ?? after;
+				if (!Number.isFinite(after)) {
+					return this.#clampRetryDelayToMax(this.#calculateDelay());
+				}
+
+				after = Math.max(0, after);
+
 				// Don't apply jitter when server provides explicit retry timing
-				return after < max ? after : max;
+				return this.#clampRetryDelayToMax(after);
 			}
 
 			if (error.response.status === 413) {
@@ -533,13 +550,28 @@ export class Ky {
 		try {
 			return await function_();
 		} catch (error) {
-			const ms = Math.min(await this.#calculateRetryDelay(error), maxSafeTimeout);
-			if (this.#retryCount < 1) {
-				throw error;
+			const retryDelay = Math.min(await this.#calculateRetryDelay(error), maxSafeTimeout);
+
+			let delayMs = retryDelay;
+			const remainingTimeout = this.#getRemainingTimeout();
+			if (remainingTimeout !== undefined) {
+				if (remainingTimeout <= 0) {
+					throw new TimeoutError(this.request);
+				}
+
+				delayMs = Math.min(delayMs, remainingTimeout);
 			}
 
 			// Only use user-provided signal for delay, not our internal abortController
-			await delay(ms, this.#userProvidedAbortSignal ? {signal: this.#userProvidedAbortSignal} : {});
+			await delay(delayMs, this.#userProvidedAbortSignal ? {signal: this.#userProvidedAbortSignal} : {});
+
+			const remainingTimeoutAfterDelay = this.#getRemainingTimeout();
+			if (
+				remainingTimeoutAfterDelay !== undefined
+				&& remainingTimeoutAfterDelay <= 0
+			) {
+				throw new TimeoutError(this.request);
+			}
 
 			// Apply custom request from forced retry before beforeRetry hooks
 			// Ensure the custom request has the correct managed signal for timeouts and user aborts
@@ -576,6 +608,14 @@ export class Ky {
 				}
 			}
 
+			const remainingTimeoutAfterBeforeRetryHooks = this.#getRemainingTimeout();
+			if (
+				remainingTimeoutAfterBeforeRetryHooks !== undefined
+				&& remainingTimeoutAfterBeforeRetryHooks <= 0
+			) {
+				throw new TimeoutError(this.request);
+			}
+
 			return this.#retry(function_);
 		}
 	}
@@ -617,7 +657,28 @@ export class Ky {
 			return this.#options.fetch(this.#originalRequest, nonRequestOptions);
 		}
 
-		return timeout(this.#originalRequest, nonRequestOptions, this.#abortController, this.#options as TimeoutOptions);
+		const remainingTimeout = this.#getRemainingTimeout() ?? this.#options.timeout;
+		if (remainingTimeout <= 0) {
+			throw new TimeoutError(this.request);
+		}
+
+		return timeout(this.#originalRequest, nonRequestOptions, this.#abortController, {
+			...this.#options,
+			timeout: remainingTimeout,
+		} as TimeoutOptions);
+	}
+
+	#getRemainingTimeout(): number | undefined {
+		if (this.#options.timeout === false) {
+			return undefined;
+		}
+
+		if (this.#startTime === undefined) {
+			return this.#options.timeout;
+		}
+
+		const elapsed = Date.now() - this.#startTime;
+		return Math.max(0, this.#options.timeout - elapsed);
 	}
 
 	#getNormalizedOptions(): NormalizedOptions {

test/hooks.ts

@@ -1,6 +1,11 @@
 import test from 'ava';
 import delay from 'delay';
-import ky, {HTTPError, isHTTPError, isForceRetryError} from '../source/index.js';
+import ky, {
+	HTTPError,
+	isHTTPError,
+	isForceRetryError,
+	TimeoutError,
+} from '../source/index.js';
 import {type Options} from '../source/types/options.js';
 import {createHttpTestServer} from './helpers/create-http-test-server.js';
 
@@ -524,6 +529,73 @@ test('beforeRetry hook is never called for the initial request', async t => {
 	await server.close();
 });
 
+test('beforeRequest hook on initial request cannot bypass total timeout budget', async t => {
+	let fetchCallCount = 0;
+
+	const customFetch: typeof fetch = async () => {
+		fetchCallCount++;
+		return new Response('ok');
+	};
+
+	const error = await t.throwsAsync(
+		ky('https://example.com', {
+			fetch: customFetch,
+			timeout: 100,
+			hooks: {
+				beforeRequest: [
+					async () => {
+						await delay(200);
+					},
+				],
+			},
+		}).text(),
+		{
+			name: 'TimeoutError',
+		},
+	);
+
+	t.true(error instanceof TimeoutError);
+	t.is(fetchCallCount, 0);
+});
+
+test('beforeRequest hook on retry cannot bypass total timeout budget', async t => {
+	let fetchCallCount = 0;
+
+	const customFetch: typeof fetch = async () => {
+		fetchCallCount++;
+		if (fetchCallCount === 1) {
+			return new Response('first-attempt', {status: 500});
+		}
+
+		return new Response('second-attempt-success');
+	};
+
+	await t.throwsAsync(
+		ky('https://example.com', {
+			fetch: customFetch,
+			timeout: 100,
+			retry: {
+				limit: 1,
+				delay: () => 0,
+			},
+			hooks: {
+				beforeRequest: [
+					async (_request, _options, state) => {
+						if (state.retryCount > 0) {
+							await delay(200);
+						}
+					},
+				],
+			},
+		}).text(),
+		{
+			name: 'TimeoutError',
+		},
+	);
+
+	t.is(fetchCallCount, 1);
+});
+
 test('beforeRetry hook allows modifications of non initial requests', async t => {
 	let requestCount = 0;
 
@@ -791,6 +863,45 @@ test('beforeRetry hook can cancel retries by returning `stop`', async t => {
 	await server.close();
 });
 
+test('beforeRetry hook respects total timeout budget', async t => {
+	let fetchCallCount = 0;
+	let beforeRetryCallCount = 0;
+
+	const customFetch: typeof fetch = async () => {
+		fetchCallCount++;
+		if (fetchCallCount === 1) {
+			return new Response('first-attempt', {status: 500});
+		}
+
+		return new Response('second-attempt-success');
+	};
+
+	await t.throwsAsync(
+		ky('https://example.com', {
+			fetch: customFetch,
+			timeout: 1000,
+			retry: {
+				limit: 1,
+				delay: () => 0,
+			},
+			hooks: {
+				beforeRetry: [
+					async () => {
+						beforeRetryCallCount++;
+						await delay(2000);
+					},
+				],
+			},
+		}).text(),
+		{
+			name: 'TimeoutError',
+		},
+	);
+
+	t.is(beforeRetryCallCount, 1);
+	t.is(fetchCallCount, 1);
+});
+
 test('catches beforeRetry thrown errors', async t => {
 	let requestCount = 0;
 
@@ -1309,6 +1420,42 @@ test('afterResponse hook can force retry with custom delay', async t => {
 	await server.close();
 });
 
+test('afterResponse forced retry respects total timeout budget', async t => {
+	let requestCount = 0;
+
+	const server = await createHttpTestServer();
+	server.get('/', (_request, response) => {
+		requestCount++;
+		response.json({error: {code: 'RETRY_WITH_DELAY'}});
+	});
+
+	await t.throwsAsync(
+		ky.get(server.url, {
+			timeout: 100,
+			retry: {
+				limit: 3,
+			},
+			hooks: {
+				afterResponse: [
+					async (_request, _options, response) => {
+						const data = await response.clone().json();
+						if (data.error?.code === 'RETRY_WITH_DELAY') {
+							return ky.retry({delay: 200});
+						}
+					},
+				],
+			},
+		}),
+		{
+			name: 'TimeoutError',
+		},
+	);
+
+	t.true(requestCount <= 1);
+
+	await server.close();
+});
+
 test('afterResponse hook forced retry respects retry limit', async t => {
 	let requestCount = 0;
 

test/http-error.ts

@@ -537,7 +537,7 @@ test('HTTPError does not throw TypeError when error response stream is already l
 	t.is(error?.data, undefined);
 });
 
-test('retry is not blocked by never-ending error response body', async t => {
+test('never-ending error response body still respects total timeout budget', async t => {
 	let requestCount = 0;
 
 	const customFetch: typeof fetch = async () => {
@@ -559,12 +559,15 @@ test('retry is not blocked by never-ending error response body', async t => {
 	};
 
 	const start = Date.now();
-	const text = await ky('https://example.com', {
+	const error = await t.throwsAsync(ky('https://example.com', {
 		fetch: customFetch,
-		retry: 1,
-		timeout: 50,
-	}).text();
-	t.is(text, 'ok');
-	t.is(requestCount, 2);
+		retry: {
+			limit: 1,
+			delay: () => 0,
+		},
+		timeout: 1000,
+	}).text());
+	t.is(error?.name, 'TimeoutError');
+	t.is(requestCount, 1);
 	t.true(Date.now() - start < 3000);
 });