Skip to content

Commit

Permalink
fs: make rimraf default to 0 retries
Browse files Browse the repository at this point in the history
This commit makes retries an opt-in feature by defaulting
to no automatic retries. This will be particularly important
once synchronous operations can sleep between attempts.

Refs: #30580
PR-URL: #30644
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
cjihrig authored and addaleax committed Nov 30, 2019
1 parent fa1f87b commit 612a3a2
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 13 deletions.
17 changes: 12 additions & 5 deletions doc/api/fs.md
Original file line number Diff line number Diff line change
Expand Up @@ -3222,7 +3222,8 @@ added: v0.0.2
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/30644
description: The `maxBusyTries` option is renamed to `maxRetries`.
description: The `maxBusyTries` option is renamed to `maxRetries`, and its
default is 0.
- 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 @@ -3252,7 +3253,7 @@ changes:
* `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`.
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`.
Expand All @@ -3271,7 +3272,8 @@ added: v0.1.21
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/30644
description: The `maxBusyTries` option is renamed to `maxRetries`.
description: The `maxBusyTries` option is renamed to `maxRetries`, and its
default is 0.
- version: v12.10.0
pr-url: https://github.com/nodejs/node/pull/29168
description: The `recursive`, `maxBusyTries`, and `emfileWait` options are
Expand All @@ -3286,6 +3288,10 @@ changes:
* `path` {string|Buffer|URL}
* `options` {Object}
* `maxRetries` {integer} If an `EBUSY`, `ENOTEMPTY`, or `EPERM` error is
encountered, Node.js will retry the operation. 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`.
Expand Down Expand Up @@ -4998,7 +5004,8 @@ added: v10.0.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/30644
description: The `maxBusyTries` option is renamed to `maxRetries`.
description: The `maxBusyTries` option is renamed to `maxRetries`, and its
default is 0.
- version: v12.10.0
pr-url: https://github.com/nodejs/node/pull/29168
description: The `recursive`, `maxBusyTries`, and `emfileWait` options are
Expand All @@ -5016,7 +5023,7 @@ changes:
* `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`.
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`.
Expand Down
11 changes: 5 additions & 6 deletions lib/internal/fs/rimraf.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,19 +25,18 @@ const notEmptyErrorCodes = new Set(['ENOTEMPTY', 'EEXIST', '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.maxRetries) {
busyTries++;
return setTimeout(_rimraf, busyTries * 100, path, options, CB);
err.code === 'EPERM') && retries < options.maxRetries) {
retries++;
return setTimeout(_rimraf, retries * 100, path, options, CB);
}

if (err.code === 'EMFILE' && timeout < options.emfileWait)
Expand Down Expand Up @@ -211,7 +210,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
2 changes: 1 addition & 1 deletion lib/internal/fs/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -563,7 +563,7 @@ function warnOnNonPortableTemplate(template) {

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

Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-fs-rmdir-recursive.js
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ function removeAsync(dir) {
{
const defaults = {
emfileWait: 1000,
maxRetries: 3,
maxRetries: 0,
recursive: false
};
const modified = {
Expand Down

0 comments on commit 612a3a2

Please sign in to comment.