Skip to content

Commit

Permalink
fs: pass ERR_DIR_CLOSED asynchronously to dir.close
Browse files Browse the repository at this point in the history
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 <duhamelantoine1995@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
  • Loading branch information
Lxxyx authored and danielleadams committed Dec 7, 2020
1 parent c04a2df commit 744b8aa
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 8 deletions.
19 changes: 14 additions & 5 deletions lib/internal/fs/dir.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ const {
ArrayPrototypeSplice,
FunctionPrototypeBind,
ObjectDefineProperty,
PromiseReject,
Symbol,
SymbolAsyncIterator,
} = primordials;
Expand Down Expand Up @@ -164,16 +165,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);
Expand Down
21 changes: 18 additions & 3 deletions test/parallel/test-fs-opendir.js
Original file line number Diff line number Diff line change
Expand Up @@ -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());

Expand Down Expand Up @@ -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());
}

0 comments on commit 744b8aa

Please sign in to comment.