From d3236fc2a6a0b7269375e453cdc8707cee5af53f Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 14 Aug 2026 20:12:41 +0000 Subject: [PATCH] node:fs: use node's ERR_INVALID_ARG_TYPE message for invalid utimes/lutimes/futimes times utimes, lutimes and futimes (sync, callback, promises and FileHandle.utimes) threw ERR_INVALID_ARG_TYPE with the message "atime must be a number or a Date" (or "atime is required" when the argument was missing). Node throws the message produced by toUnixTimestamp: 'The "time" argument must be an instance of Date or an Time in seconds. Received ...', where utimes and lutimes use the name "time" and futimes uses "atime"/"mtime". Add time_like::from_js_required, which eats the next argument (undefined when missing) and throws that message, and use it at the four call sites. --- src/runtime/node/node_fs.rs | 50 +++----------- src/runtime/node/time_like.rs | 21 +++++- test/js/node/fs/fs.test.ts | 123 +++++++++++++++++++++++++++++++++- 3 files changed, 150 insertions(+), 44 deletions(-) diff --git a/src/runtime/node/node_fs.rs b/src/runtime/node/node_fs.rs index 4919642333e9..d765f21a86e2 100644 --- a/src/runtime/node/node_fs.rs +++ b/src/runtime/node/node_fs.rs @@ -163,7 +163,7 @@ pub use super::types::PathOrFileDescriptor; /// Local alias for the many `node::foo` call sites below, routing to `super::*`. mod node { pub(super) use super::super::statfs::StatFS; - pub(super) use super::super::time_like::from_js as time_like_from_js; + pub(super) use super::super::time_like::from_js_required as time_like_from_js_required; pub(super) use super::super::types::SliceWithUnderlyingString; pub(super) use super::super::{gid_t, uid_t}; @@ -2879,28 +2879,12 @@ pub mod args { impl Lutimes { pub fn from_js(ctx: &JSGlobalObject, arguments: &mut ArgumentsSlice) -> JsResult { // `Drop for PathLike` covers the - // `time_like_from_js` throws below. + // `time_like_from_js_required` throws below. let path = PathLike::from_js_required(ctx, arguments, "path")?; - let atime = node::time_like_from_js( - ctx, - arguments.next().ok_or_else(|| { - ctx.throw_invalid_arguments(format_args!("atime is required")) - })?, - )? - .ok_or_else(|| { - ctx.throw_invalid_arguments(format_args!("atime must be a number or a Date")) - })?; - arguments.eat(); - let mtime = node::time_like_from_js( - ctx, - arguments.next().ok_or_else(|| { - ctx.throw_invalid_arguments(format_args!("mtime is required")) - })?, - )? - .ok_or_else(|| { - ctx.throw_invalid_arguments(format_args!("mtime must be a number or a Date")) - })?; - arguments.eat(); + // Node's utimes/lutimes report both timestamps as "time"; only + // futimes names them "atime"/"mtime". + let atime = node::time_like_from_js_required(ctx, arguments, "time")?; + let mtime = node::time_like_from_js_required(ctx, arguments, "time")?; Ok(Lutimes { path, atime, mtime }) } } @@ -3577,26 +3561,8 @@ pub mod args { pub(crate) fn to_thread_safe(&self) {} pub fn from_js(ctx: &JSGlobalObject, arguments: &mut ArgumentsSlice) -> JsResult { let fd = FD::from_js_required(ctx, arguments)?; - let atime = node::time_like_from_js( - ctx, - arguments.next().ok_or_else(|| { - ctx.throw_invalid_arguments(format_args!("atime is required")) - })?, - )? - .ok_or_else(|| { - ctx.throw_invalid_arguments(format_args!("atime must be a number or a Date")) - })?; - arguments.eat(); - let mtime = node::time_like_from_js( - ctx, - arguments.next().ok_or_else(|| { - ctx.throw_invalid_arguments(format_args!("mtime is required")) - })?, - )? - .ok_or_else(|| { - ctx.throw_invalid_arguments(format_args!("mtime must be a number or a Date")) - })?; - arguments.eat(); + let atime = node::time_like_from_js_required(ctx, arguments, "atime")?; + let mtime = node::time_like_from_js_required(ctx, arguments, "mtime")?; Ok(Futimes { fd, atime, mtime }) } } diff --git a/src/runtime/node/time_like.rs b/src/runtime/node/time_like.rs index c90148985274..d50218476fea 100644 --- a/src/runtime/node/time_like.rs +++ b/src/runtime/node/time_like.rs @@ -1,4 +1,4 @@ -use bun_jsc::{JSGlobalObject, JSType as JsType, JSValue, JsResult}; +use bun_jsc::{ArgumentsSlice, JSGlobalObject, JSType as JsType, JSValue, JsResult}; /// On windows, this is what libuv expects /// On unix it is what the utimens api expects @@ -49,6 +49,25 @@ pub fn from_js(global_object: &JSGlobalObject, value: JSValue) -> JsResult JsResult { + let value = arguments.next_eat().unwrap_or(JSValue::UNDEFINED); + from_js(global_object, value)?.ok_or_else(|| { + // Node passes `["Date", "Time in seconds"]` as the expected types and its + // ERR_INVALID_ARG_TYPE list renderer prints that as "an Time in seconds". + global_object.throw_invalid_argument_type_value2( + name, + "an instance of Date or an Time in seconds", + value, + ) + }) +} + #[cfg(windows)] fn from_seconds(seconds: f64) -> TimeLike { seconds diff --git a/test/js/node/fs/fs.test.ts b/test/js/node/fs/fs.test.ts index 746637c7eba1..61ba5e27c8ff 100644 --- a/test/js/node/fs/fs.test.ts +++ b/test/js/node/fs/fs.test.ts @@ -1,4 +1,4 @@ -import { beforeAll, describe, expect, it, spyOn } from "bun:test"; +import { afterAll, beforeAll, describe, expect, it, spyOn } from "bun:test"; import { bunEnv, bunExe, @@ -4760,6 +4760,127 @@ describe("utimesSync", () => { }); }); +describe("utimes/lutimes/futimes reject an invalid time with node's message", () => { + // Node's toUnixTimestamp() passes ["Date", "Time in seconds"] as the expected + // types, so its message really does say "an Time in seconds". utimes and + // lutimes validate both timestamps under toUnixTimestamp's default name + // "time"; only futimes (and so FileHandle.utimes) says "atime"/"mtime". + const invalidTime = (name: "time" | "atime" | "mtime", received: string) => ({ + name: "TypeError", + code: "ERR_INVALID_ARG_TYPE", + message: `The "${name}" argument must be an instance of Date or an Time in seconds. Received ${received}`, + }); + const pick = ({ name, code, message }: any) => ({ name, code, message }); + const thrownBy = (fn: () => unknown) => { + try { + fn(); + } catch (e) { + return pick(e); + } + throw new Error("expected to throw"); + }; + const rejectedBy = async (fn: () => Promise) => { + try { + await fn(); + } catch (e) { + return pick(e); + } + throw new Error("expected to reject"); + }; + const mustNotCall = () => { + throw new Error("callback must not be called"); + }; + + const file = join(tempDirWithFiles("fs-utimes-invalid-time", { "f.txt": "x" }), "f.txt"); + let fd: number; + beforeAll(() => { + fd = openSync(file, "r"); + }); + afterAll(() => closeSync(fd)); + + it.each([ + ["bad", "type string ('bad')"], + [{}, "an instance of Object"], + [[1], "an instance of Array"], + [null, "null"], + [undefined, "undefined"], + [NaN, "type number (NaN)"], + [Infinity, "type number (Infinity)"], + [-Infinity, "type number (-Infinity)"], + [true, "type boolean (true)"], + [1n, "type bigint (1n)"], + ] as [time: any, received: string][])("sync forms, time = %p", (time, received) => { + expect({ + utimesAtime: thrownBy(() => fs.utimesSync(file, time, 1)), + utimesMtime: thrownBy(() => fs.utimesSync(file, 1, time)), + lutimesAtime: thrownBy(() => fs.lutimesSync(file, time, 1)), + lutimesMtime: thrownBy(() => fs.lutimesSync(file, 1, time)), + futimesAtime: thrownBy(() => fs.futimesSync(fd, time, 1)), + futimesMtime: thrownBy(() => fs.futimesSync(fd, 1, time)), + }).toEqual({ + utimesAtime: invalidTime("time", received), + utimesMtime: invalidTime("time", received), + lutimesAtime: invalidTime("time", received), + lutimesMtime: invalidTime("time", received), + futimesAtime: invalidTime("atime", received), + futimesMtime: invalidTime("mtime", received), + }); + }); + + it("callback forms throw synchronously, as in node", () => { + expect({ + utimes: thrownBy(() => fs.utimes(file, "bad", 1, mustNotCall)), + lutimes: thrownBy(() => fs.lutimes(file, 1, {} as any, mustNotCall)), + futimesAtime: thrownBy(() => fs.futimes(fd, "bad", 1, mustNotCall)), + futimesMtime: thrownBy(() => fs.futimes(fd, 1, {} as any, mustNotCall)), + }).toEqual({ + utimes: invalidTime("time", "type string ('bad')"), + lutimes: invalidTime("time", "an instance of Object"), + futimesAtime: invalidTime("atime", "type string ('bad')"), + futimesMtime: invalidTime("mtime", "an instance of Object"), + }); + }); + + it("fs/promises forms and FileHandle.utimes reject", async () => { + await using handle = await promises.open(file, "r"); + expect({ + utimesAtime: await rejectedBy(() => promises.utimes(file, "bad", 1)), + utimesMtime: await rejectedBy(() => promises.utimes(file, 1, {} as any)), + lutimesAtime: await rejectedBy(() => promises.lutimes(file, "bad", 1)), + lutimesMtime: await rejectedBy(() => promises.lutimes(file, 1, {} as any)), + handleAtime: await rejectedBy(() => handle.utimes("bad", 1)), + handleMtime: await rejectedBy(() => handle.utimes(1, {} as any)), + }).toEqual({ + utimesAtime: invalidTime("time", "type string ('bad')"), + utimesMtime: invalidTime("time", "an instance of Object"), + lutimesAtime: invalidTime("time", "type string ('bad')"), + lutimesMtime: invalidTime("time", "an instance of Object"), + handleAtime: invalidTime("atime", "type string ('bad')"), + handleMtime: invalidTime("mtime", "an instance of Object"), + }); + }); + + it("a missing time is reported as undefined, as in node", () => { + expect({ + utimesNone: thrownBy(() => (fs.utimesSync as any)(file)), + utimesOne: thrownBy(() => (fs.utimesSync as any)(file, 1)), + lutimesNone: thrownBy(() => (fs.lutimesSync as any)(file)), + futimesNone: thrownBy(() => (fs.futimesSync as any)(fd)), + futimesOne: thrownBy(() => (fs.futimesSync as any)(fd, 1)), + }).toEqual({ + utimesNone: invalidTime("time", "undefined"), + utimesOne: invalidTime("time", "undefined"), + lutimesNone: invalidTime("time", "undefined"), + futimesNone: invalidTime("atime", "undefined"), + futimesOne: invalidTime("mtime", "undefined"), + }); + }); + + it("futimes reports atime before looking at mtime", () => { + expect(thrownBy(() => fs.futimesSync(fd, "bad", {} as any))).toEqual(invalidTime("atime", "type string ('bad')")); + }); +}); + it("createReadStream on a large file emits readable event correctly", () => { return new Promise((resolve, reject) => { const tmp = mkdtempSync(`${tmpdir()}/readable`);