From 2c8a2cb1f27fa0587c35d6c6d17b5fae6aff1356 Mon Sep 17 00:00:00 2001 From: Frank van Wijk Date: Wed, 14 Sep 2022 16:11:08 +0200 Subject: [PATCH 01/14] Add retry option to configure backoff limit The default backoff function is exponential, so after a couple of retries the delay will become very large. In case you are polling an api, you want at least that there is an upper limit to this delay, for example 1000ms. --- source/core/Ky.ts | 2 +- source/types/retry.ts | 12 ++++++++++++ source/utils/normalize.ts | 1 + 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/source/core/Ky.ts b/source/core/Ky.ts index 1729e8ae..2ad0bef4 100644 --- a/source/core/Ky.ts +++ b/source/core/Ky.ts @@ -226,7 +226,7 @@ export class Ky { } const BACKOFF_FACTOR = 0.3; - return BACKOFF_FACTOR * (2 ** (this._retryCount - 1)) * 1000; + return Math.min(this._options.retry.backoffLimit, BACKOFF_FACTOR * (2 ** (this._retryCount - 1)) * 1000); } return 0; diff --git a/source/types/retry.ts b/source/types/retry.ts index 6449168e..e315c044 100644 --- a/source/types/retry.ts +++ b/source/types/retry.ts @@ -33,4 +33,16 @@ export interface RetryOptions { @default Infinity */ maxRetryAfter?: number; + + /** + The upper limit of the `computedValue`. + + By default, the computedValue is calculated in the following way: + + 0.3 * (2 ** (attemptCount - 1)) * 1000 + + The delay increases exponentially. + In order to prevent this, you can set this value to a fixed value, such as 1000. + */ + backoffLimit?: number; } diff --git a/source/utils/normalize.ts b/source/utils/normalize.ts index 9f8c9dbb..504c575d 100644 --- a/source/utils/normalize.ts +++ b/source/utils/normalize.ts @@ -17,6 +17,7 @@ const defaultRetryOptions: Required = { statusCodes: retryStatusCodes, afterStatusCodes: retryAfterStatusCodes, maxRetryAfter: Number.POSITIVE_INFINITY, + backoffLimit: Number.POSITIVE_INFINITY, }; export const normalizeRetryOptions = (retry: number | RetryOptions = {}): Required => { From 4d9bfe02c9dfc58c9c38949f320e9e45db3d8d27 Mon Sep 17 00:00:00 2001 From: Frank van Wijk Date: Wed, 14 Sep 2022 21:41:14 +0200 Subject: [PATCH 02/14] Add test for retry backoffLimit --- test/retry.ts | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/test/retry.ts b/test/retry.ts index 5b59655f..9744752a 100644 --- a/test/retry.ts +++ b/test/retry.ts @@ -440,3 +440,50 @@ test('throws when retry.statusCodes is not an array', async t => { await server.close(); }); + +test('respect maximum backoff', async t => { + const retryCount = 6; + let requestCount = 0; + + const server = await createHttpTestServer(); + server.get('/', (_request, response) => { + requestCount++; + + if (requestCount === retryCount) { + response.end(fixture); + } else { + response.sendStatus(500); + } + }); + + 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, + }, + }).text(), fixture); + performance.mark('end-custom'); + + performance.measure('default', 'start', 'end'); + performance.measure('custom', 'start-custom', 'end-custom'); + + const measurements = performance.getEntriesByType('measure'); + + const duration = measurements.at(0)?.duration ?? Number.NaN; + const expectedDuration = 300 + 600 + 1200 + 2400 + 4800; + t.true(Math.abs(duration - expectedDuration) < 100, `Duration of ${duration} is not close to expected duration ${expectedDuration}`); // Allow for 100ms difference + + const customDuration = measurements.at(1)?.duration ?? Number.NaN; + const expectedCustomDuration = 300 + 600 + 1000 + 1000 + 1000; + t.true(Math.abs(customDuration - expectedCustomDuration) < 100, `Duration of ${customDuration}ms is not close to expected duration ${expectedCustomDuration}ms`); // Allow for 100ms difference + + await server.close(); +}); From d113860ff46d1f0477e79dcc3f52d4d4bad09fec Mon Sep 17 00:00:00 2001 From: Frank van Wijk Date: Thu, 15 Sep 2022 11:25:18 +0200 Subject: [PATCH 03/14] Attempt to remove flakynes --- test/retry.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/retry.ts b/test/retry.ts index 9744752a..d62447ea 100644 --- a/test/retry.ts +++ b/test/retry.ts @@ -479,11 +479,11 @@ test('respect maximum backoff', async t => { const duration = measurements.at(0)?.duration ?? Number.NaN; const expectedDuration = 300 + 600 + 1200 + 2400 + 4800; - t.true(Math.abs(duration - expectedDuration) < 100, `Duration of ${duration} is not close to expected duration ${expectedDuration}`); // Allow for 100ms difference + t.true(Math.abs(duration - expectedDuration) < 300, `Duration of ${duration} is not close to expected duration ${expectedDuration}`); // Allow for 300ms difference const customDuration = measurements.at(1)?.duration ?? Number.NaN; const expectedCustomDuration = 300 + 600 + 1000 + 1000 + 1000; - t.true(Math.abs(customDuration - expectedCustomDuration) < 100, `Duration of ${customDuration}ms is not close to expected duration ${expectedCustomDuration}ms`); // Allow for 100ms difference + t.true(Math.abs(customDuration - expectedCustomDuration) < 300, `Duration of ${customDuration}ms is not close to expected duration ${expectedCustomDuration}ms`); // Allow for 300ms difference await server.close(); }); From 9b9cd409dbce63c4e7e350a0b73337f68bebadb6 Mon Sep 17 00:00:00 2001 From: Frank van Wijk Date: Fri, 23 Sep 2022 13:54:32 +0200 Subject: [PATCH 04/14] Import performance --- test/retry.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/test/retry.ts b/test/retry.ts index d62447ea..04040a56 100644 --- a/test/retry.ts +++ b/test/retry.ts @@ -1,4 +1,5 @@ import test from 'ava'; +import {performance} from 'perf_hooks'; import ky from '../source/index.js'; import {createHttpTestServer} from './helpers/create-http-test-server.js'; From 0a18e4c7669fc2193437ff86c1147cbd424267fb Mon Sep 17 00:00:00 2001 From: Frank van Wijk Date: Fri, 23 Sep 2022 13:59:12 +0200 Subject: [PATCH 05/14] Describe the option in higher-level terms --- source/types/retry.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/source/types/retry.ts b/source/types/retry.ts index e315c044..283f950f 100644 --- a/source/types/retry.ts +++ b/source/types/retry.ts @@ -35,14 +35,14 @@ export interface RetryOptions { maxRetryAfter?: number; /** - The upper limit of the `computedValue`. + The upper limit of the delay per retry in ms. - By default, the computedValue is calculated in the following way: + By default, the delay is calculated in the following way: 0.3 * (2 ** (attemptCount - 1)) * 1000 The delay increases exponentially. In order to prevent this, you can set this value to a fixed value, such as 1000. - */ + */ backoffLimit?: number; } From 608d6b2237a05944e7b134227e24359af5beb11f Mon Sep 17 00:00:00 2001 From: Sindre Sorhus Date: Sat, 24 Sep 2022 21:16:10 +0700 Subject: [PATCH 06/14] Update retry.ts --- source/types/retry.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/source/types/retry.ts b/source/types/retry.ts index 283f950f..2807e388 100644 --- a/source/types/retry.ts +++ b/source/types/retry.ts @@ -35,13 +35,16 @@ export interface RetryOptions { maxRetryAfter?: number; /** - The upper limit of the delay per retry in ms. + The upper limit of the delay per retry in milliseconds. By default, the delay is calculated in the following way: + ``` 0.3 * (2 ** (attemptCount - 1)) * 1000 + ``` The delay increases exponentially. + In order to prevent this, you can set this value to a fixed value, such as 1000. */ backoffLimit?: number; From 6a45899189ec0d1ea9e6872eac3d34b937003c01 Mon Sep 17 00:00:00 2001 From: Frank van Wijk Date: Wed, 12 Oct 2022 14:23:59 +0200 Subject: [PATCH 07/14] Fix xo error --- test/retry.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/retry.ts b/test/retry.ts index 04040a56..9c3a021c 100644 --- a/test/retry.ts +++ b/test/retry.ts @@ -1,5 +1,5 @@ +import {performance} from 'node:perf_hooks'; import test from 'ava'; -import {performance} from 'perf_hooks'; import ky from '../source/index.js'; import {createHttpTestServer} from './helpers/create-http-test-server.js'; From 4471cab7371a2d0afce978e2ad0022daa8c1b1ca Mon Sep 17 00:00:00 2001 From: Frank van Wijk Date: Wed, 12 Oct 2022 14:33:46 +0200 Subject: [PATCH 08/14] Add the backoffLimit option to the docs --- readme.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/readme.md b/readme.md index ef253248..9852f2f4 100644 --- a/readme.md +++ b/readme.md @@ -169,6 +169,7 @@ Default: - `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` +- `backoffLimit`: `undefined` 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. @@ -176,7 +177,7 @@ If `retry` is a number, it will be used as `limit` and other defaults will remai 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). +Delays between retries is calculated with the function `0.3 * (2 ** (retry - 1)) * 1000`, where `retry` is the attempt number (starts from 1). If `backoffLimit` is set, it will maximize the delay to that number so that it won't increase further after many retries. Retries are not triggered following a [timeout](#timeout). @@ -187,7 +188,8 @@ const json = await ky('https://example.com', { retry: { limit: 10, methods: ['get'], - statusCodes: [413] + statusCodes: [413], + backoffLimit: 3000 } }).json(); ``` From f5ad03244d14eb545c1db3b39b6ad125eaa44395 Mon Sep 17 00:00:00 2001 From: Frank van Wijk Date: Wed, 12 Oct 2022 14:40:52 +0200 Subject: [PATCH 09/14] Variable timing offset based on CI env var --- test/retry.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/test/retry.ts b/test/retry.ts index 9c3a021c..d37d78f5 100644 --- a/test/retry.ts +++ b/test/retry.ts @@ -1,4 +1,5 @@ import {performance} 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'; @@ -471,6 +472,10 @@ test('respect maximum backoff', async t => { backoffLimit: 1000, }, }).text(), fixture); + + // We allow the test to take more time on CI than locally, to reduce flakiness + const allowedOffset = process.env.CI ? 1000 : 300; + performance.mark('end-custom'); performance.measure('default', 'start', 'end'); @@ -480,11 +485,11 @@ test('respect maximum backoff', async t => { const duration = measurements.at(0)?.duration ?? Number.NaN; const expectedDuration = 300 + 600 + 1200 + 2400 + 4800; - t.true(Math.abs(duration - expectedDuration) < 300, `Duration of ${duration} is not close to expected duration ${expectedDuration}`); // Allow for 300ms difference + t.true(Math.abs(duration - expectedDuration) < allowedOffset, `Duration of ${duration} is not close to expected duration ${expectedDuration}`); // Allow for 300ms difference const customDuration = measurements.at(1)?.duration ?? Number.NaN; const expectedCustomDuration = 300 + 600 + 1000 + 1000 + 1000; - t.true(Math.abs(customDuration - expectedCustomDuration) < 300, `Duration of ${customDuration}ms is not close to expected duration ${expectedCustomDuration}ms`); // Allow for 300ms difference + t.true(Math.abs(customDuration - expectedCustomDuration) < allowedOffset, `Duration of ${customDuration}ms is not close to expected duration ${expectedCustomDuration}ms`); // Allow for 300ms difference await server.close(); }); From ef32bea422ee44c052c6f1fb6f348120550d3d04 Mon Sep 17 00:00:00 2001 From: Frank van Wijk Date: Thu, 20 Oct 2022 09:22:49 +0200 Subject: [PATCH 10/14] Rephrase documentation --- readme.md | 3 ++- source/types/retry.ts | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/readme.md b/readme.md index 9852f2f4..ec23159d 100644 --- a/readme.md +++ b/readme.md @@ -177,7 +177,8 @@ If `retry` is a number, it will be used as `limit` and other defaults will remai 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). If `backoffLimit` is set, it will maximize the delay to that number so that it won't increase further after many retries. +By default, the delay is calculated with `0.3 * (2 ** (attemptCount - 1)) * 1000`. The delay increases exponentially. +To clamp the delay, set `backoffLimit` to 1000, for example. Retries are not triggered following a [timeout](#timeout). diff --git a/source/types/retry.ts b/source/types/retry.ts index 2807e388..f6a0481a 100644 --- a/source/types/retry.ts +++ b/source/types/retry.ts @@ -45,7 +45,7 @@ export interface RetryOptions { The delay increases exponentially. - In order to prevent this, you can set this value to a fixed value, such as 1000. + To clamp the delay, set `backoffLimit` to 1000, for example. */ backoffLimit?: number; } From af922baf798d8ce35d376d5e12c98529f49cd404 Mon Sep 17 00:00:00 2001 From: Frank van Wijk Date: Thu, 20 Oct 2022 10:34:48 +0200 Subject: [PATCH 11/14] Reimplement performance measurement using Node 14 API --- test/retry.ts | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/test/retry.ts b/test/retry.ts index d37d78f5..265e2a48 100644 --- a/test/retry.ts +++ b/test/retry.ts @@ -1,4 +1,4 @@ -import {performance} from 'node:perf_hooks'; +import {performance, PerformanceObserver} from 'node:perf_hooks'; import process from 'node:process'; import test from 'ava'; import ky from '../source/index.js'; @@ -458,6 +458,21 @@ 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(); + + const duration = measurements[0].duration ?? Number.NaN; + const expectedDuration = {default: 300 + 600 + 1200 + 2400 + 4800, custom: 300 + 600 + 1000 + 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 + }); + obs.observe({entryTypes: ['measure']}); + + // Start measuring performance.mark('start'); t.is(await ky(server.url, { retry: retryCount, @@ -473,23 +488,10 @@ test('respect maximum backoff', async t => { }, }).text(), fixture); - // We allow the test to take more time on CI than locally, to reduce flakiness - const allowedOffset = process.env.CI ? 1000 : 300; - performance.mark('end-custom'); performance.measure('default', 'start', 'end'); performance.measure('custom', 'start-custom', 'end-custom'); - const measurements = performance.getEntriesByType('measure'); - - const duration = measurements.at(0)?.duration ?? Number.NaN; - const expectedDuration = 300 + 600 + 1200 + 2400 + 4800; - t.true(Math.abs(duration - expectedDuration) < allowedOffset, `Duration of ${duration} is not close to expected duration ${expectedDuration}`); // Allow for 300ms difference - - const customDuration = measurements.at(1)?.duration ?? Number.NaN; - const expectedCustomDuration = 300 + 600 + 1000 + 1000 + 1000; - t.true(Math.abs(customDuration - expectedCustomDuration) < allowedOffset, `Duration of ${customDuration}ms is not close to expected duration ${expectedCustomDuration}ms`); // Allow for 300ms difference - await server.close(); }); From 3ac523fbf998c1699f8bbdf7e80b10f8d0e9bd5c Mon Sep 17 00:00:00 2001 From: Frank van Wijk Date: Thu, 20 Oct 2022 15:21:33 +0200 Subject: [PATCH 12/14] Cleanup performance observer when we're done --- test/retry.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/retry.ts b/test/retry.ts index 265e2a48..1ed880c3 100644 --- a/test/retry.ts +++ b/test/retry.ts @@ -469,6 +469,10 @@ test('respect maximum backoff', async t => { const expectedDuration = {default: 300 + 600 + 1200 + 2400 + 4800, custom: 300 + 600 + 1000 + 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(); + } }); obs.observe({entryTypes: ['measure']}); From 945a07f55638d9fdca435c59f9c9dadfc9cff18c Mon Sep 17 00:00:00 2001 From: Frank van Wijk Date: Thu, 20 Oct 2022 15:21:52 +0200 Subject: [PATCH 13/14] Prevent potential timeout by retrying less often --- .vscode/settings.json | 3 +++ test/retry.ts | 6 +++--- 2 files changed, 6 insertions(+), 3 deletions(-) create mode 100644 .vscode/settings.json diff --git a/.vscode/settings.json b/.vscode/settings.json new file mode 100644 index 00000000..8d9c9bc0 --- /dev/null +++ b/.vscode/settings.json @@ -0,0 +1,3 @@ +{ + "editor.formatOnSave": false +} \ No newline at end of file diff --git a/test/retry.ts b/test/retry.ts index 1ed880c3..5dfefc27 100644 --- a/test/retry.ts +++ b/test/retry.ts @@ -444,7 +444,7 @@ test('throws when retry.statusCodes is not an array', async t => { }); test('respect maximum backoff', async t => { - const retryCount = 6; + const retryCount = 5; let requestCount = 0; const server = await createHttpTestServer(); @@ -466,11 +466,11 @@ test('respect maximum backoff', async t => { const measurements = items.getEntries(); const duration = measurements[0].duration ?? Number.NaN; - const expectedDuration = {default: 300 + 600 + 1200 + 2400 + 4800, custom: 300 + 600 + 1000 + 1000 + 1000}[measurements[0].name] ?? 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') { + if (measurements[0].name === 'custom') { obs.disconnect(); } }); From c90d1d74c6c8e5f93c23493c431371666dc6b5a3 Mon Sep 17 00:00:00 2001 From: Frank van Wijk Date: Mon, 31 Oct 2022 09:05:48 +0100 Subject: [PATCH 14/14] Tweak docs wording a bit more --- .vscode/settings.json | 3 --- readme.md | 3 ++- source/types/retry.ts | 3 ++- 3 files changed, 4 insertions(+), 5 deletions(-) delete mode 100644 .vscode/settings.json diff --git a/.vscode/settings.json b/.vscode/settings.json deleted file mode 100644 index 8d9c9bc0..00000000 --- a/.vscode/settings.json +++ /dev/null @@ -1,3 +0,0 @@ -{ - "editor.formatOnSave": false -} \ No newline at end of file diff --git a/readme.md b/readme.md index ec23159d..c428fa41 100644 --- a/readme.md +++ b/readme.md @@ -177,8 +177,9 @@ If `retry` is a number, it will be used as `limit` and other defaults will remai 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, the delay is calculated with `0.3 * (2 ** (attemptCount - 1)) * 1000`. The delay increases exponentially. +The `backoffLimit` option is the upper limit of the delay per retry in milliseconds. 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. Retries are not triggered following a [timeout](#timeout). diff --git a/source/types/retry.ts b/source/types/retry.ts index f6a0481a..746a1ec6 100644 --- a/source/types/retry.ts +++ b/source/types/retry.ts @@ -36,6 +36,7 @@ export interface RetryOptions { /** The upper limit of the delay per retry in milliseconds. + To clamp the delay, set `backoffLimit` to 1000, for example. By default, the delay is calculated in the following way: @@ -45,7 +46,7 @@ export interface RetryOptions { The delay increases exponentially. - To clamp the delay, set `backoffLimit` to 1000, for example. + @default Infinity */ backoffLimit?: number; }