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 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
fs: rename rimraf's maxBusyTries to maxRetries
This is part of reworking the rimraf retry logic.

Refs: #30580
PR-URL: #30644
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
cjihrig committed Nov 27, 2019
commit 4059055739c1c395c00cf989d77073e862f9dae6
13 changes: 11 additions & 2 deletions doc/api/fs.md
Original file line number Diff line number Diff line change
Expand Up @@ -3220,6 +3220,9 @@ 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`.
- version: v12.10.0
pr-url: https://github.com/nodejs/node/pull/29168
description: The `recursive`, `maxBusyTries`, and `emfileWait` options are
Expand All @@ -3246,7 +3249,7 @@ changes:
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
* `maxRetries` {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`.
Expand All @@ -3266,6 +3269,9 @@ 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`.
- version: v12.10.0
pr-url: https://github.com/nodejs/node/pull/29168
description: The `recursive`, `maxBusyTries`, and `emfileWait` options are
Expand Down Expand Up @@ -4990,6 +4996,9 @@ 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`.
- version: v12.10.0
pr-url: https://github.com/nodejs/node/pull/29168
description: The `recursive`, `maxBusyTries`, and `emfileWait` options are
Expand All @@ -5004,7 +5013,7 @@ changes:
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
* `maxRetries` {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`.
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/fs/rimraf.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ function rimraf(path, options, callback) {
_rimraf(path, options, function CB(err) {
if (err) {
if ((err.code === 'EBUSY' || err.code === 'ENOTEMPTY' ||
err.code === 'EPERM') && busyTries < options.maxBusyTries) {
err.code === 'EPERM') && busyTries < options.maxRetries) {
busyTries++;
return setTimeout(_rimraf, busyTries * 100, path, options, CB);
}
Expand Down
4 changes: 2 additions & 2 deletions lib/internal/fs/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -567,7 +567,7 @@ function warnOnNonPortableTemplate(template) {

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

Expand All @@ -583,7 +583,7 @@ const validateRmdirOptions = hideStackFrames((options) => {
throw new ERR_INVALID_ARG_TYPE('recursive', 'boolean', options.recursive);

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

return options;
});
Expand Down
12 changes: 6 additions & 6 deletions test/parallel/test-fs-rmdir-recursive.js
Original file line number Diff line number Diff line change
Expand Up @@ -156,23 +156,23 @@ function removeAsync(dir) {
{
const defaults = {
emfileWait: 1000,
maxBusyTries: 3,
maxRetries: 3,
recursive: false
};
const modified = {
emfileWait: 953,
maxBusyTries: 5,
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,
maxRetries: 99,
recursive: false
});

Expand Down Expand Up @@ -205,10 +205,10 @@ function removeAsync(dir) {
});

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\./
});
}