diff --git a/src/bun.js/bindings/ErrorCode.cpp b/src/bun.js/bindings/ErrorCode.cpp index 702290f825..69fe35cd5f 100644 --- a/src/bun.js/bindings/ErrorCode.cpp +++ b/src/bun.js/bindings/ErrorCode.cpp @@ -1065,6 +1065,8 @@ JSC_DEFINE_HOST_FUNCTION(Bun::jsFunctionMakeErrorWithCode, (JSC::JSGlobalObject return JSC::JSValue::encode(createError(globalObject, ErrorCode::ERR_STREAM_UNABLE_TO_PIPE, "Cannot pipe to a closed or destroyed stream"_s)); case ErrorCode::ERR_ILLEGAL_CONSTRUCTOR: return JSC::JSValue::encode(createError(globalObject, ErrorCode::ERR_ILLEGAL_CONSTRUCTOR, "Illegal constructor"_s)); + case ErrorCode::ERR_DIR_CLOSED: + return JSC::JSValue::encode(createError(globalObject, ErrorCode::ERR_DIR_CLOSED, "Directory handle was closed"_s)); default: { break; diff --git a/src/bun.js/bindings/ErrorCode.ts b/src/bun.js/bindings/ErrorCode.ts index d191b10665..5fc2086578 100644 --- a/src/bun.js/bindings/ErrorCode.ts +++ b/src/bun.js/bindings/ErrorCode.ts @@ -92,6 +92,9 @@ const errors: ErrorCodeMapping = [ // Console ["ERR_CONSOLE_WRITABLE_STREAM", TypeError, "TypeError"], + // FS + ["ERR_DIR_CLOSED", Error], + // DNS ["ERR_DNS_SET_SERVERS_FAILED", Error], diff --git a/src/js/node/fs.ts b/src/js/node/fs.ts index f3fd95a9cb..319930fa7d 100644 --- a/src/js/node/fs.ts +++ b/src/js/node/fs.ts @@ -1013,19 +1013,25 @@ function opendirSync(path, options) { } class Dir { - #handle; + /** + * `-1` when closed. stdio handles (0, 1, 2) don't actually get closed by + * {@link close} or {@link closeSync}. + */ + #handle: number; #path; #options; #entries: any[] | null = null; constructor(handle, path, options) { - if (handle == null) throw $ERR_MISSING_ARGS("handle"); + if ($isUndefinedOrNull(handle)) throw $ERR_MISSING_ARGS("handle"); this.#handle = handle; this.#path = path; this.#options = options; } readSync() { + if (this.#handle < 0) throw $ERR_DIR_CLOSED(); + let entries = (this.#entries ??= fs.readdirSync(this.#path, { withFileTypes: true, encoding: this.#options?.encoding, @@ -1035,6 +1041,8 @@ class Dir { } read(cb?): any { + if (this.#handle < 0) throw $ERR_DIR_CLOSED(); + if (cb) { return this.read().then(entry => cb(null, entry)); } @@ -1054,13 +1062,19 @@ class Dir { } close(cb?: () => void) { + const handle = this.#handle; if (cb) { process.nextTick(cb); } - return fs.closedirSync(this.#handle); + if (handle > 2) fs.closeSync(handle); + this.#handle = -1; } - closeSync() {} + closeSync() { + const handle = this.#handle; + if (handle > 2) fs.closeSync(handle); + this.#handle = -1; + } get path() { return this.#path; diff --git a/test/harness.ts b/test/harness.ts index dcb84f3e88..7c26b67ee3 100644 --- a/test/harness.ts +++ b/test/harness.ts @@ -1040,7 +1040,7 @@ export function mergeWindowEnvs(envs: Record[]) { return flat; } -export function tmpdirSync(pattern: string = "bun.test.") { +export function tmpdirSync(pattern: string = "bun.test."): string { return fs.mkdtempSync(join(fs.realpathSync.native(os.tmpdir()), pattern)); } diff --git a/test/js/node/fs/dir.test.ts b/test/js/node/fs/dir.test.ts new file mode 100644 index 0000000000..7dcaf41b7d --- /dev/null +++ b/test/js/node/fs/dir.test.ts @@ -0,0 +1,34 @@ +import { describe, beforeAll, afterAll, it, expect } from "bun:test"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +describe("given a directory that exists", () => { + let dirname: string; + + beforeAll(() => { + const name = "dir-sync.test." + String(Math.random() * 100).substring(0, 6); + dirname = path.join(os.tmpdir(), name); + fs.mkdirSync(dirname); + }); + + afterAll(() => { + fs.rmdirSync(dirname, { recursive: true }); + }); + + it("can be opened/closed synchronously", () => { + const dir = fs.opendirSync(dirname); + expect(dir).toBeDefined(); + expect(dir).toBeInstanceOf(fs.Dir); + expect(dir.closeSync()).toBeUndefined(); + expect(() => dir.readSync()).toThrow("Directory handle was closed"); + }); + + it("can be opened/closed asynchronously", async () => { + const dir = await fs.promises.opendir(dirname); + expect(dir).toBeDefined(); + expect(dir).toBeInstanceOf(fs.Dir); + expect(await dir.close()).toBeUndefined(); + expect(() => dir.read()).toThrow("Directory handle was closed"); + }); +});