Don't break the default behavior when using custom retry function (#830)

readme.md

@@ -347,20 +347,21 @@ This also accepts an `object` with the following fields to constrain the duratio
 
 Type: `number | object`<br>
 Default:
-- retries: `2`
+- limit: `2`
+- calculateDelay: `(attemptCount, retryOptions, error, computedValue) => computedValue`
 - methods: `GET` `PUT` `HEAD` `DELETE` `OPTIONS` `TRACE`
 - statusCodes: [`408`](https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/408) [`413`](https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/413) [`429`](https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/429) [`500`](https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/500) [`502`](https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/502) [`503`](https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/503) [`504`](https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/504)
 - maxRetryAfter: `undefined`
 - errorCodes: `ETIMEDOUT` `ECONNRESET` `EADDRINUSE` `ECONNREFUSED` `EPIPE` `ENOTFOUND` `ENETUNREACH` `EAI_AGAIN`
 
-An object representing `retries`, `methods`, `statusCodes`, `maxRetryAfter` and `errorCodes` fields for the time until retry, allowed methods, allowed status codes, maximum [`Retry-After`](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Retry-After) time and allowed error codes.
+An object representing `limit`, `calculateDelay`, `methods`, `statusCodes`, `maxRetryAfter` and `errorCodes` fields for maximum retry count, retry handler, allowed methods, allowed status codes, maximum [`Retry-After`](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Retry-After) time and allowed error codes.
 
 If `maxRetryAfter` is set to `undefined`, it will use `options.timeout`.<br>
 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.
 
 Delays between retries counts with function `1000 * Math.pow(2, retry) + Math.random() * 100`, where `retry` is attempt number (starts from 1).
 
-The `retries` property can be a `number` or a `function` with `retry` and `error` arguments. The function must return a delay in milliseconds (`0` return value cancels retry).
+The `calculateDelay` property is a `function` with `attemptCount`, `retryOptions`, `error` and `computedValue` arguments for current retry count, the retry options, error and default computed value. The function must return a delay in milliseconds (`0` return value cancels retry).
 
 By default, it retries *only* on the specified methods, status codes, and on these network errors:
 - `ETIMEDOUT`: One of the [timeout](#timeout) limits were reached.

source/as-promise.ts

@@ -73,7 +73,7 @@ export default function asPromise(options: NormalizedOptions): CancelableRequest
 							...updatedOptions,
 							// @ts-ignore TS complaining that it's missing properties, which get merged
 							retry: {
-								retries: () => 0
+								calculateDelay: () => 0
 							},
 							throwHttpErrors: false,
 							responseType: 'text',

source/as-stream.ts

@@ -16,7 +16,7 @@ export default function asStream(options: NormalizedOptions): ProxyStream {
 	const piped = new Set();
 	let isFinished = false;
 
-	options.retry.retries = () => 0;
+	options.retry.calculateDelay = () => 0;
 
 	if (options.body) {
 		proxy.write = () => {

source/calculate-retry-delay.ts

@@ -0,0 +1,48 @@
+import is from '@sindresorhus/is';
+import {HTTPError, ParseError, MaxRedirectsError, GotError} from './errors';
+import {
+	RetryFunction,
+	ErrorCode,
+	StatusCode,
+	Method
+} from './utils/types';
+
+const retryAfterStatusCodes: ReadonlySet<StatusCode> = new Set([413, 429, 503]);
+
+const calculateRetryDelay: RetryFunction = ({attemptCount, retryOptions, error}) => {
+	if (attemptCount > retryOptions.limit) {
+		return 0;
+	}
+
+	const hasMethod = retryOptions.methods.has((error as GotError).options.method as Method);
+	const hasErrorCode = Reflect.has(error, 'code') && retryOptions.errorCodes.has((error as GotError).code as ErrorCode);
+	const hasStatusCode = Reflect.has(error, 'response') && retryOptions.statusCodes.has((error as HTTPError | ParseError | MaxRedirectsError).response.statusCode as StatusCode);
+	if (!hasMethod || (!hasErrorCode && !hasStatusCode)) {
+		return 0;
+	}
+
+	const {response} = error as HTTPError | ParseError | MaxRedirectsError;
+	if (response && Reflect.has(response.headers, 'retry-after') && retryAfterStatusCodes.has(response.statusCode as StatusCode)) {
+		let after = Number(response.headers['retry-after']);
+		if (is.nan(after)) {
+			after = Date.parse(response.headers['retry-after']) - Date.now();
+		} else {
+			after *= 1000;
+		}
+
+		if (after > retryOptions.maxRetryAfter) {
+			return 0;
+		}
+
+		return after;
+	}
+
+	if (response && response.statusCode === 413) {
+		return 0;
+	}
+
+	const noise = Math.random() * 100;
+	return ((2 ** (attemptCount - 1)) * 1000) + noise;
+};
+
+export default calculateRetryDelay;

source/index.ts

@@ -6,7 +6,7 @@ const defaults: Partial<Defaults> = {
 	options: {
 		method: 'GET',
 		retry: {
-			retries: 2,
+			limit: 2,
 			methods: [
 				'GET',
 				'PUT',

source/normalize-arguments.ts

@@ -14,17 +14,12 @@ import {
 	Defaults,
 	NormalizedOptions,
 	NormalizedRetryOptions,
-	RetryOption,
+	RetryOptions,
 	Method,
 	Delays,
-	ErrorCode,
-	StatusCode,
 	URLArgument,
 	URLOrOptions
 } from './utils/types';
-import {HTTPError, ParseError, MaxRedirectsError, GotError} from './errors';
-
-const retryAfterStatusCodes: ReadonlySet<StatusCode> = new Set([413, 429, 503]);
 
 let hasShownDeprecation = false;
 
@@ -80,20 +75,20 @@ export const preNormalizeArguments = (options: Options, defaults?: Options): Nor
 
 	const {retry} = options;
 	options.retry = {
-		retries: () => 0,
+		calculateDelay: retryObject => retryObject.computedValue,
 		methods: new Set(),
 		statusCodes: new Set(),
 		errorCodes: new Set(),
 		maxRetryAfter: undefined
 	};
 
 	if (is.nonEmptyObject(defaults) && retry !== false) {
-		options.retry = {...(defaults.retry as RetryOption)};
+		options.retry = {...(defaults.retry as RetryOptions)};
 	}
 
 	if (retry !== false) {
 		if (is.number(retry)) {
-			options.retry.retries = retry;
+			options.retry.limit = retry;
 		} else {
 			// eslint-disable-next-line @typescript-eslint/no-object-literal-type-assertion
 			options.retry = {...options.retry, ...retry} as NormalizedRetryOptions;
@@ -243,46 +238,6 @@ export const normalizeArguments = (url: URLOrOptions, options: NormalizedOptions
 		options.method = options.method.toUpperCase() as Method;
 	}
 
-	if (!is.function_(options.retry.retries)) {
-		const {retries} = options.retry;
-
-		options.retry.retries = (iteration, error) => {
-			if (iteration > retries) {
-				return 0;
-			}
-
-			const hasMethod = options.retry.methods.has((error as GotError).options.method as Method);
-			const hasErrorCode = Reflect.has(error, 'code') && options.retry.errorCodes.has((error as GotError).code as ErrorCode);
-			const hasStatusCode = Reflect.has(error, 'response') && options.retry.statusCodes.has((error as HTTPError | ParseError | MaxRedirectsError).response.statusCode as StatusCode);
-			if (!hasMethod || (!hasErrorCode && !hasStatusCode)) {
-				return 0;
-			}
-
-			const {response} = error as HTTPError | ParseError | MaxRedirectsError;
-			if (response && Reflect.has(response.headers, 'retry-after') && retryAfterStatusCodes.has(response.statusCode as StatusCode)) {
-				let after = Number(response.headers['retry-after']);
-				if (is.nan(after)) {
-					after = Date.parse(response.headers['retry-after']!) - Date.now();
-				} else {
-					after *= 1000;
-				}
-
-				if (after > options.retry.maxRetryAfter) {
-					return 0;
-				}
-
-				return after;
-			}
-
-			if (response && response.statusCode === 413) {
-				return 0;
-			}
-
-			const noise = Math.random() * 100;
-			return ((2 ** (iteration - 1)) * 1000) + noise;
-		};
-	}
-
 	return options;
 };
 

source/request-as-event-emitter.ts

@@ -12,6 +12,7 @@ import ResponseLike = require('responselike');
 import timedOut, {TimeoutError as TimedOutTimeoutError} from './utils/timed-out';
 import getBodySize from './utils/get-body-size';
 import isFormData from './utils/is-form-data';
+import calculateRetryDelay from './calculate-retry-delay';
 import getResponse from './get-response';
 import {uploadProgress} from './progress';
 import {CacheError, UnsupportedProtocolError, MaxRedirectsError, RequestError, TimeoutError} from './errors';
@@ -267,8 +268,20 @@ export default (options: NormalizedOptions, input?: TransformStream) => {
 	emitter.retry = (error): boolean => {
 		let backoff: number;
 
+		retryCount++;
+
 		try {
-			backoff = options.retry.retries(++retryCount, error);
+			backoff = options.retry.calculateDelay({
+				attemptCount: retryCount,
+				retryOptions: options.retry,
+				error,
+				computedValue: calculateRetryDelay({
+					attemptCount: retryCount,
+					retryOptions: options.retry,
+					error,
+					computedValue: 0
+				})
+			});
 		} catch (error2) {
 			emitError(error2);
 			return false;

source/utils/types.ts

@@ -67,20 +67,29 @@ export interface Response extends http.IncomingMessage {
 	request: { options: NormalizedOptions };
 }
 
-export type RetryFunction = (retry: number, error: Error | GotError | ParseError | HTTPError | MaxRedirectsError) => number;
+export interface RetryObject {
+	attemptCount: number;
+	retryOptions: NormalizedRetryOptions;
+	error: Error | GotError | ParseError | HTTPError | MaxRedirectsError;
+	computedValue: number;
+}
+
+export type RetryFunction = (retryObject: RetryObject) => number;
 
 export type HandlerFunction = <T extends ProxyStream | CancelableRequest<Response>>(options: NormalizedOptions, next: (options: NormalizedOptions) => T) => T;
 
-export interface RetryOption {
-	retries?: RetryFunction | number;
+export interface RetryOptions {
+	limit?: number;
+	calculateDelay?: RetryFunction;
 	methods?: Method[];
 	statusCodes?: StatusCode[];
 	errorCodes?: ErrorCode[];
 	maxRetryAfter?: number;
 }
 
 export interface NormalizedRetryOptions {
-	retries: RetryFunction;
+	limit: number;
+	calculateDelay: RetryFunction;
 	methods: ReadonlySet<Method>;
 	statusCodes: ReadonlySet<StatusCode>;
 	errorCodes: ReadonlySet<ErrorCode>;
@@ -121,7 +130,7 @@ export interface Options extends Omit<https.RequestOptions, 'agent' | 'timeout'
 	stream?: boolean;
 	encoding?: BufferEncoding | null;
 	method?: Method;
-	retry?: number | Partial<RetryOption | NormalizedRetryOptions>;
+	retry?: number | Partial<RetryOptions | NormalizedRetryOptions>;
 	throwHttpErrors?: boolean;
 	cookieJar?: CookieJar;
 	ignoreInvalidCookies?: boolean;

test/retry.ts

@@ -52,9 +52,9 @@ test('retry function gets iteration count', withServer, async (t, server, got) =
 	await got({
 		timeout: {socket: socketTimeout},
 		retry: {
-			retries: iteration => {
-				t.true(is.number(iteration));
-				return iteration < 2;
+			calculateDelay: ({attemptCount}) => {
+				t.true(is.number(attemptCount));
+				return attemptCount < 2;
 			}
 		}
 	});
@@ -88,8 +88,8 @@ test('custom retries', withServer, async (t, server, got) => {
 	const error = await t.throwsAsync(got({
 		throwHttpErrors: true,
 		retry: {
-			retries: iteration => {
-				if (iteration === 1) {
+			calculateDelay: ({attemptCount}) => {
+				if (attemptCount === 1) {
 					tried = true;
 					return 1;
 				}
@@ -293,7 +293,7 @@ test('retry function can throw', withServer, async (t, server, got) => {
 	const error = 'Simple error';
 	await t.throwsAsync(got({
 		retry: {
-			retries: () => {
+			calculateDelay: () => {
 				throw new Error(error);
 			}
 		}

test/timeout.ts

@@ -320,7 +320,7 @@ test('retries on timeout', withServer, async (t, server, got) => {
 	await t.throwsAsync(got({
 		timeout: 1,
 		retry: {
-			retries: () => {
+			calculateDelay: () => {
 				if (tried) {
 					return 0;
 				}