node:fs: use node's ERR_INVALID_ARG_TYPE message for invalid utimes/lutimes/futimes times

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<Lutimes> {
             // `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<Futimes> {
             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 })
         }
     }

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<Optio
     Ok(None)
 }
 
+/// Eats the next argument (a missing one is `undefined`, as in Node) and
+/// throws Node's `toUnixTimestamp` error when [`from_js`] rejects it.
+pub fn from_js_required(
+    global_object: &JSGlobalObject,
+    arguments: &mut ArgumentsSlice,
+    name: &str,
+) -> JsResult<TimeLike> {
+    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

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<unknown>) => {
+    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<void>((resolve, reject) => {
     const tmp = mkdtempSync(`${tmpdir()}/readable`);