[robobun]

Problem

• Errors from fd-based node:fs calls (readSync, writeSync, fstatSync, ftruncateSync, fchmodSync, fchownSync, futimesSync, fsyncSync, and their async forms) have fd: undefined. Bun 1.3.0 set err.fd on all of them. readvSync, writevSync, fdatasyncSync and closeSync still do.
• The POSIX wrappers in src/sys/lib.rs build the error with only the errno and the syscall tag: Error::from_code_int(e, Tag::read) at src/sys/lib.rs:2019, and the same shape for write, pread, pwrite, fstat, fstatx, fchmod, fchown, ftruncate, futimens, fsync, lseek, recv, send and pwritev. Nothing calls .with_fd(fd). The Windows wrappers in src/sys/sys_uv.rs do call it.

Fix

• Add check_fd! and check_once_fd! next to the existing check_p! and check_once_p! macros in posix_impl. They attach the fd the same way the path variants attach the path.
• Use them in every wrapper above. The Linux map_err arms and the top-level pwritev get .with_fd(fd) directly, like readv and writev already do.
• statx_impl attaches the fd only for fstatx. statx and lstatx pass AT_FDCWD plus a path, and node_fs.rs attaches the path for those.
• node_fs.rs fsync now uses errno_sys_fd, like fdatasync and the Windows path. The unused errno_sys helper is removed.
• Verified: test/js/node/fs/fs.test.ts (13 new cases, 9 fail on bun 1.4.1). Also test/js/node/fs/promises.test.js, fs-stat-seccomp-linux.test.ts, fs-write-offset-bound.test.ts, test/js/bun/io/bun-write.test.js, test/js/bun/util/bun-file.test.ts, error-name-preservation.test.ts.

Background

• bun_sys::Error (src/sys/Error.rs) is the syscall error. It carries errno, syscall, path, dest and fd. to_system_error copies each field that is set onto the JS SystemError, so err.fd in JS exists only when the wrapper sets fd.
• with_fd and with_path are the two builders for that context. The path-based wrappers use check_p!, which calls with_path. Before this change there was no fd equivalent, so the fd wrappers used plain check!.
• The macOS arms call the $NOCANCEL syscalls once and do not retry on EINTR. check_once_fd! is the single-shot form for those arms. The old check_once! had no other users and is removed.

Notes

Repro on bun 1.4.1:

const fs = require("fs");
const fd = fs.openSync("/etc/hostname", "r");
try { fs.writeSync(fd, "x"); } catch (e) { console.log(e.code, "fd" in e, e.fd); }
// EBADF false undefined

bun 1.3.0 prints EBADF true <fd>. Node does not set err.fd on these errors at all, so this restores Bun's own behavior rather than matching Node.

node_fs.rs pread_inner and pwrite_inner rebuild the error with fd themselves, which is why readSync and writeSync with a position kept err.fd. They are unchanged.

with_fd has a debug_assert!(fd != Fd::INVALID). Every wrapper changed here receives a validated fd: node:fs rejects negative fds with ERR_OUT_OF_RANGE before the syscall, and the internal callers (Blob, FileSink, copy_file, the resolver) check for Fd::INVALID before they read or write. The Windows wrappers have used with_fd on these same paths all along. #31165 proposes to drop that assertion and is independent of this change.

The fchownSync case is skipped on Windows because libuv implements uv_fs_fchown as a no-op there.

cargo check -p bun_sys passes for x86_64-unknown-linux-gnu, aarch64-apple-darwin and x86_64-unknown-freebsd.

test/js/bun/io/bun-write.test.js "on large files" timed out at 5 s once under the debug ASAN build (a 256 MB copy with copy_file_range disabled). It exercises a success path only and is unrelated to this change.

---

no test proof · iteration 2 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/node/fs/fs.test.ts

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{On-demand reviews are free for the next 24 days. After that, they cost $0.25 per reviewed file.}

Or wait 5 minutes for your next included review.

View limit details

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

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 812414fb-0bd6-4cdf-80a8-749231479ce0

📥 Commits

Reviewing files that changed from the base of the PR and between a92d84e and bcc2904.

📒 Files selected for processing (3)

• src/runtime/node/node_fs.rs
• src/sys/lib.rs
• test/js/node/fs/fs.test.ts

---

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

[robobun]

^{Updated 11:50 PM PT - Aug 27th, 2026}

❌ @robobun, your commit bcc2904 has 1 failures in Build #107404 (All Failures):

• test/js/web/url/url.test.ts - code 1: TypeError: Invalid URL on 🍎 any x64

---

🧪   To try this PR locally:

bunx bun-pr 40712

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

bun-40712 --bun

[robobun]

Status: ready for review.

Repro: fs.writeSync(fd, "x") on a read-only fd throws EBADF with err.fd === undefined on bun 1.4.1. bun 1.3.0 sets err.fd. The new cases in test/js/node/fs/fs.test.ts fail on 1.4.1 and pass with this branch.

CI on bcc2904 (build 107404): 180 of 181 jobs pass. The one failure is test/js/web/url/url.test.ts on darwin x64 (special-scheme hosts use the Unicode 16 IDNA table, TypeError: Invalid URL). That test fails on main too and does not touch this change.

[claude]

LGTM — the Windows readv/writev syscall-tag branching addresses my earlier note, and the rest is a mechanical .with_fd(fd) mirror of the existing .with_path() pattern.

What was reviewed: Checked every check! → check_fd! / check_once! → check_once_fd! swap preserves the EINTR-retry vs single-shot semantics per arm. Confirmed the removed check_once! and MaybeSysResultExt::errno_sys have no remaining callers (the errno_sys hits in sys/lib.rs, sys/windows/mod.rs, posix_event_loop.rs, udp_socket.rs are separate definitions). Looked at whether the "writeSync on a read-only fd" test would see EPERM instead of EBADF on Windows — ruled out. Verified fstatx's .with_fd(fd) is applied only on the fd-taking wrapper, not statx/lstatx which pass AT_FDCWD + path.

Extended reasoning...

Overview

This PR attaches the file descriptor to bun_sys::Error at every fd-taking POSIX syscall wrapper in src/sys/lib.rs, so JS-visible node:fs errors carry err.fd. It introduces check_fd! (EINTR-retrying) and macOS-only check_once_fd! (single-shot for $NOCANCEL arms) that mirror the existing check_p!/check_once_p! path-attaching macros, and swaps them in at read/write/pread/pwrite/fstat/fchmod/fchown/ftruncate/futimens/fsync/lseek/recv/send/pwritev plus the corresponding linux_syscall::* .map_err arms. fstatx maps its error through .with_fd(fd). In node_fs.rs, _fsync switches to errno_sys_fd and the now-unused errno_sys trait method is removed. The dead check_once! macro is deleted. Tests add a table-driven describe block in fs.test.ts covering 12 fd-based sync calls on a bad fd plus a real read-only fd case.

Security risks

None. The change only adds metadata (fd) to already-constructed error objects on failure paths; no new syscalls, no input parsing, no auth/crypto surface. The fd value echoed back is one the caller already supplied.

Level of scrutiny

Moderate, focused on cross-platform consistency. Each wrapper has 2-3 #[cfg(...)] arms (macOS $NOCANCEL, Linux/Android direct syscall, other-POSIX libc), and each arm was touched. I verified the retry-vs-single-shot semantics were preserved in every arm by comparing the old macro's body against the new one — check_fd! is byte-identical to check! plus .with_fd($fd), and check_once_fd! is byte-identical to the removed check_once! plus .with_fd($fd). The statx_impl change from struct-init to Error::from_code_int is behaviorally equivalent (same fields set). The deleted EINTR explanatory comment's content survives on the check_once_p! line comment.

Other factors

My earlier inline comment (readvSync/writevSync asserting syscall: "readv"/"writev" on Windows where sys_uv::preadv/pwritev tag as read/write) was addressed in a follow-up commit with isWindows ? "read" : "readv" branching. The one candidate this run investigated — whether the read-only-fd writeSync test would see EPERM on Windows — was ruled out. Grep confirms no remaining callers of the removed check_once! macro or the MaybeSysResultExt::errno_sys trait method (other errno_sys hits are unrelated free functions or a separate Windows-only trait on bun_sys::Result). The github-actions bot's inline notes at src/sys/lib.rs were followed by author commits and the current diff shows no residual issue at those lines. The change is mechanical enough, and mirrors an established in-tree pattern closely enough, that I'm comfortable approving.