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: nodejs#30580 PR-URL: nodejs#30644 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]>
cjihrig · Nov 27, 2019 · 4c77a76 · 4c77a76
1 parent 7e85f06
commit 4c77a76
Show file tree

Hide file tree

Showing 4 changed files with 18 additions and 39 deletions.
diff --git a/doc/api/fs.md b/doc/api/fs.md
@@ -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
@@ -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`.
@@ -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
@@ -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
@@ -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`.

diff --git a/lib/internal/fs/rimraf.js b/lib/internal/fs/rimraf.js
@@ -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;

diff --git a/lib/internal/fs/utils.js b/lib/internal/fs/utils.js
@@ -28,7 +28,6 @@ const {
 const { once } = require('internal/util');
 const { toPathIfFileURL } = require('internal/url');
 const {
-  validateInt32,
   validateUint32
 } = require('internal/validators');
 const pathModule = require('path');
@@ -566,7 +565,6 @@ function warnOnNonPortableTemplate(template) {
 }
 
 const defaultRmdirOptions = {
-  emfileWait: 1000,
   maxRetries: 0,
   recursive: false,
 };
@@ -582,7 +580,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;

diff --git a/test/parallel/test-fs-rmdir-recursive.js b/test/parallel/test-fs-rmdir-recursive.js
@@ -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
   };
@@ -171,7 +169,6 @@ function removeAsync(dir) {
   assert.deepStrictEqual(validateRmdirOptions({
     maxRetries: 99
   }), {
-    emfileWait: 1000,
     maxRetries: 99,
     recursive: false
   });
@@ -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 });
   }, {