Skip to content

fs: keep WriteStream short-write retries at the current offset - #36136

Closed
robobun wants to merge 1 commit into
mainfrom
farm/2fbac61b/writestream-short-write-nan-position
Closed

fs: keep WriteStream short-write retries at the current offset#36136
robobun wants to merge 1 commit into
mainfrom
farm/2fbac61b/writestream-short-write-nan-position

fs: keep WriteStream short-write retries at the current offset

e45615e
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed Jul 27, 2026 in 13m 50s

Code review found 1 potential issue

Found 3 candidates, confirmed 1. See review comments for details.

Details

Severity Count
🔴 Important 0
🟡 Nit 1
🟣 Pre-existing 0
Severity File:Line Issue
🟡 Nit src/runtime/node/node_fs.rs:10225-10238 fs.writev/readv position parser still coerces NaN to offset 0

Annotations

Check warning on line 10238 in src/runtime/node/node_fs.rs

See this annotation in the file changed.

@claude claude / Claude Code Review

fs.writev/readv position parser still coerces NaN to offset 0

Same-class site not covered: `FdVectorIo::from_js` (node_fs.rs:2829-2830, backing `fs.writev`/`writevSync`/`readv`/`readvSync`) still does `Some(pos_value.to_int64() as u64)` when `is_number()`, so `fs.writev(fd, bufs, NaN, cb)` still becomes `pwritev` at offset 0 instead of the current file offset. Node's `WriteBuffers`/`ReadBuffers` route position through the same `GetOffset` helper this PR mirrors, so `write_position_from_js` (adapted for `Option<u64>`) should replace the `to_int64()` coercio