Clamp Retry-After time to retry.maxRetryAfter (#603)

* Clamp Retry-After time to retry.maxRetryAfter

We now use `maxRetryAfter` as a limit, rather than a threshold for canceling the retry. If `Retry-After` is greater than `maxRetryAfter`, then `Retry-After` will be capped.

* Document `maxRetryAfter` changes

readme.md

@@ -220,7 +220,7 @@ An object representing `limit`, `methods`, `statusCodes` and `maxRetryAfter` fie
 
 If `retry` is a number, it will be used as `limit` and other defaults will remain in place.
 
-If `maxRetryAfter` is set to `undefined`, it will use `options.timeout`. If [`Retry-After`](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Retry-After) header is greater than `maxRetryAfter`, it will cancel the request.
+If `maxRetryAfter` is set to `undefined`, it will use `options.timeout`. If [`Retry-After`](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Retry-After) header is greater than `maxRetryAfter`, it will use `maxRetryAfter`.
 
 The `backoffLimit` option is the upper limit of the delay per retry in milliseconds.
 To clamp the delay, set `backoffLimit` to 1000, for example.

source/core/Ky.ts

@@ -219,18 +219,13 @@ export class Ky {
 
 				const retryAfter = error.response.headers.get('Retry-After');
 				if (retryAfter && this._options.retry.afterStatusCodes.includes(error.response.status)) {
-					let after = Number(retryAfter);
+					let after = Number(retryAfter) * 1000;
 					if (Number.isNaN(after)) {
 						after = Date.parse(retryAfter) - Date.now();
-					} else {
-						after *= 1000;
 					}
 
-					if (this._options.retry.maxRetryAfter !== undefined && after > this._options.retry.maxRetryAfter) {
-						return 0;
-					}
-
-					return after;
+					const max = this._options.retry.maxRetryAfter ?? after;
+					return after < max ? after : max;
 				}
 
 				if (error.response.status === 413) {

test/retry.ts

@@ -221,44 +221,54 @@ test('respect retry methods', async t => {
 });
 
 test('respect maxRetryAfter', async t => {
+	const retryCount = 4;
 	let requestCount = 0;
 
 	const server = await createHttpTestServer();
-	server.get('/', async (_request, response) => {
+	server.get('/', (_request, response) => {
 		requestCount++;
 
-		response.writeHead(413, {
-			'Retry-After': 1,
-		});
+		if (requestCount === retryCount + 1) {
+			response.end(fixture);
+		} else {
+			response.writeHead(413, {
+				'Retry-After': 1,
+			});
 
-		response.end('');
+			response.end('');
+		}
 	});
 
-	await t.throwsAsync(
-		ky(server.url, {
-			retry: {
-				limit: 5,
-				maxRetryAfter: 100,
-			},
-		}).text(),
-		{
-			message: /Payload Too Large/,
+	await withPerformance({
+		t,
+		expectedDuration: 420 + 420 + 420 + 420,
+		async test() {
+			t.is(await ky(server.url, {
+				retry: {
+					limit: retryCount,
+					maxRetryAfter: 420,
+				},
+			}).text(), fixture);
 		},
-	);
-	t.is(requestCount, 1);
+	});
+
+	t.is(requestCount, 5);
 
 	requestCount = 0;
-	await t.throwsAsync(
-		ky(server.url, {
-			retry: {
-				limit: 4,
-				maxRetryAfter: 2000,
-			},
-		}).text(),
-		{
-			message: /Payload Too Large/,
+
+	await withPerformance({
+		t,
+		expectedDuration: 1000 + 1000 + 1000 + 1000,
+		async test() {
+			t.is(await ky(server.url, {
+				retry: {
+					limit: retryCount,
+					maxRetryAfter: 2000,
+				},
+			}).text(), fixture);
 		},
-	);
+	});
+
 	t.is(requestCount, 5);
 
 	await server.close();
@@ -467,6 +477,8 @@ test('respect maximum backoffLimit', async t => {
 		},
 	});
 
+	t.is(requestCount, 5);
+
 	requestCount = 0;
 
 	await withPerformance({
@@ -482,18 +494,20 @@ test('respect maximum backoffLimit', async t => {
 		},
 	});
 
+	t.is(requestCount, 5);
+
 	await server.close();
 });
 
 test('respect custom retry.delay', async t => {
-	const retryCount = 5;
+	const retryCount = 4;
 	let requestCount = 0;
 
 	const server = await createHttpTestServer();
 	server.get('/', (_request, response) => {
 		requestCount++;
 
-		if (requestCount === retryCount) {
+		if (requestCount === retryCount + 1) {
 			response.end(fixture);
 		} else {
 			response.sendStatus(500);
@@ -513,5 +527,7 @@ test('respect custom retry.delay', async t => {
 		},
 	});
 
+	t.is(requestCount, 5);
+
 	await server.close();
 });