From f9d8494410ab8ebc53dcf938395ca17a9a864bfc Mon Sep 17 00:00:00 2001 From: cjihrig Date: Mon, 25 Nov 2019 16:43:59 -0500 Subject: [PATCH] fs: add retryDelay option to rimraf This commit adds a retryDelay option to rimraf which configures the amount of time between retry operations. Refs: https://github.com/nodejs/node/issues/30580 PR-URL: https://github.com/nodejs/node/pull/30644 Reviewed-By: Anna Henningsen Reviewed-By: James M Snell --- doc/api/fs.md | 26 +++++++++++++++++------- lib/internal/fs/rimraf.js | 3 ++- lib/internal/fs/utils.js | 3 +++ test/parallel/test-fs-rmdir-recursive.js | 11 ++++++++++ 4 files changed, 35 insertions(+), 8 deletions(-) diff --git a/doc/api/fs.md b/doc/api/fs.md index 887bb9e54a6b96..d52fe47d0bbcc6 100644 --- a/doc/api/fs.md +++ b/doc/api/fs.md @@ -3224,7 +3224,8 @@ changes: pr-url: https://github.com/nodejs/node/pull/30644 description: The `maxBusyTries` option is renamed to `maxRetries`, and its default is 0. The `emfileWait` option has been removed, and - `EMFILE` errors use the same retry logic as other errors. + `EMFILE` errors use the same retry logic as other errors. The + `retryDelay` option is now supported. - version: v12.10.0 pr-url: https://github.com/nodejs/node/pull/29168 description: The `recursive`, `maxBusyTries`, and `emfileWait` options are @@ -3249,12 +3250,15 @@ changes: * `options` {Object} * `maxRetries` {integer} If an `EBUSY`, `EMFILE`, `ENOTEMPTY`, or `EPERM` error is encountered, Node.js will retry the operation with a linear backoff - wait of 100ms longer on each try. This option represents the number of - retries. This option is ignored if the `recursive` option is not `true`. + wait of `retryDelay` ms longer on each try. This option represents the number + of retries. This option is ignored if the `recursive` option is not `true`. **Default:** `0`. * `recursive` {boolean} If `true`, perform a recursive directory removal. In recursive mode, errors are not reported if `path` does not exist, and operations are retried on failure. **Default:** `false`. + * `retryDelay` {integer} The amount of time in milliseconds to wait between + retries. This option is ignored if the `recursive` option is not `true`. + **Default:** `100`. * `callback` {Function} * `err` {Error} @@ -3272,7 +3276,8 @@ changes: pr-url: https://github.com/nodejs/node/pull/30644 description: The `maxBusyTries` option is renamed to `maxRetries`, and its default is 0. The `emfileWait` option has been removed, and - `EMFILE` errors use the same retry logic as other errors. + `EMFILE` errors use the same retry logic as other errors. The + `retryDelay` option is now supported. - version: v12.10.0 pr-url: https://github.com/nodejs/node/pull/29168 description: The `recursive`, `maxBusyTries`, and `emfileWait` options are @@ -3294,6 +3299,9 @@ changes: * `recursive` {boolean} If `true`, perform a recursive directory removal. In recursive mode, errors are not reported if `path` does not exist, and operations are retried on failure. **Default:** `false`. + * `retryDelay` {integer} The amount of time in milliseconds to wait between + retries. This option is ignored if the `recursive` option is not `true`. + **Default:** `100`. Synchronous rmdir(2). Returns `undefined`. @@ -5005,7 +5013,8 @@ changes: pr-url: https://github.com/nodejs/node/pull/30644 description: The `maxBusyTries` option is renamed to `maxRetries`, and its default is 0. The `emfileWait` option has been removed, and - `EMFILE` errors use the same retry logic as other errors. + `EMFILE` errors use the same retry logic as other errors. The + `retryDelay` option is now supported. - version: v12.10.0 pr-url: https://github.com/nodejs/node/pull/29168 description: The `recursive`, `maxBusyTries`, and `emfileWait` options are @@ -5018,12 +5027,15 @@ changes: * `options` {Object} * `maxRetries` {integer} If an `EBUSY`, `EMFILE`, `ENOTEMPTY`, or `EPERM` error is encountered, Node.js will retry the operation with a linear backoff - wait of 100ms longer on each try. This option represents the number of - retries. This option is ignored if the `recursive` option is not `true`. + wait of `retryDelay` ms longer on each try. This option represents the number + of retries. This option is ignored if the `recursive` option is not `true`. **Default:** `0`. * `recursive` {boolean} If `true`, perform a recursive directory removal. In recursive mode, errors are not reported if `path` does not exist, and operations are retried on failure. **Default:** `false`. + * `retryDelay` {integer} The amount of time in milliseconds to wait between + retries. This option is ignored if the `recursive` option is not `true`. + **Default:** `100`. * Returns: {Promise} Removes the directory identified by `path` then resolves the `Promise` with diff --git a/lib/internal/fs/rimraf.js b/lib/internal/fs/rimraf.js index 9d5fcad49c9fca..3212fdd539ed45 100644 --- a/lib/internal/fs/rimraf.js +++ b/lib/internal/fs/rimraf.js @@ -35,7 +35,8 @@ function rimraf(path, options, callback) { if (err) { if (retryErrorCodes.has(err.code) && retries < options.maxRetries) { retries++; - return setTimeout(_rimraf, retries * 100, path, options, CB); + const delay = retries * options.retryDelay; + return setTimeout(_rimraf, delay, path, options, CB); } // The file is already gone. diff --git a/lib/internal/fs/utils.js b/lib/internal/fs/utils.js index 0e77ddc267c564..1a5df71028c942 100644 --- a/lib/internal/fs/utils.js +++ b/lib/internal/fs/utils.js @@ -24,6 +24,7 @@ const { const { once } = require('internal/util'); const { toPathIfFileURL } = require('internal/url'); const { + validateInt32, validateUint32 } = require('internal/validators'); const pathModule = require('path'); @@ -562,6 +563,7 @@ function warnOnNonPortableTemplate(template) { } const defaultRmdirOptions = { + retryDelay: 100, maxRetries: 0, recursive: false, }; @@ -577,6 +579,7 @@ const validateRmdirOptions = hideStackFrames((options) => { if (typeof options.recursive !== 'boolean') throw new ERR_INVALID_ARG_TYPE('recursive', 'boolean', options.recursive); + validateInt32(options.retryDelay, 'retryDelay', 0); validateUint32(options.maxRetries, 'maxRetries'); return options; diff --git a/test/parallel/test-fs-rmdir-recursive.js b/test/parallel/test-fs-rmdir-recursive.js index 53f863872d80ea..628bba5d6fc4ab 100644 --- a/test/parallel/test-fs-rmdir-recursive.js +++ b/test/parallel/test-fs-rmdir-recursive.js @@ -155,10 +155,12 @@ function removeAsync(dir) { // Test input validation. { const defaults = { + retryDelay: 100, maxRetries: 0, recursive: false }; const modified = { + retryDelay: 953, maxRetries: 5, recursive: true }; @@ -169,6 +171,7 @@ function removeAsync(dir) { assert.deepStrictEqual(validateRmdirOptions({ maxRetries: 99 }), { + retryDelay: 100, maxRetries: 99, recursive: false }); @@ -193,6 +196,14 @@ function removeAsync(dir) { }); }); + common.expectsError(() => { + validateRmdirOptions({ retryDelay: -1 }); + }, { + code: 'ERR_OUT_OF_RANGE', + type: RangeError, + message: /^The value of "retryDelay" is out of range\./ + }); + common.expectsError(() => { validateRmdirOptions({ maxRetries: -1 }); }, {