Retry request even if retry delay is zero (#604)

sindresorhus · Jun 30, 2024 · 4886b66 · 4886b66
1 parent e044626
commit 4886b66
Show file tree

Hide file tree

Showing 2 changed files with 71 additions and 37 deletions.
diff --git a/source/core/Ky.ts b/source/core/Ky.ts
@@ -211,33 +211,33 @@ export class Ky {
 	protected _calculateRetryDelay(error: unknown) {
 		this._retryCount++;
 
-		if (this._retryCount <= this._options.retry.limit && !(error instanceof TimeoutError)) {
-			if (error instanceof HTTPError) {
-				if (!this._options.retry.statusCodes.includes(error.response.status)) {
-					return 0;
-				}
+		if (this._retryCount > this._options.retry.limit || error instanceof TimeoutError) {
+			throw error;
+		}
 
-				const retryAfter = error.response.headers.get('Retry-After');
-				if (retryAfter && this._options.retry.afterStatusCodes.includes(error.response.status)) {
-					let after = Number(retryAfter) * 1000;
-					if (Number.isNaN(after)) {
-						after = Date.parse(retryAfter) - Date.now();
-					}
+		if (error instanceof HTTPError) {
+			if (!this._options.retry.statusCodes.includes(error.response.status)) {
+				throw error;
+			}
 
-					const max = this._options.retry.maxRetryAfter ?? after;
-					return after < max ? after : max;
+			const retryAfter = error.response.headers.get('Retry-After');
+			if (retryAfter && this._options.retry.afterStatusCodes.includes(error.response.status)) {
+				let after = Number(retryAfter) * 1000;
+				if (Number.isNaN(after)) {
+					after = Date.parse(retryAfter) - Date.now();
 				}
 
-				if (error.response.status === 413) {
-					return 0;
-				}
+				const max = this._options.retry.maxRetryAfter ?? after;
+				return after < max ? after : max;
 			}
 
-			const retryDelay = this._options.retry.delay(this._retryCount);
-			return Math.min(this._options.retry.backoffLimit, retryDelay);
+			if (error.response.status === 413) {
+				throw error;
+			}
 		}
 
-		return 0;
+		const retryDelay = this._options.retry.delay(this._retryCount);
+		return Math.min(this._options.retry.backoffLimit, retryDelay);
 	}
 
 	protected _decorateResponse(response: Response): Response {
@@ -253,28 +253,28 @@ export class Ky {
 			return await function_();
 		} catch (error) {
 			const ms = Math.min(this._calculateRetryDelay(error), maxSafeTimeout);
-			if (ms !== 0 && this._retryCount > 0) {
-				await delay(ms, {signal: this._options.signal});
+			if (this._retryCount < 1) {
+				throw error;
+			}
 
-				for (const hook of this._options.hooks.beforeRetry) {
-					// eslint-disable-next-line no-await-in-loop
-					const hookResult = await hook({
-						request: this.request,
-						options: (this._options as unknown) as NormalizedOptions,
-						error: error as Error,
-						retryCount: this._retryCount,
-					});
-
-					// If `stop` is returned from the hook, the retry process is stopped
-					if (hookResult === stop) {
-						return;
-					}
-				}
+			await delay(ms, {signal: this._options.signal});
 
-				return this._retry(function_);
+			for (const hook of this._options.hooks.beforeRetry) {
+				// eslint-disable-next-line no-await-in-loop
+				const hookResult = await hook({
+					request: this.request,
+					options: (this._options as unknown) as NormalizedOptions,
+					error: error as Error,
+					retryCount: this._retryCount,
+				});
+
+				// If `stop` is returned from the hook, the retry process is stopped
+				if (hookResult === stop) {
+					return;
+				}
 			}
 
-			throw error;
+			return this._retry(function_);
 		}
 	}
 

diff --git a/test/retry.ts b/test/retry.ts
@@ -86,6 +86,40 @@ test('not on POST', async t => {
 	await server.close();
 });
 
+test('respect Retry-After: 0 and retry immediately', async t => {
+	const retryCount = 4;
+	let requestCount = 0;
+
+	const server = await createHttpTestServer();
+	server.get('/', (_request, response) => {
+		requestCount++;
+
+		if (requestCount === retryCount + 1) {
+			response.end(fixture);
+		} else {
+			response.writeHead(413, {
+				'Retry-After': 0,
+			});
+
+			response.end('');
+		}
+	});
+
+	await withPerformance({
+		t,
+		expectedDuration: 4 + 4 + 4 + 4,
+		async test() {
+			t.is(await ky(server.url, {
+				retry: retryCount,
+			}).text(), fixture);
+		},
+	});
+
+	t.is(requestCount, 5);
+
+	await server.close();
+});
+
 test('respect 413 Retry-After', async t => {
 	let requestCount = 0;