Skip to content

Commit 1bffd8d

Browse files
Lxxyxtargos
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 cda0a80 commit 1bffd8d

File tree

2 files changed

+32
-8
lines changed

2 files changed

+32
-8
lines changed

lib/internal/fs/dir.js

+14-5
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
const {
44
ObjectDefineProperty,
5+
PromiseReject,
56
Symbol,
67
SymbolAsyncIterator,
78
} = primordials;
@@ -157,16 +158,24 @@ class Dir {
157158
}
158159

159160
close(callback) {
160-
if (this[kDirClosed] === true) {
161-
throw new ERR_DIR_CLOSED();
162-
}
163-
161+
// Promise
164162
if (callback === undefined) {
163+
if (this[kDirClosed] === true) {
164+
return PromiseReject(new ERR_DIR_CLOSED());
165+
}
165166
return this[kDirClosePromisified]();
166-
} else if (typeof callback !== 'function') {
167+
}
168+
169+
// callback
170+
if (typeof callback !== 'function') {
167171
throw new ERR_INVALID_CALLBACK(callback);
168172
}
169173

174+
if (this[kDirClosed] === true) {
175+
process.nextTick(callback, new ERR_DIR_CLOSED());
176+
return;
177+
}
178+
170179
if (this[kDirOperationQueue] !== null) {
171180
this[kDirOperationQueue].push(() => {
172181
this.close(callback);

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)