Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
fs: remove rimraf's emfileWait option
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 BethGriggs committed Feb 6, 2020
1 parent ccc228b commit 7a32198
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
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
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
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
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 7a32198

Please sign in to comment.