From b15ce1dc16d04133dab74c6953d86fa1015ef673 Mon Sep 17 00:00:00 2001 From: Szymon Marczak <36894700+szmarczak@users.noreply.github.com> Date: Sun, 8 Sep 2019 18:42:04 +0200 Subject: [PATCH] Don't break the default behavior when using custom retry function (#830) --- readme.md | 7 ++-- source/as-promise.ts | 2 +- source/as-stream.ts | 2 +- source/calculate-retry-delay.ts | 48 +++++++++++++++++++++++++++ source/index.ts | 2 +- source/normalize-arguments.ts | 53 +++--------------------------- source/request-as-event-emitter.ts | 15 ++++++++- source/utils/types.ts | 19 ++++++++--- test/retry.ts | 12 +++---- test/timeout.ts | 2 +- 10 files changed, 94 insertions(+), 68 deletions(-) create mode 100644 source/calculate-retry-delay.ts diff --git a/readme.md b/readme.md index f98a435f2..b8b8918c7 100644 --- a/readme.md +++ b/readme.md @@ -347,20 +347,21 @@ This also accepts an `object` with the following fields to constrain the duratio Type: `number | object`
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`.
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. diff --git a/source/as-promise.ts b/source/as-promise.ts index 08448c60d..b21d9e06a 100644 --- a/source/as-promise.ts +++ b/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', diff --git a/source/as-stream.ts b/source/as-stream.ts index 32b20b99c..7d06e531e 100644 --- a/source/as-stream.ts +++ b/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 = () => { diff --git a/source/calculate-retry-delay.ts b/source/calculate-retry-delay.ts new file mode 100644 index 000000000..9195c4136 --- /dev/null +++ b/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 = 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; diff --git a/source/index.ts b/source/index.ts index 05a555532..47c63edfb 100644 --- a/source/index.ts +++ b/source/index.ts @@ -6,7 +6,7 @@ const defaults: Partial = { options: { method: 'GET', retry: { - retries: 2, + limit: 2, methods: [ 'GET', 'PUT', diff --git a/source/normalize-arguments.ts b/source/normalize-arguments.ts index e0e74944a..0a4f7a041 100644 --- a/source/normalize-arguments.ts +++ b/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 = new Set([413, 429, 503]); let hasShownDeprecation = false; @@ -80,7 +75,7 @@ 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(), @@ -88,12 +83,12 @@ export const preNormalizeArguments = (options: Options, defaults?: Options): Nor }; 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; }; diff --git a/source/request-as-event-emitter.ts b/source/request-as-event-emitter.ts index 8a9e55445..09d2b2f56 100644 --- a/source/request-as-event-emitter.ts +++ b/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; diff --git a/source/utils/types.ts b/source/utils/types.ts index a46a8b6ec..f10e785ab 100644 --- a/source/utils/types.ts +++ b/source/utils/types.ts @@ -67,12 +67,20 @@ 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 = >(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[]; @@ -80,7 +88,8 @@ export interface RetryOption { } export interface NormalizedRetryOptions { - retries: RetryFunction; + limit: number; + calculateDelay: RetryFunction; methods: ReadonlySet; statusCodes: ReadonlySet; errorCodes: ReadonlySet; @@ -121,7 +130,7 @@ export interface Options extends Omit; + retry?: number | Partial; throwHttpErrors?: boolean; cookieJar?: CookieJar; ignoreInvalidCookies?: boolean; diff --git a/test/retry.ts b/test/retry.ts index 6d4b44e7c..4840dbff7 100644 --- a/test/retry.ts +++ b/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); } } diff --git a/test/timeout.ts b/test/timeout.ts index 2193864b7..980df059f 100644 --- a/test/timeout.ts +++ b/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; }