From 1650c883419f1698bc436228351861653fa507d4 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 16 Aug 2026 01:46:11 +0000 Subject: [PATCH] node:fs: name a bad flush option "options.flush" and render dotted names as properties fs.writeFile, writeFileSync, appendFile, appendFileSync, fs.promises.writeFile, fs.promises.appendFile and FileHandle#appendFile rejected a non-boolean flush with 'The "flush" argument must be of type boolean'. Node validates the option as options.flush, so its message is 'The "options.flush" property must be of type boolean'. The option name now matches node, and the Rust ERR_INVALID_ARG_TYPE renderers in JSGlobalObject apply the same subject rule the C++ renderer already uses: a name containing "." is a property, anything else is an argument. The other dotted names passed to these renderers (fs.rm retry options, net.SocketAddress options, http2 priority options) now match node as well. --- src/jsc/JSGlobalObject.rs | 35 +++++++++++++--- src/runtime/node/node_fs.rs | 8 ++-- test/js/node/fs/fs.test.ts | 83 +++++++++++++++++++++++++++++++++++++ 3 files changed, 117 insertions(+), 9 deletions(-) diff --git a/src/jsc/JSGlobalObject.rs b/src/jsc/JSGlobalObject.rs index 804bdabdaa80..6033ffe24728 100644 --- a/src/jsc/JSGlobalObject.rs +++ b/src/jsc/JSGlobalObject.rs @@ -541,6 +541,7 @@ impl JSGlobalObject { } /// "The {argname} argument must be of type {typename}. Received {value}" + /// (a dotted `argname` is a "property", see `InvalidArgTypeName`). /// /// Accepts `&str`, `&[u8]`, or `b"..."` for `argname`/`typename`. pub fn throw_invalid_argument_type_value( @@ -556,8 +557,8 @@ impl JSGlobalObject { self.err( JscError::INVALID_ARG_TYPE, format_args!( - "The \"{}\" argument must be of type {}. Received {}", - bstr::BStr::new(argname.as_ref()), + "The {} must be of type {}. Received {}", + InvalidArgTypeName(argname.as_ref()), bstr::BStr::new(typename.as_ref()), actual_string_value ), @@ -565,6 +566,8 @@ impl JSGlobalObject { .throw() } + /// Like [`Self::throw_invalid_argument_type_value`], but `typename` is the whole + /// phrase after "must be" (e.g. `"an instance of Array"`). pub fn throw_invalid_argument_type_value2( &self, argname: impl AsRef<[u8]>, @@ -578,8 +581,8 @@ impl JSGlobalObject { self.err( JscError::INVALID_ARG_TYPE, format_args!( - "The \"{}\" argument must be {}. Received {}", - bstr::BStr::new(argname.as_ref()), + "The {} must be {}. Received {}", + InvalidArgTypeName(argname.as_ref()), bstr::BStr::new(typename.as_ref()), actual_string_value ), @@ -627,8 +630,8 @@ impl JSGlobalObject { self.err( JscError::INVALID_ARG_TYPE, format_args!( - "The \"{}\" argument must be one of type {}. Received {}", - bstr::BStr::new(argname.as_ref()), + "The {} must be one of type {}. Received {}", + InvalidArgTypeName(argname.as_ref()), bstr::BStr::new(typename.as_ref()), actual_string_value ), @@ -1380,6 +1383,26 @@ pub struct SysErrOptions { pub name: Option<&'static [u8]>, } +/// The subject of Node's `ERR_INVALID_ARG_TYPE` message: `"name" argument`, +/// `"options.name" property` for a dotted name, or the name verbatim when it +/// already ends in ` argument` ("first argument"). Same rule as +/// `Bun::Message::addParameter` in ErrorCode.cpp. +/// https://github.com/nodejs/node/blob/v26.3.0/lib/internal/errors.js#L1407-L1414 +struct InvalidArgTypeName<'a>(&'a [u8]); + +impl core::fmt::Display for InvalidArgTypeName<'_> { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + let name = bstr::BStr::new(self.0); + if self.0.ends_with(b" argument") { + write!(f, "{name}") + } else if strings::contains_char(self.0, b'.') { + write!(f, "\"{name}\" property") + } else { + write!(f, "\"{name}\" argument") + } + } +} + // Unified with the crate-root definitions (lib.rs) — re-exported here so // `bun_jsc::js_global_object::{IntegerRange, ValidateObjectOpts}` keep // resolving for any caller that named them via this path. The previous local diff --git a/src/runtime/node/node_fs.rs b/src/runtime/node/node_fs.rs index dd6773bdc442..6504a929b23e 100644 --- a/src/runtime/node/node_fs.rs +++ b/src/runtime/node/node_fs.rs @@ -4199,9 +4199,11 @@ pub mod args { if flush_.is_boolean() || flush_.is_undefined_or_null() { flush = flush_ == JSValue::TRUE; } else { - return Err( - ctx.throw_invalid_argument_type_value(b"flush", b"boolean", flush_) - ); + return Err(ctx.throw_invalid_argument_type_value( + b"options.flush", + b"boolean", + flush_, + )); } } } diff --git a/test/js/node/fs/fs.test.ts b/test/js/node/fs/fs.test.ts index 7fd0be69245b..872492a519ff 100644 --- a/test/js/node/fs/fs.test.ts +++ b/test/js/node/fs/fs.test.ts @@ -2763,6 +2763,89 @@ describe("writeFile/appendFile data argument", () => { }); }); +describe("writeFile/appendFile flush option", () => { + // Node validates it as `options.flush`, so ERR_INVALID_ARG_TYPE calls it a + // property, not an argument. Every string/Buffer entry point shares one + // native options parser, so they all have to agree. + const invalidArgType = (received: string) => + expect.objectContaining({ + code: "ERR_INVALID_ARG_TYPE", + message: `The "options.flush" property must be of type boolean. Received ${received}`, + }); + const cases: [flush: unknown, received: string][] = [ + ["yes", "type string ('yes')"], + [1, "type number (1)"], + [{}, "an instance of Object"], + ]; + + it.each(cases)("sync and callback forms throw for { flush: %p }", (flush, received) => { + using dir = tempDir("fs-flush-option", { "f.txt": "keep" }); + const file = join(String(dir), "f.txt"); + const options = { flush } as any; + + expect(() => writeFileSync(file, "x", options)).toThrow(invalidArgType(received)); + expect(() => writeFileSync(file, Buffer.from("x"), options)).toThrow(invalidArgType(received)); + expect(() => fs.appendFileSync(file, "x", options)).toThrow(invalidArgType(received)); + expect(() => fs.writeFile(file, "x", options, () => {})).toThrow(invalidArgType(received)); + expect(() => fs.appendFile(file, "x", options, () => {})).toThrow(invalidArgType(received)); + const fd = openSync(file, "r+"); + try { + expect(() => writeFileSync(fd, "x", options)).toThrow(invalidArgType(received)); + } finally { + closeSync(fd); + } + + expect(readFileSync(file, "utf8")).toBe("keep"); + }); + + it.each(cases)("promise forms reject for { flush: %p }", async (flush, received) => { + using dir = tempDir("fs-flush-option", { "f.txt": "keep" }); + const file = join(String(dir), "f.txt"); + const options = { flush } as any; + + await expect(promises.writeFile(file, "x", options)).rejects.toThrow(invalidArgType(received)); + await expect(promises.writeFile(file, Buffer.from("x"), options)).rejects.toThrow(invalidArgType(received)); + await expect(promises.appendFile(file, "x", options)).rejects.toThrow(invalidArgType(received)); + await using handle = await promises.open(file, "r+"); + await expect(handle.appendFile("x", options)).rejects.toThrow(invalidArgType(received)); + + expect(readFileSync(file, "utf8")).toBe("keep"); + }); + + it("accepts the values node accepts", () => { + using dir = tempDir("fs-flush-option", {}); + const file = join(String(dir), "f.txt"); + for (const flush of [undefined, null, false, true]) { + writeFileSync(file, "a", { flush } as any); + fs.appendFileSync(file, "b", { flush } as any); + expect(readFileSync(file, "utf8")).toBe("ab"); + } + }); +}); + +describe("ERR_INVALID_ARG_TYPE names an option as a property and a parameter as an argument", () => { + // Same renderer as the flush errors above; these pin the naming rule for a + // dotted name reached through the validators and for a plain name. + it("dotted option name", () => { + using dir = tempDir("fs-arg-type-name", { "f.txt": "" }); + expect(() => rmSync(join(String(dir), "f.txt"), { maxRetries: "a" } as any)).toThrow( + expect.objectContaining({ + code: "ERR_INVALID_ARG_TYPE", + message: `The "options.maxRetries" property must be of type number. Received type string ('a')`, + }), + ); + }); + + it("plain parameter name", () => { + expect(() => fs.fsyncSync("x" as any)).toThrow( + expect.objectContaining({ + code: "ERR_INVALID_ARG_TYPE", + message: `The "fd" argument must be of type number. Received type string ('x')`, + }), + ); + }); +}); + function triggerDOMJIT(target: fs.Stats, fn: (..._: any[]) => any, result: any) { for (let i = 0; i < 9999; i++) { if (fn.apply(target) !== result) {