Skip to content

Commit

Permalink
fs: remove rimraf's emfileWait option
Browse files Browse the repository at this point in the history
This commit removes the emfileWait option. EMFILE errors are
now handled the same as any other retriable error.

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 targos committed Jan 13, 2020
1 parent 022ecbd commit 0a33f17
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 39 deletions.
35 changes: 16 additions & 19 deletions doc/api/fs.md
Original file line number Diff line number Diff line change
Expand Up @@ -3223,7 +3223,8 @@ 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.
default is 0. The `emfileWait` option has been removed, and
`EMFILE` errors use the same retry logic as other errors.
- version: v12.10.0
pr-url: https://github.com/nodejs/node/pull/29168
description: The `recursive`, `maxBusyTries`, and `emfileWait` options are
Expand All @@ -3246,14 +3247,11 @@ 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`.
* `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:** `0`.
* `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`.
**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 @@ -3273,7 +3271,8 @@ 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.
default is 0. The `emfileWait` option has been removed, and
`EMFILE` errors use the same retry logic as other errors.
- 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 @@ -5005,7 +5004,8 @@ 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.
default is 0. The `emfileWait` option has been removed, and
`EMFILE` errors use the same retry logic as other errors.
- version: v12.10.0
pr-url: https://github.com/nodejs/node/pull/29168
description: The `recursive`, `maxBusyTries`, and `emfileWait` options are
Expand All @@ -5016,14 +5016,11 @@ 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`.
* `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:** `0`.
* `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`.
**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
8 changes: 2 additions & 6 deletions lib/internal/fs/rimraf.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,26 +22,22 @@ const {
const { join } = require('path');
const { setTimeout } = require('timers');
const notEmptyErrorCodes = new Set(['ENOTEMPTY', 'EEXIST', 'EPERM']);
const retryErrorCodes = new Set(['EBUSY', 'EMFILE', 'ENOTEMPTY', 'EPERM']);
const isWindows = process.platform === 'win32';
const epermHandler = isWindows ? fixWinEPERM : _rmdir;
const epermHandlerSync = isWindows ? fixWinEPERMSync : _rmdirSync;


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

_rimraf(path, options, function CB(err) {
if (err) {
if ((err.code === 'EBUSY' || err.code === 'ENOTEMPTY' ||
err.code === 'EPERM') && retries < options.maxRetries) {
if (retryErrorCodes.has(err.code) && retries < options.maxRetries) {
retries++;
return setTimeout(_rimraf, retries * 100, 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
3 changes: 0 additions & 3 deletions lib/internal/fs/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ const {
const { once } = require('internal/util');
const { toPathIfFileURL } = require('internal/url');
const {
validateInt32,
validateUint32
} = require('internal/validators');
const pathModule = require('path');
Expand Down Expand Up @@ -563,7 +562,6 @@ function warnOnNonPortableTemplate(template) {
}

const defaultRmdirOptions = {
emfileWait: 1000,
maxRetries: 0,
recursive: false,
};
Expand All @@ -579,7 +577,6 @@ 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.maxRetries, 'maxRetries');

return options;
Expand Down
11 changes: 0 additions & 11 deletions test/parallel/test-fs-rmdir-recursive.js
Original file line number Diff line number Diff line change
Expand Up @@ -155,12 +155,10 @@ function removeAsync(dir) {
// Test input validation.
{
const defaults = {
emfileWait: 1000,
maxRetries: 0,
recursive: false
};
const modified = {
emfileWait: 953,
maxRetries: 5,
recursive: true
};
Expand All @@ -171,7 +169,6 @@ function removeAsync(dir) {
assert.deepStrictEqual(validateRmdirOptions({
maxRetries: 99
}), {
emfileWait: 1000,
maxRetries: 99,
recursive: false
});
Expand All @@ -196,14 +193,6 @@ function removeAsync(dir) {
});
});

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

common.expectsError(() => {
validateRmdirOptions({ maxRetries: -1 });
}, {
Expand Down

0 comments on commit 0a33f17

Please sign in to comment.