Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fs: refactor rimraf retry options #30644

Merged
merged 5 commits into from Nov 27, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
61 changes: 45 additions & 16 deletions doc/api/fs.md
Expand Up @@ -3220,6 +3220,13 @@ Synchronous rename(2). Returns `undefined`.
<!-- YAML
added: v0.0.2
changes:
- version: REPLACEME
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. The
`retryDelay` option is now supported. `ENFILE` errors are now
retried.
- version: v12.10.0
pr-url: https://github.com/nodejs/node/pull/29168
description: The `recursive`, `maxBusyTries`, and `emfileWait` options are
Expand All @@ -3242,17 +3249,17 @@ changes:

* `path` {string|Buffer|URL}
* `options` {Object}
* `emfileWait` {integer} If an `EMFILE` error is encountered, Node.js will
retry the operation with a linear backoff of 1ms longer on each try until the
timeout duration passes this limit. This option is ignored if the `recursive`
option is not `true`. **Default:** `1000`.
* `maxBusyTries` {integer} If an `EBUSY`, `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`. **Default:** `3`.
* `maxRetries` {integer} If an `EBUSY`, `EMFILE`, `ENFILE`, `ENOTEMPTY`, or
`EPERM` error is encountered, Node.js will retry the operation with a linear
backoff 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}

Expand All @@ -3266,6 +3273,13 @@ Windows and an `ENOTDIR` error on POSIX.
<!-- YAML
added: v0.1.21
changes:
- version: REPLACEME
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. The
`retryDelay` option is now supported. `ENFILE` errors are now
retried.
- version: v12.10.0
pr-url: https://github.com/nodejs/node/pull/29168
description: The `recursive`, `maxBusyTries`, and `emfileWait` options are
Expand All @@ -3280,9 +3294,17 @@ changes:

* `path` {string|Buffer|URL}
* `options` {Object}
* `maxRetries` {integer} If an `EBUSY`, `EMFILE`, `ENFILE`, `ENOTEMPTY`, or
`EPERM` error is encountered, Node.js will retry the operation with a linear
backoff 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`.

Synchronous rmdir(2). Returns `undefined`.

Expand Down Expand Up @@ -4990,6 +5012,13 @@ upon success.
<!-- YAML
added: v10.0.0
changes:
- version: REPLACEME
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. The
`retryDelay` option is now supported. `ENFILE` errors are now
retried.
- version: v12.10.0
pr-url: https://github.com/nodejs/node/pull/29168
description: The `recursive`, `maxBusyTries`, and `emfileWait` options are
Expand All @@ -5000,17 +5029,17 @@ changes:

* `path` {string|Buffer|URL}
* `options` {Object}
* `emfileWait` {integer} If an `EMFILE` error is encountered, Node.js will
retry the operation with a linear backoff of 1ms longer on each try until the
timeout duration passes this limit. This option is ignored if the `recursive`
option is not `true`. **Default:** `1000`.
* `maxBusyTries` {integer} If an `EBUSY`, `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`. **Default:** `3`.
* `maxRetries` {integer} If an `EBUSY`, `EMFILE`, `ENFILE`, `ENOTEMPTY`, or
`EPERM` error is encountered, Node.js will retry the operation with a linear
backoff 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
Expand Down
19 changes: 8 additions & 11 deletions lib/internal/fs/rimraf.js
Expand Up @@ -22,27 +22,24 @@ const {
const { join } = require('path');
const { setTimeout } = require('timers');
const notEmptyErrorCodes = new Set(['ENOTEMPTY', 'EEXIST', 'EPERM']);
const retryErrorCodes = new Set(
['EBUSY', 'EMFILE', 'ENFILE', 'ENOTEMPTY', 'EPERM']);
const isWindows = process.platform === 'win32';
const epermHandler = isWindows ? fixWinEPERM : _rmdir;
const epermHandlerSync = isWindows ? fixWinEPERMSync : _rmdirSync;
const numRetries = isWindows ? 100 : 1;


function rimraf(path, options, callback) {
let timeout = 0; // For EMFILE handling.
let busyTries = 0;
let retries = 0;

_rimraf(path, options, function CB(err) {
if (err) {
if ((err.code === 'EBUSY' || err.code === 'ENOTEMPTY' ||
err.code === 'EPERM') && busyTries < options.maxBusyTries) {
busyTries++;
return setTimeout(_rimraf, busyTries * 100, path, options, CB);
if (retryErrorCodes.has(err.code) && retries < options.maxRetries) {
retries++;
const delay = retries * options.retryDelay;
return setTimeout(_rimraf, delay, path, options, CB);
}

if (err.code === 'EMFILE' && timeout < options.emfileWait)
return setTimeout(_rimraf, timeout++, path, options, CB);

// The file is already gone.
if (err.code === 'ENOENT')
err = null;
Expand Down Expand Up @@ -211,7 +208,7 @@ function _rmdirSync(path, options, originalErr) {
rimrafSync(join(path, child), options);
});

for (let i = 0; i < numRetries; i++) {
for (let i = 0; i < options.maxRetries + 1; i++) {
try {
return rmdirSync(path, options);
} catch {} // Ignore errors.
Expand Down
8 changes: 4 additions & 4 deletions lib/internal/fs/utils.js
Expand Up @@ -566,8 +566,8 @@ function warnOnNonPortableTemplate(template) {
}

const defaultRmdirOptions = {
emfileWait: 1000,
maxBusyTries: 3,
retryDelay: 100,
maxRetries: 0,
recursive: false,
};

Expand All @@ -582,8 +582,8 @@ const validateRmdirOptions = hideStackFrames((options) => {
if (typeof options.recursive !== 'boolean')
throw new ERR_INVALID_ARG_TYPE('recursive', 'boolean', options.recursive);

validateInt32(options.emfileWait, 'emfileWait', 0);
validateUint32(options.maxBusyTries, 'maxBusyTries');
validateInt32(options.retryDelay, 'retryDelay', 0);
validateUint32(options.maxRetries, 'maxRetries');

return options;
});
Expand Down
22 changes: 11 additions & 11 deletions test/parallel/test-fs-rmdir-recursive.js
Expand Up @@ -155,24 +155,24 @@ function removeAsync(dir) {
// Test input validation.
{
const defaults = {
emfileWait: 1000,
maxBusyTries: 3,
retryDelay: 100,
maxRetries: 0,
recursive: false
};
const modified = {
emfileWait: 953,
maxBusyTries: 5,
retryDelay: 953,
maxRetries: 5,
recursive: true
};

assert.deepStrictEqual(validateRmdirOptions(), defaults);
assert.deepStrictEqual(validateRmdirOptions({}), defaults);
assert.deepStrictEqual(validateRmdirOptions(modified), modified);
assert.deepStrictEqual(validateRmdirOptions({
maxBusyTries: 99
maxRetries: 99
}), {
emfileWait: 1000,
maxBusyTries: 99,
retryDelay: 100,
maxRetries: 99,
recursive: false
});

Expand All @@ -197,18 +197,18 @@ function removeAsync(dir) {
});

common.expectsError(() => {
validateRmdirOptions({ emfileWait: -1 });
validateRmdirOptions({ retryDelay: -1 });
}, {
code: 'ERR_OUT_OF_RANGE',
type: RangeError,
message: /^The value of "emfileWait" is out of range\./
message: /^The value of "retryDelay" is out of range\./
});

common.expectsError(() => {
validateRmdirOptions({ maxBusyTries: -1 });
validateRmdirOptions({ maxRetries: -1 });
}, {
code: 'ERR_OUT_OF_RANGE',
type: RangeError,
message: /^The value of "maxBusyTries" is out of range\./
message: /^The value of "maxRetries" is out of range\./
});
}