From 6804448b3dabd0ff01b7f325abeb6785d6de0d08 Mon Sep 17 00:00:00 2001 From: rclarey Date: Fri, 29 Sep 2023 16:28:12 +0900 Subject: [PATCH 1/6] add retry.delay option to control delay --- source/core/Ky.ts | 4 ++-- source/types/options.ts | 2 +- source/types/retry.ts | 7 ++++++ source/utils/normalize.ts | 1 + test/retry.ts | 48 ++++++++++++++++++++++++++++++++++++--- 5 files changed, 56 insertions(+), 6 deletions(-) diff --git a/source/core/Ky.ts b/source/core/Ky.ts index f45d1011..a987f032 100644 --- a/source/core/Ky.ts +++ b/source/core/Ky.ts @@ -234,8 +234,8 @@ export class Ky { } } - const BACKOFF_FACTOR = 0.3; - return Math.min(this._options.retry.backoffLimit, BACKOFF_FACTOR * (2 ** (this._retryCount - 1)) * 1000); + const retryDelay = this._options.retry.delay(this._retryCount); + return Math.min(this._options.retry.backoffLimit, retryDelay); } return 0; diff --git a/source/types/options.ts b/source/types/options.ts index 0b4c116a..6bb04bee 100644 --- a/source/types/options.ts +++ b/source/types/options.ts @@ -138,7 +138,7 @@ export interface Options extends Omit { // eslint-disabl 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 is calculated with the function `0.3 * (2 ** (retry - 1)) * 1000`, where `retry` is the attempt number (starts from 1). + By default delays between retries are calculated with the function `0.3 * (2 ** (retry - 1)) * 1000`, where `retry` is the attempt number (starts from 1), however this can be changed by passing a `delay` function. Retries are not triggered following a timeout. diff --git a/source/types/retry.ts b/source/types/retry.ts index f25b664e..16a16e30 100644 --- a/source/types/retry.ts +++ b/source/types/retry.ts @@ -49,4 +49,11 @@ export type RetryOptions = { @default Infinity */ backoffLimit?: number; + + /** + A function to calculate the delay between retries given `attemptCount` (starts from 1). + + @default attemptCount => 0.3 * (2 ** (attemptCount - 1)) * 1000 + */ + delay?: (attemptCount: number) => number; }; diff --git a/source/utils/normalize.ts b/source/utils/normalize.ts index 504c575d..8376f641 100644 --- a/source/utils/normalize.ts +++ b/source/utils/normalize.ts @@ -18,6 +18,7 @@ const defaultRetryOptions: Required = { afterStatusCodes: retryAfterStatusCodes, maxRetryAfter: Number.POSITIVE_INFINITY, backoffLimit: Number.POSITIVE_INFINITY, + delay: attemptCount => 0.3 * (2 ** (attemptCount - 1)) * 1000, }; export const normalizeRetryOptions = (retry: number | RetryOptions = {}): Required => { diff --git a/test/retry.ts b/test/retry.ts index ef94f36a..edc72c13 100644 --- a/test/retry.ts +++ b/test/retry.ts @@ -8,6 +8,8 @@ const fixture = 'fixture'; const defaultRetryCount = 2; const retryAfterOn413 = 2; const lastTried413access = Date.now(); +// We allow the tests to take more time on CI than locally, to reduce flakiness +const allowedOffset = process.env.CI ? 1000 : 300; test('network error', async t => { let requestCount = 0; @@ -458,9 +460,6 @@ test('respect maximum backoff', async t => { } }); - // We allow the test to take more time on CI than locally, to reduce flakiness - const allowedOffset = process.env.CI ? 1000 : 300; - // Register observer that asserts on duration when a measurement is performed const obs = new PerformanceObserver(items => { const measurements = items.getEntries(); @@ -499,3 +498,46 @@ test('respect maximum backoff', async t => { await server.close(); }); + +test('respect custom retry.delay', async t => { + const retryCount = 5; + let requestCount = 0; + + const server = await createHttpTestServer(); + server.get('/', (_request, response) => { + requestCount++; + + if (requestCount === retryCount) { + response.end(fixture); + } else { + response.sendStatus(500); + } + }); + + // Register observer that asserts on duration when a measurement is performed + const obs = new PerformanceObserver(items => { + const measurements = items.getEntries(); + + const duration = measurements[0].duration ?? Number.NaN; + const expectedDuration = 200 + 300 + 400 + 500; + + t.true(Math.abs(duration - expectedDuration) < allowedOffset, `Duration of ${duration}ms is not close to expected duration ${expectedDuration}ms`); + + obs.disconnect(); + }); + obs.observe({entryTypes: ['measure']}); + + // Start measuring + performance.mark('start'); + t.is(await ky(server.url, { + retry: { + limit: retryCount, + delay: n => 100 * (n + 1), + }, + }).text(), fixture); + performance.mark('end'); + + performance.measure('linear', 'start', 'end'); + + await server.close(); +}); From 37fb68ea8e1267991767f0751ca0075edf008049 Mon Sep 17 00:00:00 2001 From: Sindre Sorhus Date: Sun, 15 Oct 2023 18:38:28 +0700 Subject: [PATCH 2/6] Update retry.ts --- test/retry.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/test/retry.ts b/test/retry.ts index edc72c13..8d143572 100644 --- a/test/retry.ts +++ b/test/retry.ts @@ -8,6 +8,7 @@ const fixture = 'fixture'; const defaultRetryCount = 2; const retryAfterOn413 = 2; const lastTried413access = Date.now(); + // We allow the tests to take more time on CI than locally, to reduce flakiness const allowedOffset = process.env.CI ? 1000 : 300; From aa4433478082f6101bc0d8a9b0398a5b9ceff311 Mon Sep 17 00:00:00 2001 From: Russell Clarey Date: Mon, 16 Oct 2023 10:17:33 +0900 Subject: [PATCH 3/6] Refine doc comment Co-authored-by: Sindre Sorhus --- source/types/options.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/types/options.ts b/source/types/options.ts index 6bb04bee..4855d48e 100644 --- a/source/types/options.ts +++ b/source/types/options.ts @@ -138,7 +138,7 @@ export interface Options extends Omit { // eslint-disabl 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. - By default delays between retries are calculated with the function `0.3 * (2 ** (retry - 1)) * 1000`, where `retry` is the attempt number (starts from 1), however this can be changed by passing a `delay` function. + By default, delays between retries are calculated with the function `0.3 * (2 ** (attemptCount - 1)) * 1000`, where `attemptCount` is the attempt number (starts from 1), however this can be changed by passing a `delay` function. Retries are not triggered following a timeout. From 5a782990b54a44240f148ce3e51872a955e014fe Mon Sep 17 00:00:00 2001 From: rclarey Date: Mon, 16 Oct 2023 12:14:48 +0900 Subject: [PATCH 4/6] create performance observer helper for tests --- test/helpers/with-performance-observer.ts | 42 ++++++++++ test/retry.ts | 93 +++++++++-------------- 2 files changed, 76 insertions(+), 59 deletions(-) create mode 100644 test/helpers/with-performance-observer.ts diff --git a/test/helpers/with-performance-observer.ts b/test/helpers/with-performance-observer.ts new file mode 100644 index 00000000..6d28ebfa --- /dev/null +++ b/test/helpers/with-performance-observer.ts @@ -0,0 +1,42 @@ +import {performance, PerformanceObserver} from 'node:perf_hooks'; +import process from 'node:process'; +import type {ExecutionContext} from 'ava'; + +type Arg = { + name: string; + expectedDuration: number; + t: ExecutionContext; + test: () => Promise; +}; + +// We allow the tests to take more time on CI than locally, to reduce flakiness +const allowedOffset = process.env.CI ? 1000 : 300; + +export async function withPerformanceObserver({ + name, + expectedDuration, + t, + test, +}: Arg) { + // Register observer that asserts on duration when a measurement is performed + const obs = new PerformanceObserver(items => { + const measurements = items.getEntries(); + + const duration = measurements[0].duration ?? Number.NaN; + + t.true( + Math.abs(duration - expectedDuration) < allowedOffset, + `Duration of ${duration}ms is not close to expected duration ${expectedDuration}ms`, + ); + + obs.disconnect(); + }); + obs.observe({entryTypes: ['measure']}); + + // Start measuring + performance.mark(`start-${name}`); + await test(); + performance.mark(`end-${name}`); + + performance.measure(name, `start-${name}`, `end-${name}`); +} diff --git a/test/retry.ts b/test/retry.ts index 8d143572..5826c146 100644 --- a/test/retry.ts +++ b/test/retry.ts @@ -1,17 +1,13 @@ -import {performance, PerformanceObserver} from 'node:perf_hooks'; -import process from 'node:process'; import test from 'ava'; import ky from '../source/index.js'; import {createHttpTestServer} from './helpers/create-http-test-server.js'; +import {withPerformanceObserver} from './helpers/with-performance-observer.js'; const fixture = 'fixture'; const defaultRetryCount = 2; const retryAfterOn413 = 2; const lastTried413access = Date.now(); -// We allow the tests to take more time on CI than locally, to reduce flakiness -const allowedOffset = process.env.CI ? 1000 : 300; - test('network error', async t => { let requestCount = 0; @@ -461,41 +457,31 @@ test('respect maximum backoff', async t => { } }); - // Register observer that asserts on duration when a measurement is performed - const obs = new PerformanceObserver(items => { - const measurements = items.getEntries(); - - const duration = measurements[0].duration ?? Number.NaN; - const expectedDuration = {default: 300 + 600 + 1200 + 2400, custom: 300 + 600 + 1000 + 1000}[measurements[0].name] ?? Number.NaN; - - t.true(Math.abs(duration - expectedDuration) < allowedOffset, `Duration of ${duration}ms is not close to expected duration ${expectedDuration}ms`); // Allow for 300ms difference - - if (measurements[0].name === 'custom') { - obs.disconnect(); - } + await withPerformanceObserver({ + t, + name: 'default', + expectedDuration: 300 + 600 + 1200 + 2400, + async test() { + t.is(await ky(server.url, { + retry: retryCount, + }).text(), fixture); + }, }); - obs.observe({entryTypes: ['measure']}); - // Start measuring - performance.mark('start'); - t.is(await ky(server.url, { - retry: retryCount, - }).text(), fixture); - performance.mark('end'); - - performance.mark('start-custom'); requestCount = 0; - t.is(await ky(server.url, { - retry: { - limit: retryCount, - backoffLimit: 1000, + await withPerformanceObserver({ + t, + name: 'custom', + expectedDuration: 300 + 600 + 1000 + 1000, + async test() { + t.is(await ky(server.url, { + retry: { + limit: retryCount, + backoffLimit: 1000, + }, + }).text(), fixture); }, - }).text(), fixture); - - performance.mark('end-custom'); - - performance.measure('default', 'start', 'end'); - performance.measure('custom', 'start-custom', 'end-custom'); + }); await server.close(); }); @@ -515,30 +501,19 @@ test('respect custom retry.delay', async t => { } }); - // Register observer that asserts on duration when a measurement is performed - const obs = new PerformanceObserver(items => { - const measurements = items.getEntries(); - - const duration = measurements[0].duration ?? Number.NaN; - const expectedDuration = 200 + 300 + 400 + 500; - - t.true(Math.abs(duration - expectedDuration) < allowedOffset, `Duration of ${duration}ms is not close to expected duration ${expectedDuration}ms`); - - obs.disconnect(); - }); - obs.observe({entryTypes: ['measure']}); - - // Start measuring - performance.mark('start'); - t.is(await ky(server.url, { - retry: { - limit: retryCount, - delay: n => 100 * (n + 1), + await withPerformanceObserver({ + t, + name: 'linear', + expectedDuration: 200 + 300 + 400 + 500, + async test() { + t.is(await ky(server.url, { + retry: { + limit: retryCount, + delay: n => 100 * (n + 1), + }, + }).text(), fixture); }, - }).text(), fixture); - performance.mark('end'); - - performance.measure('linear', 'start', 'end'); + }); await server.close(); }); From 88b361caa8df9ac20af1885d6f9b5a8a69e46056 Mon Sep 17 00:00:00 2001 From: rclarey Date: Mon, 16 Oct 2023 12:18:17 +0900 Subject: [PATCH 5/6] update readme with delay option --- readme.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/readme.md b/readme.md index 594e4ffe..2c2f2e82 100644 --- a/readme.md +++ b/readme.md @@ -196,6 +196,7 @@ Default: - `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` - `backoffLimit`: `undefined` +- `delay`: `attemptCount => 0.3 * (2 ** (attemptCount - 1)) * 1000` An object representing `limit`, `methods`, `statusCodes` and `maxRetryAfter` fields for maximum retry count, allowed methods, allowed status codes and maximum [`Retry-After`](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Retry-After) time. @@ -207,6 +208,9 @@ The `backoffLimit` option is the upper limit of the delay per retry in milliseco To clamp the delay, set `backoffLimit` to 1000, for example. By default, the delay is calculated with `0.3 * (2 ** (attemptCount - 1)) * 1000`. The delay increases exponentially. +The `delay` option can be used to change how the delay between retries is calculated. The function receives one parameter, +the attempt count, starting at `1`. + Retries are not triggered following a [timeout](#timeout). ```js From 39f30774cdd458d3507c9f49c5c9cbe5fd7f6f3a Mon Sep 17 00:00:00 2001 From: Sindre Sorhus Date: Tue, 17 Oct 2023 01:56:51 +0700 Subject: [PATCH 6/6] Update readme.md --- readme.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/readme.md b/readme.md index 2c2f2e82..7ec0aa6e 100644 --- a/readme.md +++ b/readme.md @@ -208,8 +208,7 @@ The `backoffLimit` option is the upper limit of the delay per retry in milliseco To clamp the delay, set `backoffLimit` to 1000, for example. By default, the delay is calculated with `0.3 * (2 ** (attemptCount - 1)) * 1000`. The delay increases exponentially. -The `delay` option can be used to change how the delay between retries is calculated. The function receives one parameter, -the attempt count, starting at `1`. +The `delay` option can be used to change how the delay between retries is calculated. The function receives one parameter, the attempt count, starting at `1`. Retries are not triggered following a [timeout](#timeout).