fs: pass ERR_DIR_CLOSED asynchronously to dir.close

Pass the error to the callback or returns a rejected Promise instead of throwing a synchonous error. Fixes: #36237 PR-URL: #36243 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Yongsheng Zhang <[email protected]>
nodejs · May 1, 2021 · 1bffd8d · 1bffd8d
1 parent cda0a80
commit 1bffd8d
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 8 deletions.
diff --git a/lib/internal/fs/dir.js b/lib/internal/fs/dir.js
@@ -2,6 +2,7 @@
 
 const {
   ObjectDefineProperty,
+  PromiseReject,
   Symbol,
   SymbolAsyncIterator,
 } = primordials;
@@ -157,16 +158,24 @@ class Dir {
   }
 
   close(callback) {
-    if (this[kDirClosed] === true) {
-      throw new ERR_DIR_CLOSED();
-    }
-
+    // Promise
     if (callback === undefined) {
+      if (this[kDirClosed] === true) {
+        return PromiseReject(new ERR_DIR_CLOSED());
+      }
       return this[kDirClosePromisified]();
-    } else if (typeof callback !== 'function') {
+    }
+
+    // callback
+    if (typeof callback !== 'function') {
       throw new ERR_INVALID_CALLBACK(callback);
     }
 
+    if (this[kDirClosed] === true) {
+      process.nextTick(callback, new ERR_DIR_CLOSED());
+      return;
+    }
+
     if (this[kDirOperationQueue] !== null) {
       this[kDirOperationQueue].push(() => {
         this.close(callback);

diff --git a/test/parallel/test-fs-opendir.js b/test/parallel/test-fs-opendir.js
@@ -223,12 +223,11 @@ async function doAsyncIterInvalidCallbackTest() {
 }
 doAsyncIterInvalidCallbackTest().then(common.mustCall());
 
-// Check if directory already closed - throw an exception
+// Check first call to close() - should not report an error.
 async function doAsyncIterDirClosedTest() {
   const dir = await fs.promises.opendir(testDir);
   await dir.close();
-
-  assert.throws(() => dir.close(), dirclosedError);
+  await assert.rejects(() => dir.close(), dirclosedError);
 }
 doAsyncIterDirClosedTest().then(common.mustCall());
 
@@ -267,3 +266,19 @@ async function doConcurrentAsyncMixedOps() {
   await promise2;
 }
 doConcurrentAsyncMixedOps().then(common.mustCall());
+
+// Check if directory already closed - the callback should pass an error.
+{
+  const dir = fs.opendirSync(testDir);
+  dir.closeSync();
+  dir.close(common.mustCall((error) => {
+    assert.strictEqual(error.code, dirclosedError.code);
+  }));
+}
+
+// Check if directory already closed - throw an promise exception.
+{
+  const dir = fs.opendirSync(testDir);
+  dir.closeSync();
+  assert.rejects(dir.close(), dirclosedError).then(common.mustCall());
+}