Repository navigation
Conversation
…utimes/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.
|
Warning Review limit reached
Next review available in: 1 minute Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
Comment |
|
Status: reproduced on bun 1.4.0 and on main, fix pushed in this PR, self-review in progress. Reproduced by running |
Problem
fs.utimesSync(file, "bad", 1)throwsTypeError [ERR_INVALID_ARG_TYPE]: atime must be a number or a Date. Node v26.3.0 throwsThe "time" argument must be an instance of Date or an Time in seconds. Received type string ('bad'). Class andcodealready match; only the message differs.utimes,lutimesandfutimes, sync, callback,fs/promisesandFileHandle.utimes, foratimeand formtime. A missing argument saysatime is requiredwhere node says the same message withReceived undefined."time"inutimes/lutimesand"atime"/"mtime"only infutimes(and thereforeFileHandle.utimes); bun saidatime/mtimeeverywhere.args::Lutimes::from_js(also used for utimes,args::Utimesis an alias of it) andargs::Futimes::from_jsinsrc/runtime/node/node_fs.rsformat their own text withthrow_invalid_argumentsinstead of building theERR_INVALID_ARG_TYPEmessage with the received value.Fix
time_like::from_js_required(ctx, arguments, name)insrc/runtime/node/time_like.rs: eats the next argument (undefinedwhen it is missing, which is what node's validator sees too) and, when the existingtime_like::from_jsrejects it, throws throughJSGlobalObject::throw_invalid_argument_type_value2with node's expected-type phrase. The four sites innode_fs.rscall it; utimes/lutimes pass"time", futimes passes"atime"and"mtime".toUnixTimestampthrowsERR_INVALID_ARG_TYPE(name, ['Date', 'Time in seconds'], time), and node's list renderer prints that array asan instance of Date or an Time in seconds; thean Timeis node's own output and is reproduced on purpose (same approach as thean Promisemessage insrc/runtime/webcore/wasm_streaming.rs). TheReceived ...half comes from the samedetermineSpecificTypeport that every otherERR_INVALID_ARG_TYPEin bun uses, so it already matches node for strings, objects,null,undefined,NaN, bigints and so on.time_like::from_jsis untouched, only its rejection is reported differently.test/js/node/fs/fs.test.ts, newdescribe("utimes/lutimes/futimes reject an invalid time with node's message"): ten invalid values across utimes/lutimes/futimes for both atime and mtime, the callback forms,fs/promisesandFileHandle.utimes, missing arguments, and atime being reported before mtime. All 14 tests fail on bun 1.4.0 and pass with this change. Every expected string is what node v26.3.0 prints for the same call (see the details block).fs.test.tswith the debug build: 524 pass. The one failure is thereaddirSync ... x 100test hitting its 10s timeout under the debug build here; it does not touch this code.test-fs-utimes.js,test-fs-timestamp-parsing-error.js,test-fs-utimes-y2K38.js,test-fs-promises.jsand the fourtest-fs-cp-*-preserve-timestamps*tests still exit 0 (they cover the accepted inputs).fs._toUnixTimestamp(src/js/node/fs.ts). The realfs.utimes*functions validate innode_fs.rs, which is what this PR changes; the two do not touch the same lines.Background
ERR_INVALID_ARG_TYPEmessage shape: node rendersThe "<name>" argument must be <expected>. Received <value>.<expected>is built from an array of entries: class names becomean instance of X, any other phrase is printed as written withanin front when it starts with a capital, and the groups are joined withor.<value>comes fromdetermineSpecificType, e.g.type string ('bad'),an instance of Object,undefined. Bun ports both insrc/jsc/bindings/ErrorCode.cpp; from Rust,throw_invalid_argument_type_value2(name, phrase, value)takes the already rendered<expected>phrase and renders<value>through that port.toUnixTimestamp(time, name = 'time')in node'slib/internal/fs/utils.jsturns a finite number, a numeric string or aDateinto seconds and throws the error above for anything else.fs.utimesandfs.lutimescall it with the default name;fs.futimespasses'atime'and'mtime';FileHandle.utimesis implemented with futimes. Bun's equivalent istime_like::from_js, called from theargs::*::from_jsargument parsers that the sync, callback and promise forms of anode:fsfunction share, which is why one change covers every form.bun 1.4.0 vs node v26.3.0 (same output for the callback and promise forms)
A script exercising the six functions (sync, callback, promises, FileHandle) with 18 different invalid values prints identical output under this build and under node, with one pre-existing exception that this PR leaves alone: for an invalid
Date(new Date(NaN)) node passes NaN through to the binding and leaves the timestamp untouched, while bun rejects it (now with the message above,Received an instance of Date). That is a question of whether to throw, not of the message, so it is not covered by the new tests.