Skip to content

node:fs: convert Stats date fields like Node.js - #42564

Closed
robobun wants to merge 5 commits into
mainfrom
robobun/fdec675e/fs-stats-date-round
Closed

robobun wants to merge 5 commits into
mainfrom
robobun/fdec675e/fs-stats-date-round

Conversation

@robobun

@robobun robobun commented Sep 13, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

  • The atime, mtime, ctime and birthtime getters of fs.Stats and BigIntStats convert *Ms to a Date differently from Node.js. A real fs.stat() result with a fraction of 0.5 ms or more gives a Date 1 ms early. A BigInt at or past 2^63 wraps to 64 bits. A Number *Ms on BigIntStats, or a BigInt *Ms on Stats, throws TypeError: Conversion from 'BigInt' to 'number' is not allowed.
  • The cause is getDateField in src/jsc/bindings/NodeFSStatBinding.cpp:233. It calls toBigInt64 for one class and toNumber for the other, with no round. DateInstance::create then truncates.

Fix

  • getDateField now does what Node's dateFromMs does: new Date(MathRound(Number(ms))). It calls toNumeric, converts a BigInt result with JSBigInt::toNumber, and rounds with jsRound (the Math.round implementation, so -0.5 gives 0 and -1.5 gives -1).
  • A BigInt past 2^63 becomes a double past the Date range, so TimeClip gives an Invalid Date. Both classes accept a Number or a BigInt, and each getter works on an object of the other class.
  • Verified: test/js/node/fs/fs-stats-constructor.test.ts (4 new tests fail on 1.4.3, pass with the fix). Also fs.test.ts, fs-stats-truncate.test.ts, and the upstream test-fs-stat.js, test-fs-stat-bigint.js and test-fs-stat-date.mjs.

Background

  • Stats and BigIntStats store atimeMs and friends as own properties. The Date getters live on the prototype, compute the Date from the *Ms field, and cache it on the instance. Node does the same with ObjectDefineProperty in lib/internal/fs/utils.js.
  • toNumeric is the ECMAScript ToNumeric operation. It returns a Number or a BigInt and throws for a Symbol. Number(x) is ToNumeric followed by a BigInt to double conversion, which is what JSBigInt::toNumber does.
  • jsRound is the function behind Math.round in JavaScriptCore. std::round differs for negative halves.
Notes

Node reference: https://github.com/nodejs/node/blob/v26.3.0/lib/internal/fs/utils.js#L459-L469

The BigIntStats *Ms fields are integer BigInts (the fraction lives in *Ns), so a real BigInt stat is not affected by the round. The test asserts that too.

#42518 changes how the getters store the cached Date on the instance. It does not touch the conversion, so the two changes do not overlap in behaviour.


[human-review] gate passed · iteration 1 · 2 files touched

fails on main (without fix)
ASAN without fix: 4 FAILED
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/js/node/fs/fs-stats-constructor.test.ts
bun test v1.4.3 (b99371011)

test/js/node/fs/fs-stats-constructor.test.ts:
(pass) new Stats(...) assigns fields in Node's order [12.95ms]
(pass) Stats(...) without new assigns fields in Node's order [3.13ms]
(pass) Stats instances share Stats.prototype [28.00ms]
75 |     expect({
76 |       mtime: stats.mtime.getTime(),
77 |       atime: stats.atime.getTime(),
78 |       bigintMtimeMs: bigint.mtimeMs,
79 |       bigintMtime: bigint.mtime.getTime(),
80 |     }).toEqual({
            ^
error: expect(received).toEqual(expected)

  {
-   "atime": 1700000000790,
+   "atime": 1700000000789,
    "bigintMtime": 1700000000789,
    "bigintMtimeMs": 1700000000789n,
-   "mtime": 1700000000790,
+   "mtime": 1700000000789,
  }

- Expected  - 2
+ Received  + 2

      at <anonymous> (/workspace/bun/test/js/node/fs/fs-stats-constructor.test.ts:80:8)
(fail) Stats date getters convert *Ms like Node.js > a real stat rounds the fraction instead of truncating it [42.69ms]
91 |       // The getter caches the Date o
... (truncated)

release without fix: all passed
bun test v1.4.3-canary.1 (08fa3e246)

test/js/node/fs/fs-stats-constructor.test.ts:
(pass) new Stats(...) assigns fields in Node's order [0.05ms]
(pass) Stats(...) without new assigns fields in Node's order [0.06ms]
(pass) Stats instances share Stats.prototype [0.17ms]
(pass) Stats date getters convert *Ms like Node.js > a real stat rounds the fraction instead of truncating it [0.87ms]
(pass) Stats date getters convert *Ms like Node.js > a Number *Ms rounds like Math.round [0.11ms]
(pass) Stats date getters convert *Ms like Node.js > a BigInt *Ms past 64 bits is an Invalid Date, not a wrapped value [0.06ms]
(pass) Stats date getters convert *Ms like Node.js > both classes accept a Number or a BigInt *Ms [0.09ms]
(pass) Stats date getters convert *Ms like Node.js > a *Ms that is not numeric still throws from the coercion [0.11ms]
(pass) Stats methods and accessors called without a receiver > a mode method called through a captured binding is treated like an undefined receiver [0.09ms]
(pass) Stats methods and accessors called without a receiver > a date getter called through a captured binding is treated like an undefined receiver [0.08ms]
(pass) Stats methods and ac
... (truncated)
passes on PR (with fix)
ASAN with fix: all passed
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/js/node/fs/fs-stats-constructor.test.ts
bun test v1.4.3 (b99371011)

test/js/node/fs/fs-stats-constructor.test.ts:
(pass) new Stats(...) assigns fields in Node's order [11.28ms]
(pass) Stats(...) without new assigns fields in Node's order [2.54ms]
(pass) Stats instances share Stats.prototype [25.29ms]
(pass) Stats date getters convert *Ms like Node.js > a real stat rounds the fraction instead of truncating it [33.02ms]
(pass) Stats date getters convert *Ms like Node.js > a Number *Ms rounds like Math.round [6.08ms]
(pass) Stats date getters convert *Ms like Node.js > a BigInt *Ms past 64 bits is an Invalid Date, not a wrapped value [4.01ms]
(pass) Stats date getters convert *Ms like Node.js > both classes accept a Number or a BigInt *Ms [6.13ms]
(pass) Stats date getters convert *Ms like Node.js > a *Ms that is not numeric still throws from the coercion [7.40ms]
(pass) Stats methods and accessors called without a receiver > a mode method called through a captured binding is treated like an undefined receiver [7.20ms]
(pass) Stats m
... (truncated)

release with fix: all passed
$ bun scripts/build.ts --profile=release
[configured] bun-profile → bun (stripped) in 752ms (unchanged)
ninja: Entering directory `/workspace/bun/build/release'
[1/7] cxx obj/unified/UnifiedSource-src_jsc_bindings-3.cpp.o
[2/7] gen cpp.rs (cppbind)
[2/7] cargo bun_runtime → libbun_runtime.a
�[1m�[92m   Compiling�[0m bun_core v0.0.0 (/workspace/bun/src/bun_core)
�[1m�[92m   Compiling�[0m bun_errno v0.0.0 (/workspace/bun/src/errno)
�[1m�[92m   Compiling�[0m bun_ptr v0.0.0 (/workspace/bun/src/ptr)
�[1m�[92m   Compiling�[0m bun_boringssl_sys v0.0.0 (/workspace/bun/src/boringssl_sys)
�[1m�[92m   Compiling�[0m bun_safety v0.0.0 (/workspace/bun/src/safety)
�[1m�[92m   Compiling�[0m bun_base64 v0.0.0 (/workspace/bun/src/base64)
�[1m�[92m   Compiling�[0m bun_cares_sys v0.0.0 (/workspace/bun/src/cares_sys)
�[1m�[92m   Compiling�[0m bun_zlib_sys v0.0.0 (/workspace/bun/src/zlib_sys)
�[1m�[92m   Compiling�[0m bun_zstd v0.0.0 (/workspace/bun/src/zstd)
�[1m�[92m   Compiling�[0m bun_picohttp v0.0.0 (/workspace/bun/src/picohttp)
�[1m�[92m   Compiling�[0m bun_brotli v0.0.0 (/workspace/bun/src/brotli)
�[1m�[92m   Compiling�[0m bun_output v0.0.0 (/workspace/bun/src/output)
�[1m�[92
... (truncated)
diff hotspot
src/jsc/bindings/NodeFSStatBinding.cpp       |  8 ++-
 test/js/node/fs/fs-stats-constructor.test.ts | 92 +++++++++++++++++++++++++++-
 2 files changed, 97 insertions(+), 3 deletions(-)

gate history · 2 passed · 0 rejected · iteration 1

evidence per changed file
file                                          reads  edits  tests
src/jsc/bindings/NodeFSStatBinding.cpp            1      2     10
test/js/node/fs/fs-stats-constructor.test.ts      2      7     10

root cause · written by the author bot

The date getters of fs.Stats and BigIntStats in NodeFSStatBinding.cpp converted the *Ms field with toBigInt64 for BigInt stats and toNumber for Number stats, then handed the result straight to the Date constructor. That meant the millisecond value was truncated rather than rounded, BigInt values at or beyond 2^63 wrapped to 64 bits instead of producing an Invalid Date, and a *Ms value of the other numeric type threw a conversion error. The fix uses a single shared coercion for both classes that converts a BigInt to a Number the way Number(x) does, applies Math.round, and…

The atime, mtime, ctime and birthtime getters of Stats and BigIntStats
now compute new Date(Math.round(Number(ms))), the same as Node.js.
A value with a fraction of 0.5 ms or more rounds up instead of being
truncated. A BigInt past 64 bits becomes an Invalid Date instead of a
wrapped value. Both classes accept a Number or a BigInt *Ms field.

Fixes #42562
@robobun

robobun commented Sep 13, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 7:15 AM PT - Oct 28th, 2026

❌ @robobun, your commit f01089c has 5 failures in Build #134358 (All Failures):

  • test/aot/generated-births-and-uses-of-one-use-of-two-kinds-of-value-3.test.ts - code 1 on 🐧 13 aarch64 - test-aot-drawn
  • test/aot/perf/generated-code-of-operations-4.test.ts - code 1 on 🐧 13 x64 - test-aot-drawn
  • test/aot/perf/generated-code-of-operations-3.test.ts - code 1 on 🐧 13 x64 - test-aot-drawn
  • test/aot/perf/generated-code-of-operations-1.test.ts - code 1 on 🐧 13 aarch64 - test-aot-drawn
  • Failed to create agent for 🐧 3.23 aarch64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.325c3488-12d1-4ac7-be80-2298f12df67c
  • Failed to create agent for 🐧 3.23 aarch64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.b10c0e49-6484-4ce1-ab77-a4523aa3ce84
  • Failed to create agent for 🐧 3.23 aarch64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.5049b5d0-3fa2-4dae-ba73-4330cf6310d1
  • Failed to create agent for 🐧 25.04 aarch64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.da69c721-6e00-4e6b-b3e0-1b3283ebbba6
  • Failed to create agent for 🐧 13 aarch64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.5c553973-5f85-4979-824b-c7d047a99cc1
  • Failed to create agent for 🐧 25.04 aarch64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.0f5e0c72-fb6d-435b-b1f9-0acbd4493935
  • Failed to create agent for 🐧 25.04 aarch64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.c774c76a-c96a-47f4-a368-62ae5ba08f1c
  • Failed to create agent for 🐧 25.04 aarch64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.2273ca28-1524-4789-9ec7-65da4afa64cd
  • Failed to create agent for 🐧 25.04 aarch64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.823e6962-e08d-4369-8596-04562dfc1f28
  • Failed to create agent for 🐧 25.04 aarch64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.a42ef20b-fcff-42d2-868a-66d369b6d131
  • Failed to create agent for 🐧 13 aarch64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.3c4bfccb-36c0-4a89-9bb2-3693621a2e88
  • Failed to create agent for 🐧 13 aarch64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.1a10b913-21e6-4f79-b989-c0b6d4d91034
  • Failed to create agent for 🐧 13 aarch64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.a256544a-589c-4ead-89d7-0cf70cb23d3e
  • Failed to create agent for 🐧 13 aarch64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.abcf06ab-1270-44f0-bf17-980104046938
  • Failed to create agent for 🐧 13 aarch64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.2c8b91e7-0570-450c-a7a7-3ed27f82f5ff
  • Failed to create agent for 🐧 25.04 aarch64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.cb3eb6ec-b62d-425d-91a2-8bc8595e11ef
  • Failed to create agent for 🐧 13 aarch64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.ae72316e-4753-448f-81b5-565eb8cfd44e
  • Failed to create agent for 🐧 13 aarch64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.09c829e2-9f74-4e01-9049-f4c422b48d2d
  • Failed to create agent for 🐧 13 aarch64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.80087a79-9956-41d0-90dd-caa3f78ed0dc
  • Failed to create agent for 🐧 25.04 aarch64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.3c475b1a-9273-4b8e-813f-450961c12f16
  • Failed to create agent for 🐧 25.04 aarch64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.bb5a0413-4789-45f4-9fb6-759650898837
  • Failed to create agent for 🐧 13 aarch64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.80d6adfe-66e4-4d8a-978c-fe93010f741f
  • Failed to create agent for 🐧 13 aarch64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.decdd986-cc5c-4abd-95a3-7f6870d6cf96
  • Failed to create agent for 🐧 25.04 aarch64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.b436173f-9f19-4a62-8e0c-68f22bf53031
  • Failed to create agent for 🐧 13 aarch64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.cbc00109-c17b-460f-ae05-b8fed4e990d0
  • Failed to create agent for 🐧 25.04 aarch64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.b03794c1-1def-4690-b529-adca32ec03d5
  • Failed to create agent for 🐧 25.04 aarch64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.e4747e1a-d0f0-429a-bab9-b150b564a134
  • Failed to create agent for 🐧 13 aarch64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.d6da6de6-85e4-4b00-955c-cb4f7e42ff93
  • Failed to create agent for 🐧 13 aarch64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.8049114b-4455-49f9-8680-31a295f187b4
  • Failed to create agent for 🐧 25.04 aarch64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.9a26a55e-473f-4ed4-9d64-2f8ff902b439
  • Failed to create agent for 🐧 13 aarch64 - test-aot-drawn
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.d4ad77c6-bd08-4ab1-8b65-2438aef5d8f6
  • Failed to create agent for 🐧 13 aarch64 - test-aot-drawn
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.c6e8e1b7-efab-410c-80be-ffdb52c738d3
  • Failed to create agent for 🐧 13 aarch64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.9c65aa48-5496-420e-819d-7463fec68bf8
  • Failed to create agent for 🐧 25.04 aarch64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.331e5d16-b7de-46e0-9cd8-6f9fc24f570d
  • Failed to create agent for 🐧 13 aarch64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.80f4fcc4-3bb3-494e-870c-31246175568b
  • Failed to create agent for 🐧 13 aarch64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.7cab3f14-d3aa-43b6-ba23-d61b5027cd90
  • Failed to create agent for 🐧 25.04 aarch64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.666e92b5-e8e6-4d6a-a175-473e25b4d17d
  • Failed to create agent for 🐧 25.04 aarch64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.ce05ed23-442b-4911-967c-cc4cf6f1303b
  • Failed to create agent for 🐧 13 aarch64 - test-aot
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.c7d56606-fb74-4e9c-af45-ecb342ec40d3
  • Failed to create agent for 🐧 25.04 aarch64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.cee45cfb-bbd7-46e3-8a18-59de00c2cf19
  • Failed to create agent for 🐧 13 aarch64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.016a77ae-f491-49d0-88f0-ffc118447bb0
  • Failed to create agent for 🐧 13 aarch64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.ef49e052-f19e-4bfc-b7e7-fccb93d5a7e5
  • Failed to create agent for 🐧 13 aarch64 - test-aot-drawn
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.899a2eec-8adf-435f-b0e2-3139e234051b
  • Failed to create agent for 🐧 13 aarch64 - test-aot-drawn
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.70c4fe1c-4d89-46df-b666-90bfb26ce7a9
  • Failed to create agent for 🐧 aarch64 - verify-baseline
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.e4bfb845-96df-4b02-8434-4d1e3c51f473
  • Failed to create agent for 🐧 13 aarch64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.92ff2590-f31c-40eb-8733-51cd6fa7767d
  • Failed to create agent for 🐧 25.04 aarch64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.693c25de-f3b1-48cb-b06f-c6d64b848174
  • Failed to create agent for 🐧 25.04 aarch64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.33455fc2-6243-497f-988a-8fd76b97f830
  • Failed to create agent for 🐧 3.23 x64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.d92a5a00-309b-4cbb-87d4-ab603478b3fd
  • Failed to create agent for 🐧 3.23 x64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.958d672c-0fc7-4efc-bdc1-50c086d4ebe4
  • Failed to create agent for 🐧 x64-musl - verify-baseline
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.ae8c6f5a-86eb-45d6-9a16-8b00d1d59716
  • Failed to create agent for 🐧 3.23 x64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.9df83f96-0fc3-4ef6-af51-f2d747a17e40
  • Failed to create agent for 🐧 3.23 x64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.344d51fe-e3a0-499a-8f86-e4da7488172b
  • Failed to create agent for 🐧 3.23 x64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.26611076-7740-4bf5-9ef4-3e372a3557e4
  • Failed to create agent for 🐧 3.23 x64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.7f18c429-710e-4321-8740-dd9bba13cac6
  • Failed to create agent for 🐧 3.23 x64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.c4ee8cab-8e17-4c6f-8e36-57823804fe99
  • Failed to create agent for 🐧 3.23 x64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.e3815e6f-1698-4250-8dec-6d55315250ca
  • Failed to create agent for 🐧 3.23 x64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.e21c258a-9ed7-4fb9-a3eb-314598ba9cf7
  • Failed to create agent for 🐧 3.23 x64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.f28c1e02-69b1-4292-b2b7-2db9c90aeacd
  • Failed to create agent for 🐧 3.23 x64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.12767e2e-44fd-4857-b0e4-560b259208f3
  • Failed to create agent for 🐧 3.23 x64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.fa3fcf3d-d446-4d7b-85f6-50476d309af0
  • Failed to create agent for 🐧 3.23 x64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.bc50ab6c-ce68-4692-9ec4-801669499dc8
  • Failed to create agent for 🐧 3.23 x64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.74519c34-da2e-4701-a21e-066ddb7f68ab
  • Failed to create agent for 🐧 3.23 x64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.1bdd6f10-8681-4405-a4b8-e46aa53da542
  • Failed to create agent for 🐧 3.23 x64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.23f9d0c1-9a35-4e04-b707-cec158eab5ad
  • Failed to create agent for 🐧 3.23 x64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.c3eb5b43-e4ae-4f12-b696-312bd9ff9658
  • Failed to create agent for 🐧 3.23 x64 - test-bun
  • CapacityLimitExceededThis account has reached its capacity limit. Reach out to your account team for assistance.6ff4e978-0dd1-4632-8f33-37c3421831f4

🧪   To try this PR locally:

bunx bun-pr 42564

That installs a local version of the PR into your bun-42564 executable, so you can run:

bun-42564 --bun

@coderabbitai

coderabbitai Bot commented Sep 13, 2026 •

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 37 seconds.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

Run configuration
Commits

Reviewing files that changed from the base of the PR and between b10313b and f01089c.

Files selected for processing: 2
  • src/jsc/bindings/NodeFSStatBinding.cpp
  • test/js/node/fs/fs-stats-constructor.test.ts

No actionable comments were generated in the recent review.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Essentials

Run ID: c9c6ed7d-f262-4caf-ad35-1fa0a9d743f3

📥 Commits

Reviewing files that changed from the base of the PR and between 48519f2 and b10313b.

📒 Files selected for processing (1)
  • test/js/node/fs/fs-stats-constructor.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 1 remains after this review.

Walkthrough

The change updates Stats and BigIntStats date conversion to use shared numeric coercion, BigInt-to-Number conversion, and rounding. Tests cover filesystem timestamps, numeric boundaries, cross-type assignments, oversized values, and Symbols.

Changes

Filesystem statistics date conversion

Layer / File(s) Summary
Numeric date conversion and validation
src/jsc/bindings/NodeFSStatBinding.cpp, test/js/node/fs/fs-stats-constructor.test.ts
Date fields accept Number and BigInt values through shared numeric coercion, convert BigInts to Numbers, round the result, and construct the Date. Tests cover filesystem timestamps, fractional values, boundaries, oversized BigInts, cross-type assignments, and TypeError propagation for Symbols.

Priority: Low

Merge Risk: Minimal · previous review b1031

The reviewed date-conversion paths have no confirmed contract violation or merge-blocking risk.

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly and concisely describes the primary change: updating Node.js filesystem Stats date-field conversion behavior.
Description check ✅ Passed The description fully explains the problem, fix, behavior changes, and verification results. It does not use the template headings exactly, but it provides the required information in clearly labeled …
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@robobun

robobun commented Sep 13, 2026

Copy link
Copy Markdown
Collaborator Author

The automated review raised no actionable items. No open review threads. CI build #115030 is running.

@robobun

robobun commented Sep 13, 2026

Copy link
Copy Markdown
Collaborator Author

The second automated review covered 48519f2 (autofix removed an unused import from the test). No actionable items. Waiting on CI build #115030.

@robobun

robobun commented Sep 13, 2026

Copy link
Copy Markdown
Collaborator Author

The automated review of 08fa3e2 hit its rate limit. That commit only changes the test: it no longer assumes a nanosecond timestamp resolution for mtimeMs (NTFS stores 100 ns). No open review threads. CI build #115031 is running.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

@robobun

robobun commented Sep 13, 2026

Copy link
Copy Markdown
Collaborator Author

The one review nit (an unused writeFileSync import in the test) was already removed in 48519f2. The thread is resolved. Nothing else is open.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — the unused writeFileSync import from the earlier review is gone, and I have nothing further.

What was reviewed:

  • getDateField exception handling: toNumeric is followed by RETURN_IF_EXCEPTION; JSBigInt::toNumber cannot throw; asNumber() is safe because ToNumeric guarantees Number-or-BigInt and the BigInt branch converts first.
  • Semantics vs Node: Number(x) is spec'd as ToNumeric + BigInt→double (ES2020+), so toNumeric + JSBigInt::toNumber + jsRound is exactly new Date(MathRound(Number(ms))); the isBigInt template param is still live for the structure fast-path at line 227.
  • Real-stat test portability: the mtimeMs assertion is a range (≥789.5, <790) that holds on ext4/tmpfs/APFS/NTFS, and utimesSync sets atime explicitly so noatime mounts don't affect it.
Extended reasoning...

Overview

The PR changes one templated helper, getDateField, in src/jsc/bindings/NodeFSStatBinding.cpp (6 lines) so that fs.Stats/BigIntStats date getters compute new Date(Math.round(Number(ms))) like Node.js does, instead of branching on the class and either wrapping past 64 bits (toBigInt64) or truncating (toNumber → DateInstance::create). It adds a describe block with five tests to test/js/node/fs/fs-stats-constructor.test.ts covering a real stat with a fractional millisecond, the full Math.round input space (±0.5, ±1.5, past-Date-range, Infinity, NaN), BigInts past 2⁶³/2⁶⁴, cross-acceptance of Number/BigInt on either class (including via detached prototype getters), and the Symbol → TypeError path.

Security risks

None. This is a numeric coercion path for filesystem timestamp fields; no auth, crypto, path handling, or untrusted-length arithmetic is involved. The only user-reachable throw is the existing ToNumeric TypeError for Symbol, which is the Node-matching contract.

Level of scrutiny

Low-to-moderate. The C++ change is tiny and mechanical, but it's in a JSC binding where exception-scope discipline matters. I verified: toNumeric is followed by RETURN_IF_EXCEPTION; JSBigInt::toNumber is a pure BigInt→double conversion that cannot throw; after ToNumeric the result is guaranteed Number-or-BigInt by spec, so numeric.asNumber() after the BigInt branch is a validated (not blind) accessor call. I also confirmed against the ES2020+ spec that Number(value) is defined as ToNumeric followed by BigInt→Number, so the PR's toNumeric + JSBigInt::toNumber composition is exactly Number(ms) — including for objects whose valueOf returns a BigInt. jsRound gives Math.round's half-toward-+∞ (so -0.5 → -0, -1.5 → -1), which is what the tests assert and what Node's MathRound does. The isBigInt template parameter is not made dead — it still selects the structure for the fast-path getDirect at line 227.

Other factors

Since the previous review, commit b10313b dropped the unused writeFileSync import I flagged; the current diff imports only Stats, statSync, utimesSync. The tests follow the repo conventions the review rules call out: tempDir from harness, combined-object .toEqual assertions, specific TypeError class on the throw path, and a fresh Stats per input to defeat the getter's own-property cache. The one filesystem-touching test writes its mtimeMs assertion as a range that accounts for the float-precision loss in utimesSync's seconds→nanoseconds conversion (the comment names 789.5999 on Linux and 789.6 on Windows), and only checks mtime/atime — not birthtime, which varies by platform. The remaining four tests overwrite *Ms on a stat of import.meta.path, so they're hermetic and resolution-independent.

src/jsc/bindings/NodeFSStatBinding.cpp: both sides added includes at the
same place. Keep both.
@robobun

robobun commented Oct 28, 2026

Copy link
Copy Markdown
Collaborator Author

Build #134190 could not start any build agent (EC2 CapacityLimitExceeded on all six build lanes), so no test ran on the merge commit f01089c. The earlier build #115031 ran the full suite on the same diff: fs-stats-constructor.test.ts passed on every lane, and the only red suites were bun install tests that hit 504s from api.github.com. The diff is ready for review. A maintainer can restart the build when agent capacity is back.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — the earlier nit (unused writeFileSync import) is addressed and the fix is small and self-contained.

What was reviewed:

  • getDateField in src/jsc/bindings/NodeFSStatBinding.cpp: toNumeric is followed by RETURN_IF_EXCEPTION, and the BigInt-to-Number branch before asNumber() mirrors the existing IDLLooseDoubleOrBigInt converter in src/jsc/bindings/Bindgen/IDLConvertNumbers.h, so asNumber() never sees a non-Number.
  • All eight date getters (both Stats and BigIntStats) route through this one template, so the whole sibling set gets the rounding fix.
  • Checked fs-stats-truncate.test.ts and the rest of test/ for assertions on the old truncating behavior — none found.
Extended reasoning...

The PR changes four lines in the shared getDateField template in src/jsc/bindings/NodeFSStatBinding.cpp so fs.Stats and fs.BigIntStats date getters compute new Date(Math.round(Number(ms))) like Node, and adds five tests in test/js/node/fs/fs-stats-constructor.test.ts covering a real utimesSync fixture, a Math.round table including the Date range edge, out-of-range BigInt, cross-class Number/BigInt acceptance, and the Symbol throw. It touches no security-sensitive surface. The coercion sequence is byte-for-byte the pattern already used in IDLConvertNumbers.h, the exception check is present, the only change since my last review is a merge from main plus removal of the import I flagged, and neither file is covered by a CODEOWNER, which decided approve.

@robobun

robobun commented Oct 28, 2026

Copy link
Copy Markdown
Collaborator Author

Closing: #47233 replaces this PR and #42563. It carries the conversion of *Ms by its value (no 64-bit wrap, either numeric type in both classes), without the Math.round. The round moves mtime.getTime() by 1 ms for each file whose mtimeMs fraction is 0.5 or more, so it waits for a maintainer's decision as one commit (eec912d, branch robobun/2cfe25fb/fs-stats-date-round). The measured costs of the round are in the Notes of #47233.

@robobun robobun closed this Oct 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant