Skip to content

Commit 744b8aa

Browse files
Lxxyxdanielleadams
authored andcommitted
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]>
1 parent c04a2df commit 744b8aa

File tree

2 files changed

+32
-8
lines changed

2 files changed

+32
-8
lines changed

Diff for: lib/internal/fs/dir.js

+14-5
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ const {
66
ArrayPrototypeSplice,
77
FunctionPrototypeBind,
88
ObjectDefineProperty,
9+
PromiseReject,
910
Symbol,
1011
SymbolAsyncIterator,
1112
} = primordials;
@@ -164,16 +165,24 @@ class Dir {
164165
}
165166

166167
close(callback) {
167-
if (this[kDirClosed] === true) {
168-
throw new ERR_DIR_CLOSED();
169-
}
170-
168+
// Promise
171169
if (callback === undefined) {
170+
if (this[kDirClosed] === true) {
171+
return PromiseReject(new ERR_DIR_CLOSED());
172+
}
172173
return this[kDirClosePromisified]();
173-
} else if (typeof callback !== 'function') {
174+
}
175+
176+
// callback
177+
if (typeof callback !== 'function') {
174178
throw new ERR_INVALID_CALLBACK(callback);
175179
}
176180

181+
if (this[kDirClosed] === true) {
182+
process.nextTick(callback, new ERR_DIR_CLOSED());
183+
return;
184+
}
185+
177186
if (this[kDirOperationQueue] !== null) {
178187
this[kDirOperationQueue].push(() => {
179188
this.close(callback);

Diff for: test/parallel/test-fs-opendir.js

+18-3
Original file line numberDiff line numberDiff line change
@@ -223,12 +223,11 @@ async function doAsyncIterInvalidCallbackTest() {
223223
}
224224
doAsyncIterInvalidCallbackTest().then(common.mustCall());
225225

226-
// Check if directory already closed - throw an exception
226+
// Check first call to close() - should not report an error.
227227
async function doAsyncIterDirClosedTest() {
228228
const dir = await fs.promises.opendir(testDir);
229229
await dir.close();
230-
231-
assert.throws(() => dir.close(), dirclosedError);
230+
await assert.rejects(() => dir.close(), dirclosedError);
232231
}
233232
doAsyncIterDirClosedTest().then(common.mustCall());
234233

@@ -267,3 +266,19 @@ async function doConcurrentAsyncMixedOps() {
267266
await promise2;
268267
}
269268
doConcurrentAsyncMixedOps().then(common.mustCall());
269+
270+
// Check if directory already closed - the callback should pass an error.
271+
{
272+
const dir = fs.opendirSync(testDir);
273+
dir.closeSync();
274+
dir.close(common.mustCall((error) => {
275+
assert.strictEqual(error.code, dirclosedError.code);
276+
}));
277+
}
278+
279+
// Check if directory already closed - throw an promise exception.
280+
{
281+
const dir = fs.opendirSync(testDir);
282+
dir.closeSync();
283+
assert.rejects(dir.close(), dirclosedError).then(common.mustCall());
284+
}

0 commit comments

Comments
 (0)