From 52f9370c94473218f6ad5f17e996acf91820d1be Mon Sep 17 00:00:00 2001 From: isaacs Date: Tue, 10 Jan 2023 10:06:31 -0800 Subject: [PATCH] tests for retry-busy behavior --- README.md | 6 +- lib/retry-busy.js | 15 ++- tap-snapshots/test/retry-busy.js.test.cjs | 19 ++++ test/retry-busy.js | 110 ++++++++++++++++++++++ 4 files changed, 144 insertions(+), 6 deletions(-) create mode 100644 tap-snapshots/test/retry-busy.js.test.cjs create mode 100644 test/retry-busy.js diff --git a/README.md b/README.md index fc71619d..9599a038 100644 --- a/README.md +++ b/README.md @@ -37,9 +37,11 @@ Options: - `backoff`: Windows only. Rate of exponential backoff for async removal in case of `EBUSY`, `EMFILE`, and `ENFILE` errors. Should be a number greater than 1. Default `1.2` -- `maxBackoff`: Windows only. Maximum backoff time in ms to +- `maxBackoff`: Windows only. Maximum total backoff time in ms to attempt asynchronous retries in case of `EBUSY`, `EMFILE`, and - `ENFILE` errors. Default `100` + `ENFILE` errors. Default `200`. With the default `1.2` backoff + rate, this results in 14 retries, with the final retry being + delayed 33ms. Any other options are provided to the native Node.js `fs.rm` implementation when that is used. diff --git a/lib/retry-busy.js b/lib/retry-busy.js index 03cf8bf1..af7286a4 100644 --- a/lib/retry-busy.js +++ b/lib/retry-busy.js @@ -1,10 +1,12 @@ -const MAXBACKOFF = 100 +// note: max backoff is the maximum that any *single* backoff will do +// +const MAXBACKOFF = 200 const RATE = 1.2 const MAXRETRIES = 10 const codes = new Set(['EMFILE', 'ENFILE', 'EBUSY']) const retryBusy = fn => { - const method = async (path, opt, backoff = 1) => { + const method = async (path, opt, backoff = 1, total = 0) => { const mbo = opt.maxBackoff || MAXBACKOFF const rate = opt.backoff || RATE const max = opt.retries || MAXRETRIES @@ -15,10 +17,11 @@ const retryBusy = fn => { } catch (er) { if (codes.has(er.code)) { backoff = Math.ceil(backoff * rate) - if (backoff < mbo) { + total = backoff + total + if (total < mbo) { return new Promise((res, rej) => { setTimeout(() => { - method(path, opt, backoff).then(res, rej) + method(path, opt, backoff, total).then(res, rej) }, backoff) }) } @@ -56,6 +59,10 @@ const retryBusySync = fn => { } module.exports = { + MAXBACKOFF, + RATE, + MAXRETRIES, + codes, retryBusy, retryBusySync, } diff --git a/tap-snapshots/test/retry-busy.js.test.cjs b/tap-snapshots/test/retry-busy.js.test.cjs new file mode 100644 index 00000000..90e77bc0 --- /dev/null +++ b/tap-snapshots/test/retry-busy.js.test.cjs @@ -0,0 +1,19 @@ +/* IMPORTANT + * This snapshot file is auto-generated, but designed for humans. + * It should be checked into source control and tracked carefully. + * Re-generate by setting TAP_SNAPSHOT=1 and running tests. + * Make sure to inspect the output below. Do not ignore changes! + */ +'use strict' +exports[`test/retry-busy.js TAP > default settings 1`] = ` +Object { + "codes": Set { + "EMFILE", + "ENFILE", + "EBUSY", + }, + "MAXBACKOFF": 200, + "MAXRETRIES": 10, + "RATE": 1.2, +} +` diff --git a/test/retry-busy.js b/test/retry-busy.js new file mode 100644 index 00000000..a16f7031 --- /dev/null +++ b/test/retry-busy.js @@ -0,0 +1,110 @@ +const { + retryBusy, + retryBusySync, + MAXBACKOFF, + RATE, + MAXRETRIES, + codes, +} = require('../lib/retry-busy.js') + +const t = require('tap') + +t.matchSnapshot( + { + MAXBACKOFF, + RATE, + MAXRETRIES, + codes, + }, + 'default settings' +) + +t.test('basic working operation when no errors happen', async t => { + let calls = 0 + const arg = {} + const opt = {} + const method = (a, b) => { + t.equal(a, arg, 'got first argument') + t.equal(b, undefined, 'did not get another argument') + calls++ + } + const rBS = retryBusySync(method) + rBS(arg, opt) + t.equal(calls, 1) + const rB = retryBusy(method) + await rB(arg, opt).then(() => t.equal(calls, 2)) +}) + +t.test('retry when known error code thrown', t => { + t.plan(codes.size) + + for (const code of codes) { + t.test(code, async t => { + let thrown = false + let calls = 0 + const arg = {} + const opt = {} + const method = (a, b) => { + t.equal(a, arg, 'got first argument') + t.equal(b, undefined, 'did not get another argument') + if (!thrown) { + thrown = true + t.equal(calls, 0, 'first call') + calls++ + throw Object.assign(new Error(code), { code }) + } else { + t.equal(calls, 1, 'second call') + calls++ + thrown = false + } + } + const rBS = retryBusySync(method) + rBS(arg, opt) + t.equal(calls, 2) + calls = 0 + const rB = retryBusy(method) + await rB(arg, opt).then(() => t.equal(calls, 2)) + }) + } +}) + +t.test('retry and eventually give up', t => { + t.plan(codes.size) + const opt = { + maxBackoff: 2, + retries: 2, + } + + for (const code of codes) { + t.test(code, async t => { + let calls = 0 + const arg = {} + const method = (a, b) => { + t.equal(a, arg, 'got first argument') + t.equal(b, undefined, 'did not get another argument') + calls++ + throw Object.assign(new Error(code), { code }) + } + const rBS = retryBusySync(method) + t.throws(() => rBS(arg, opt), { code }) + t.equal(calls, 3) + calls = 0 + const rB = retryBusy(method) + await t.rejects(rB(arg, opt)).then(() => t.equal(calls, 3)) + }) + } +}) + +t.test('throw unknown error gives up right away', async t => { + const arg = {} + const opt = {} + const method = (a, b) => { + t.equal(a, arg, 'got first argument') + t.equal(b, undefined, 'did not get another argument') + throw Object.assign(new Error('nope'), { code: 'nope' }) + } + const rBS = retryBusySync(method) + t.throws(() => rBS(arg, opt), { code: 'nope' }) + const rB = retryBusy(method) + await t.rejects(rB(arg, opt), { code: 'nope' }) +})