Harden retry timing and callback validation

sindresorhus · sindresorhus · commit 9d47b60e2c8f · 2026-03-26T23:03:48.000+07:00
diff --git a/index.d.ts b/index.d.ts
@@ -22,7 +22,7 @@ export type RetryContext = {
 	This is calculated based on `minTimeout`, `factor`, `maxTimeout`, and `randomize` options.
 
 	Note: The actual delay may be shorter if it would exceed `maxRetryTime`.
-	This is `0` when the retry is skipped or when no retry will occur.
+	This is `0` when the retry is skipped or when no retry will occur based on the checks completed before the current callback runs.
 	*/
 	readonly retryDelay: number;
 };
diff --git a/index.js b/index.js
@@ -32,6 +32,16 @@ function validateNumberOption(name, value, {min = 0, allowInfinity = false} = {}
 	}
 }
 
+function validateFunctionOption(name, value) {
+	if (value === undefined) {
+		return;
+	}
+
+	if (typeof value !== 'function') {
+		throw new TypeError(`Expected \`${name}\` to be a function.`);
+	}
+}
+
 export class AbortError extends Error {
 	constructor(message) {
 		super();
@@ -67,6 +77,31 @@ function calculateRemainingTime(start, max) {
 	return max - (performance.now() - start);
 }
 
+async function delayForRetry(delay, options) {
+	if (delay <= 0) {
+		return;
+	}
+
+	await new Promise((resolve, reject) => {
+		const onAbort = () => {
+			clearTimeout(timeoutToken);
+			options.signal?.removeEventListener('abort', onAbort);
+			reject(options.signal.reason);
+		};
+
+		const timeoutToken = setTimeout(() => {
+			options.signal?.removeEventListener('abort', onAbort);
+			resolve();
+		}, delay);
+
+		if (options.unref) {
+			timeoutToken.unref?.();
+		}
+
+		options.signal?.addEventListener('abort', onAbort, {once: true});
+	});
+}
+
 async function onAttemptFailure({error, attemptNumber, retriesConsumed, startTime, options}) {
 	const normalizedError = error instanceof Error
 		? error
@@ -136,35 +171,22 @@ async function onAttemptFailure({error, attemptNumber, retriesConsumed, startTim
 		throw normalizedError;
 	}
 
+	const remainingTimeAfterShouldRetry = calculateRemainingTime(startTime, maxRetryTime);
+
+	if (remainingTimeAfterShouldRetry <= 0) {
+		throw normalizedError;
+	}
+
 	if (!consumeRetry) {
 		options.signal?.throwIfAborted();
 		return false;
 	}
 
-	const finalDelay = Math.min(effectiveDelay, remainingTime);
+	const finalDelay = Math.min(effectiveDelay, remainingTimeAfterShouldRetry);
 
 	options.signal?.throwIfAborted();
 
-	if (finalDelay > 0) {
-		await new Promise((resolve, reject) => {
-			const onAbort = () => {
-				clearTimeout(timeoutToken);
-				options.signal?.removeEventListener('abort', onAbort);
-				reject(options.signal.reason);
-			};
-
-			const timeoutToken = setTimeout(() => {
-				options.signal?.removeEventListener('abort', onAbort);
-				resolve();
-			}, finalDelay);
-
-			if (options.unref) {
-				timeoutToken.unref?.();
-			}
-
-			options.signal?.addEventListener('abort', onAbort, {once: true});
-		});
-	}
+	await delayForRetry(finalDelay, options);
 
 	options.signal?.throwIfAborted();
 
@@ -191,6 +213,9 @@ export default async function pRetry(input, options = {}) {
 	options.shouldConsumeRetry ??= () => true;
 
 	// Validate numeric options and normalize edge cases
+	validateFunctionOption('onFailedAttempt', options.onFailedAttempt);
+	validateFunctionOption('shouldRetry', options.shouldRetry);
+	validateFunctionOption('shouldConsumeRetry', options.shouldConsumeRetry);
 	validateNumberOption('factor', options.factor, {min: 0, allowInfinity: false});
 	validateNumberOption('minTimeout', options.minTimeout, {min: 0, allowInfinity: false});
 	validateNumberOption('maxTimeout', options.maxTimeout, {min: 0, allowInfinity: true});
diff --git a/readme.md b/readme.md
@@ -90,7 +90,7 @@ The `context` object contains:
 - `attemptNumber` - The attempt number (starts at 1)
 - `retriesLeft` - Number of retries remaining
 - `retriesConsumed` - Number of retries consumed so far
-- `retryDelay` - The delay in milliseconds before the next retry (based on `minTimeout`, `factor`, `maxTimeout`, and `randomize`). This is `0` when the retry is skipped or when no retry will occur.
+- `retryDelay` - The delay in milliseconds before the next retry (based on `minTimeout`, `factor`, `maxTimeout`, and `randomize`). This is `0` when the retry is skipped or when no retry will occur based on the checks completed before the current callback runs.
 
 The `onFailedAttempt` function can return a promise. For example, to add a [delay](https://github.com/sindresorhus/delay):
 
diff --git a/test.js b/test.js
@@ -82,6 +82,25 @@ test('shouldRetry is not called for non-network TypeError', async t => {
 	t.is(shouldRetryCalled, 0);
 });
 
+test('async shouldRetry respects maxRetryTime after callback', async t => {
+	let attempts = 0;
+
+	await t.throwsAsync(pRetry(async () => {
+		attempts++;
+		throw new Error('fail');
+	}, {
+		retries: 5,
+		maxRetryTime: 50,
+		minTimeout: 0,
+		async shouldRetry() {
+			await delay(100);
+			return true;
+		},
+	}));
+
+	t.is(attempts, 1);
+});
+
 test('does not retry non-network TypeError when shouldConsumeRetry returns false', async t => {
 	const typeErrorFixture = new TypeError('type-error-fixture');
 	let attempts = 0;
@@ -100,6 +119,26 @@ test('does not retry non-network TypeError when shouldConsumeRetry returns false
 	t.is(attempts, 1);
 });
 
+test('throws on invalid callback options', async t => {
+	await t.throwsAsync(pRetry(async () => {}, {
+		onFailedAttempt: 1,
+	}), {
+		message: 'Expected `onFailedAttempt` to be a function.',
+	});
+
+	await t.throwsAsync(pRetry(async () => {}, {
+		shouldRetry: 1,
+	}), {
+		message: 'Expected `shouldRetry` to be a function.',
+	});
+
+	await t.throwsAsync(pRetry(async () => {}, {
+		shouldConsumeRetry: 1,
+	}), {
+		message: 'Expected `shouldConsumeRetry` to be a function.',
+	});
+});
+
 test('retry on TypeError - failed to fetch', async t => {
 	const typeErrorFixture = new TypeError('Failed to fetch');
 	let index = 0;
