node:fs: report mkdir/readdir/rm boolean option type errors like node by robobun · Pull Request #39328 · oven-sh/bun

robobun · 2026-08-16T07:30:56Z

Problem

fs.mkdirSync(p, { recursive: "x" }) throws ERR_INVALID_ARG_TYPE with The "recursive" property must be of type boolean, got string. Node v26.3.0 throws the same code with The "options.recursive" property must be of type boolean. Received type string ('x'). fs.mkdir and fs.promises.mkdir share the parser, as do fs.readdir/readdirSync/promises.readdir for recursive and withFileTypes.
fs.rmSync(p, { recursive: "x" }) and { force: "x" } (also fs.rm, fs.promises.rm) say The "options.recursive" property must be of type boolean. with no Received ... half; fs.rmSync(p, "x") and fs.rmdirSync(d, "x") say The "options" argument must be of type object. without it either. rmSync(p, { recursive: "x", force: "y" }) reports recursive; node reports force.
Causes, all in src/runtime/node/node_fs.rs: Mkdir::from_js (:3395) and Readdir::from_js (:3488, :3491) read the option through JSValue::get_boolean_strict, whose error is Bun's ..., got <typeof> template with the bare key; RmDir::from_js_impl (:3307-3323, :3346) formats its messages by hand. Node validates every one of these with validateBoolean(value, 'options.<name>') (and validateObject(options, 'options')).
The shared Rust renderer these sites need, JSGlobalObject::throw_invalid_argument_type_value (src/jsc/JSGlobalObject.rs:559), hardcodes "<name>" argument, so it could not produce "options.recursive" property either.
The codes were already right, which is why the vendored tests did not catch it: test-fs-mkdir.js carried a typeof Bun branch with the old wording and test-fs-rm.js matches the message with a prefix regex.

Fix

node_fs.rs: a small validate_boolean_option (node's validateBoolean) throws through throw_invalid_argument_type_value with the dotted name; mkdir's and readdir's recursive, readdir's withFileTypes, and rm/rmdir's force and recursive use it. rm's non-object options error goes through the same renderer. force is checked before recursive, the order of node's validateRmOptions.
JSGlobalObject.rs: the three throw_invalid_argument_type_value* renderers format the name through InvalidArgTypeName, node's subject rule ("x" argument, "a.b" property, a name ending in argument verbatim), which is the rule Bun::Message::addParameter in ErrorCode.cpp already applies to every C++ caller. This hunk is byte-identical to the one carried by the open node: render the Rust validators' ERR_INVALID_ARG_TYPE messages like Node #38663, node:fs: report a bad flush option as "options.flush", render dotted names as properties in ERR_INVALID_ARG_TYPE #39266 and node:fs: validate options.signal in readFile, writeFile and watch like node #39274, so these PRs merge in any order; whichever lands first carries it and the others reduce to their call sites.
Why this is right: for node:fs errors node's text is the contract, and the Received ... half already comes from the same determineSpecificType the JS builtins use, so the result is node's exact text. A 160-case matrix (mkdir, readdir, rm, rmdir; sync, callback and promise entry points; 8 non-boolean values each) against a local node v26.3.0 changes 122 lines with this PR, and every line where both runtimes throw now carries identical text; the lines still differing are pre-existing behavioral differences (which values readdir and rmdir accept, callback-form rm delivering the error through the callback where node throws synchronously, the stat family validating options node ignores), listed below and not touched here.
Which values are accepted does not change: the sites keep the same get/get_own lookups and the same is_boolean test, so rm's own-key recursive: undefined is still rejected (node's spread-over-defaults semantics) and mkdir's missing or undefined recursive is still the default. Only the text and the force/recursive order change.
Renderer blast radius: only names containing a . render differently. The ones passed today are options.retryDelay/options.maxRetries (fs.rm), options.address/options.flowlabel (net.SocketAddress), options.silent/endStream/exclusive/parent/weight/signal (http2 request options), handle.fd (cluster), and Bun's own compress.encoding (fetch) and rule.syscall/rule.action (socket fault injection). Node renders all of its own as property; no test in the repo pinned argument for any of them. Plain names render exactly as before.
Deliberately left alone: get_boolean_strict itself and its stat/fstat/statfs/watchFile callers (bigint, throwIfNoEntry, persistent), which node does not validate at all, so there is no node text to match; validators::validate_boolean (that is node: render the Rust validators' ERR_INVALID_ARG_TYPE messages like Node #38663's change); the behavioral deltas the matrix surfaced, which are tracked separately: readdir rejects recursive: null, non-boolean withFileTypes, and non-boolean recursive in promises.readdir where node coerces; rmdir still validates force/retryDelay/maxRetries, which node v26 ignores; and the JS rm wrappers report ERR_FS_EISDIR before the options are validated when the target is a directory. The draft node: restore 9 concealed-gap tests to verbatim upstream (arg-type messages, win32 paths, parseEnv) #35423 carries a broader variant of the mkdir/readdir recursive part (it renames throw_invalid_property_type for all callers); it does not cover withFileTypes, rm, or the options argument, and the two overlapping hunks rebase trivially in either order.
Verified:
- test/js/node/fs/fs.test.ts: new fs boolean options report ERR_INVALID_ARG_TYPE like node block (mkdir, readdir and rm through sync, callback and promise entry points, the Received shapes, the force-before-recursive order, the options argument for rm and rmdir, and that the file is still there afterwards) plus the updated mkdirSync > throws for invalid options: 11 tests, all fail on the released build, all pass with this change. The whole file: 521 pass; the one remaining failure is noted below.
- test/js/node/test/parallel/test-fs-mkdir.js now asserts upstream's text; it fails on the released build and passes here. test-fs-rm.js, test-fs-promises.js, test-fs-readdir.js, test-fs-readdir-recursive.js, test-fs-readdir-types.js, test-fs-rmdir-type-check.js pass.
- Other callers of the changed renderers: test/js/web/fetch/wasm-streaming.test.ts, test/js/node/child_process/child_process-node.test.js, test/js/node/net/socketaddress.spec.ts, test/js/web/fetch/fetch-compress.test.ts, test-crypto-random.js, test-socketaddress.js, test-http2-client-request-options-errors.js pass (one unrelated timeout below).
- cargo clippy -p bun_runtime -p bun_jsc is clean.

Background

Node's ERR_INVALID_ARG_TYPE(name, expected, actual) (lib/internal/errors.js) renders The <subject> must be of type <expected>. Received <description of actual>. The subject is "name" argument, unless the name contains a ., in which case it is an option path such as options.recursive and renders as "name" property; a name that already ends in argument is used verbatim. The description of the actual value (type string ('x'), null, an instance of Array) is determineSpecificType.
Bun renders this error in two places: C++ (ErrorCode.cpp, behind $ERR_INVALID_ARG_TYPE in the JS builtins and the C++ bindings) and the Rust helpers on JSGlobalObject used by natively implemented modules such as node:fs. The Rust helpers already call the C++ determineSpecificType for the Received half; only the subject rule was missing.
fs.rm validates its options the way node's validateRmOptions does: the caller's own keys are spread over the defaults, then force, recursive, retryDelay, maxRetries are validated in that order. That is why an own recursive: undefined is an error for rm but not for mkdir, and why force is reported first when both booleans are bad.

Unrelated failures seen while running the suites in this (debug, ASAN) container

fs.test.ts > fs/promises > readdirSync(path, {recursive: true} should work x 100 exceeds its 10 s budget here (12 s). Timing the body: the four recursive readdirSync calls take 0.4 s; sorting the four 6660-entry arrays takes 7.5 s under the debug build. No code on that path is touched by this PR.
socketaddress.spec.ts > does not leak memory exceeds its 5 s budget here (6.8 s); it constructs 101k valid addresses and never reaches an error renderer.

Before / after (right column is also node v26.3.0's output)

call	before	after
`mkdirSync(p, { recursive: "x" })`	`The "recursive" property must be of type boolean, got string`	`The "options.recursive" property must be of type boolean. Received type string ('x')`
`promises.mkdir(p, { recursive: null })`	`The "recursive" property must be of type boolean, got object`	`The "options.recursive" property must be of type boolean. Received null`
`readdirSync(d, { recursive: [] })`	`The "recursive" property must be of type boolean, got array`	`The "options.recursive" property must be of type boolean. Received an instance of Array`
`readdirSync(d, { withFileTypes: 1 })`	`The "withFileTypes" property must be of type boolean, got number`	`The "options.withFileTypes" property must be of type boolean. Received type number (1)` (node coerces this one; Bun keeps rejecting it)
`rmSync(f, { force: "x" })`	`The "options.force" property must be of type boolean.`	`The "options.force" property must be of type boolean. Received type string ('x')`
`rmSync(f, { recursive: undefined })`	`The "options.recursive" property must be of type boolean.`	`The "options.recursive" property must be of type boolean. Received undefined`
`rmSync(f, { recursive: "x", force: "y" })`	`The "options.recursive" property must be of type boolean.`	`The "options.force" property must be of type boolean. Received type string ('y')`
`rmSync(f, "x")`	`The "options" argument must be of type object.`	`The "options" argument must be of type object. Received type string ('x')`
`rmSync(f, { maxRetries: "x" })` (renderer only)	`The "options.maxRetries" argument must be of type number. Received type string ('x')`	`The "options.maxRetries" property must be of type number. Received type string ('x')`

…rties like node fs.mkdir and fs.readdir validated `recursive` (and readdir's `withFileTypes`) through JSValue::get_boolean_strict, whose message is `The "recursive" property must be of type boolean, got string`, and fs.rm hand-formatted `The "options.recursive" property must be of type boolean.` without node's `Received ...` tail. Node validates all of these with validateBoolean(value, 'options.<name>'), so the message is `The "options.recursive" property must be of type boolean. Received type string ('x')`. Route these sites (and rm's non-object `options` error) through JSGlobalObject::throw_invalid_argument_type_value with the dotted name, and teach that renderer (plus the `..._value2` / `..._one_of` variants) node's subject rule: a dotted name is a "property", a plain name an "argument". rm now checks `force` before `recursive`, the order node's validateRmOptions uses. test-fs-mkdir.js goes back to upstream's assertion text.

coderabbitai · 2026-08-16T07:31:14Z

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 26 minutes

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

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 1fcbbe18-cf27-4edd-af19-529bba4de240

📥 Commits

Reviewing files that changed from the base of the PR and between aec33f5 and 2a15b56.

📒 Files selected for processing (4)

src/jsc/JSGlobalObject.rs
src/runtime/node/node_fs.rs
test/js/node/fs/fs.test.ts
test/js/node/test/parallel/test-fs-mkdir.js

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

robobun · 2026-08-16T07:31:59Z

Status: reproduced, fixed, CI green (Buildkite build 99292, all lanes passed); ready for review.

Reproduced on the released build (USE_SYSTEM_BUN=1 bun test test/js/node/fs/fs.test.ts -t "boolean options report|throws for invalid options": 11 fail, all on the message text) and by diffing a 160-case mkdir/readdir/rm/rmdir matrix against node v26.3.0.
With this branch the same 11 tests pass, test-fs-mkdir.js passes with upstream's assertion text, and the matrix matches node on every line where both throw.
The one review note so far (rmdir ordering) is answered below: the case is unreachable through the public rmdir entry points, and the rmdir force check predates this PR.

robobun · 2026-08-16T07:37:09Z

Heads-up on the readdir part of this PR: #39330 changes which values Readdir::from_js accepts, to match node (a null recursive is ignored, withFileTypes is coerced like node's !!options.withFileTypes, and fs.promises.readdir only tests recursive for truthiness). Once both land, the readdir assertions here that withFileTypes: "x" / withFileTypes: 1 / withFileTypes: null throw and that promises.readdir(d, { recursive: "x" }) rejects no longer hold; the options.recursive wording from this PR still applies to the remaining readdirSync/readdir rejection. The mkdir and rm parts do not overlap.

claude

LGTM — error-message wording change with comprehensive coverage. The inline nit about rmdir's force-before-recursive order is real but non-blocking (only observable when both options are bad on rmdir, where force isn't a documented option and Bun validating it at all is already a tracked pre-existing divergence).

Extended reasoning...

Overview

This PR aligns the ERR_INVALID_ARG_TYPE error text for boolean options in fs.mkdir/readdir/rm/rmdir (all sync/callback/promise entry points) with Node v26.3.0's exact wording. Two mechanical pieces: a new InvalidArgTypeName formatter in src/jsc/JSGlobalObject.rs implementing Node's subject rule (dotted name → "x.y" property, trailing argument verbatim, otherwise "x" argument) applied to the three throw_invalid_argument_type_value* renderers; and a small validate_boolean_option helper in node_fs.rs that replaces get_boolean_strict at four call sites and the hand-formatted messages in RmDir::from_js_impl. force is now validated before recursive to match validateRmOptions. Tests: an 11-case block in fs.test.ts covering all three entry-point styles plus the un-special-casing of the vendored test-fs-mkdir.js.

Security risks

None. Pure error-message text change; no new syscalls, no allocation-size arithmetic, no untrusted-input parsing beyond what already existed.

Level of scrutiny

Medium-low. The node_fs.rs hunks are localized and I confirmed val.get(ctx, name) returns None for undefined/missing (JSValue.rs:1136), so acceptance semantics for mkdir/readdir are unchanged vs. the old get_boolean_strict path — only the error text differs. The shared-renderer hunk has broader reach, but the PR enumerated every existing dotted-name caller (options.retryDelay/maxRetries, net.SocketAddress, http2 request options, handle.fd, compress.encoding, socket fault-injection rules), verified none pinned argument in a test, and ran the affected suites. Plain (non-dotted) names render byte-identically to before. The same renderer hunk is already carried by three other open PRs.

Other factors

The one inline finding is a nit: the force-before-recursive reorder also applies to rmdir via the shared parser, so rmdirSync(d, { recursive: 'x', force: 'y' }) now names force where Node names recursive. Impact is negligible — same error code, only the property named in the message differs, and only when both are non-boolean on an API where force isn't documented; the description already scopes "rmdir validates force at all" out as a separately-tracked delta. Not worth blocking on. Test quality is high: hermetic tempDirs, exact { name, code, message } assertions, negative checks that nothing was created/removed, and the vendored Node test now asserts upstream text instead of a Bun-specific branch.

robobun · 2026-08-16T07:55:06Z

On the rmdir ordering note: the reorder is not observable through fs.rmdir, rmdirSync or promises.rmdir. All three reject any defined recursive in JS before the native parser runs (ERR_INVALID_ARG_VALUE, "The property 'options.recursive' is no longer supported", which is also what node v26 does; see rmdirSync/rmdir in src/js/node/fs.ts and fs.promises.ts, covered in fs.test.ts around the rmdir recursive tests), so for rmdir the native parser only ever sees recursive absent. Checked on this branch and on node v26.3.0:

rmdirSync(d, { recursive: "x", force: "y" })   node: ERR_INVALID_ARG_VALUE ... 'options.recursive' is no longer supported. Received 'x'
                                               bun:  same (before and after this PR)
rmdirSync(d, { force: "y" })                   node: ok
                                               bun:  ERR_INVALID_ARG_TYPE on options.force (before and after this PR; only the text changed)

Node's validateRmdirOptions in v26 validates nothing but the options object itself, so the second line (and the same for retryDelay/maxRetries) is the pre-existing rmdir difference the description lists as left alone. Dropping those checks for rmdir is an accept/reject change rather than a wording one, so I have kept it out of this PR; it is a three-line follow-up in from_js_impl if wanted.

robobun · 2026-08-16T08:06:48Z

^{Updated 1:06 AM PT - Aug 16th, 2026}

✅ @robobun, your commit 2a15b56a9a7dbc4d95a172d50bd4436603dce968 passed in Build #99292! 🎉

🧪 To try this PR locally:

bunx bun-pr 39328

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

bun-39328 --bun

github-actions Bot added the claude label Aug 16, 2026

robobun mentioned this pull request Aug 16, 2026

node:fs: accept the readdir recursive/withFileTypes values node accepts #39330

Open

claude Bot reviewed Aug 16, 2026

View reviewed changes

Comment thread src/runtime/node/node_fs.rs

robobun mentioned this pull request Aug 16, 2026

node:fs: validate rm options before the ERR_FS_EISDIR check #39338

Open

Navigation Menu

Uh oh!

node:fs: report mkdir/readdir/rm boolean option type errors like node - #39328

node:fs: report mkdir/readdir/rm boolean option type errors like node#39328
robobun wants to merge 1 commit into
mainfrom
farm/2d7a8080/fs-boolean-option-messages

robobun commented Aug 16, 2026

Uh oh!

coderabbitai Bot commented Aug 16, 2026

Review limit reached

Uh oh!

robobun commented Aug 16, 2026 •

edited

Loading

Uh oh!

robobun commented Aug 16, 2026

Uh oh!

claude Bot left a comment

Uh oh!

Uh oh!

robobun commented Aug 16, 2026

Uh oh!

robobun commented Aug 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

robobun commented Aug 16, 2026

Problem

Fix

Background

Uh oh!

coderabbitai Bot commented Aug 16, 2026

Review limit reached

Uh oh!

robobun commented Aug 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

robobun commented Aug 16, 2026

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Overview

Security risks

Level of scrutiny

Other factors

Uh oh!

Uh oh!

robobun commented Aug 16, 2026

Uh oh!

robobun commented Aug 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

robobun commented Aug 16, 2026 •

edited

Loading