node errors: render a single class name in ERR_INVALID_ARG_TYPE as "an instance of" like node

[robobun]

Problem

• ERR_INVALID_ARG_TYPE messages whose expected type is a single class name say must be of type X; node says must be an instance of X. For example events.on(ee, "x", { signal: 1 }) throws The "options.signal" property must be of type AbortSignal. Received type number (1); node v26.3.0 throws ... must be an instance of AbortSignal. ....
• Cause: the single-string formatter Message::ERR_INVALID_ARG_TYPE (src/jsc/bindings/ErrorCode.cpp:644 on main) prints of type + the string for anything that is not a phrase with spaces. Node's lib/internal/errors.js turns a string expected into a one-entry list and classifies the entry (kTypes name, class name, other) the same way it classifies list entries; the list overload a few lines below already ports that classification, the single-string one never did.
• Every single-string caller is affected: the 21 C++ ERR::INVALID_ARG_TYPE(..., "ClassName"_s, ...) sites (validateAbortSignal, Buffer.copyBytesFrom, X509Certificate#checkPrivateKey/#verify, Histogram#add, the Bun.readableStreamTo* consumers, ...) and the 26 builtin $ERR_INVALID_ARG_TYPE(name, "ClassName", value) sites, which compile to the same formatter (ErrorCode.cpp:1673): stream.addAbortSignal, Readable/Writable/Duplex.fromWeb, events.addAbortListener, events.once, perf_hooks.timerify, new Readable({ signal }), and so on. The two builtin sites that pass "Object" (inspector.Session#post and the internal zlib transform params check) render of type Object where node lower-cases to of type object, same cause.
• Two of the C++ sites also use the wrong names: Histogram#add reports The "argument" argument ... Histogram (node: "other" / RecordableHistogram), and X509Certificate#checkIssued throws a hand-written issuer must be a JSX509Certificate (node: The "otherCert" argument must be an instance of X509Certificate. Received ...).

Fix

• Message::ERR_INVALID_ARG_TYPE (single-string overload) now classifies its argument like one node list entry, reusing the list overload's isPrimitiveTypeName/isClassName: a kTypes name renders of type + lower-cased name, a class name renders an instance of X. The existing phrase handling and the verbatim of type ... fallback for pre-rendered lists ("string or an instance of Buffer, ...") are unchanged, so callers passing a lower-case primitive name or a pre-rendered list produce byte-identical output.
• Histogram#add passes "other" / "RecordableHistogram"; X509Certificate#checkIssued goes through ERR::INVALID_ARG_TYPE with "otherCert" / "X509Certificate". Nothing else at the call sites changes; the other 19 C++ sites and all builtin sites are fixed by the formatter. (Buffer.copyBytesFrom is deliberately left alone so this does not collide with node:buffer: reject DataView in Buffer.copyBytesFrom #35239, which switches that line to INVALID_ARG_INSTANCE for a different reason; either order of landing gives the same text.) Pre-rendered list sites are a separate change (node: pass ERR_INVALID_ARG_TYPE expected types as lists in the C++ bindings #38690, node: pass ERR_INVALID_ARG_TYPE expected types as lists so fs streams, zlib and diagnostics_channel messages match node #38657).
• Why this is correct: for node:* APIs node's output is the contract, and node's classification is exactly the one-entry case of the list port that already exists in this file. Every assertion added below is the string node v26.3.0 prints for the same call; the three Bun-only sites whose text changes as a side effect (Bun.readableStreamTo*, CompressionStream/TextDecoderStream chunk errors, the $checkBufferRead guard) go from of type ReadableStream to an instance of ReadableStream, which is the wording the rest of the tree already uses for class checks via INVALID_ARG_INSTANCE. No test in the tree pinned any of the old strings.
• Verified:
  • test/js/node/errors/invalid-arg-type-expected.test.ts (new): 17 cases covering the three routes into the formatter (C++ name, JS-supplied dotted name via validateAbortSignal, builtin $ERR_INVALID_ARG_TYPE including an async rejection), the "Object" lower-casing, and three unchanged renderings (primitive, pre-rendered list, phrase). 14 fail on the unfixed build, 17 pass with the fix.
  • test/js/bun/perf_hooks/histogram.test.ts: add() name and class name (fails before, passes after).
  • test/js/node/crypto/x509-subclass.test.ts: checkIssued, checkPrivateKey and verify messages for five non-KeyObject values, plus wrong-key-type and success paths (6 fail before, 19 pass after).
  • test/js/web/streams/streams.test.js: the seven Bun.readableStreamTo* consumers (7 fail before, pass after).
  • A 20-call probe script diffed against real node v26.3.0 is identical with the fix; buffer.test.js, event-emitter.test.ts, node-stream.test.js, perf_hooks.test.ts, inspector.test.ts, compression.test.ts, text-decoder-stream.test.ts, x509.test.ts, all of streams.test.js, and 17 vendored node tests for the affected APIs (test-stream-add-abort-signal, test-events-once, test-events-on-async-iterator, test-crypto-x509, test-buffer-from, the five test-whatwg-webstreams-adapters-*, ...) pass on the debug build.

Background

• ERR_INVALID_ARG_TYPE message shape: The "<name>" argument|property must be <expected>. Received <value>. Node builds <expected> from a list: entries in its kTypes list (string, number, object, Function, ...) become of type x (lower-cased), entries matching /^[A-Z][a-zA-Z0-9]*$/ become an instance of X, anything else is printed as written. A string expected is wrapped into a one-entry list first, so 'AbortSignal' and ['AbortSignal'] render identically.
• Bun has two formatter overloads in ErrorCode.cpp: the list overload (used when a caller passes an array or a std::span) is a port of the grouping above; the single-string overload is reached by C++ callers passing one WTF::String and by $ERR_INVALID_ARG_TYPE when its second argument is a string. Some C++ callers pass the single-string overload a list they already rendered by hand ("string or an instance of Buffer"); node has no equivalent, which is why that overload keeps a verbatim fallback instead of delegating to the list renderer outright.
• INVALID_ARG_INSTANCE is the existing helper that always prints an instance of <text>; it stays, since it also takes pre-rendered instance lists. After this change passing a bare class name to INVALID_ARG_TYPE produces the same text, so callers no longer have to know about it.

bun 1.4.0 vs node v26.3.0 for the calls asserted in the tests

Buffer.copyBytesFrom(1)
  bun:  The "view" argument must be of type TypedArray. Received type number (1)
  node: The "view" argument must be an instance of TypedArray. Received type number (1)
events.on(ee, "x", { signal: {} })
  bun:  The "options.signal" property must be of type AbortSignal. Received an instance of Object
  node: The "options.signal" property must be an instance of AbortSignal. Received an instance of Object
stream.addAbortSignal(1, readable)
  bun:  The "signal" argument must be of type AbortSignal. Received type number (1)
  node: The "signal" argument must be an instance of AbortSignal. Received type number (1)
Readable.fromWeb(1)
  bun:  The "readableStream" argument must be of type ReadableStream. Received type number (1)
  node: The "readableStream" argument must be an instance of ReadableStream. Received type number (1)
Duplex.fromWeb({ readable: 1, writable: 1 })
  bun:  The "pair.readable" property must be of type ReadableStream. Received type number (1)
  node: The "pair.readable" property must be an instance of ReadableStream. Received type number (1)
perf_hooks.timerify(fn, { histogram: 1 })
  bun:  The "options.histogram" property must be of type RecordableHistogram. Received type number (1)
  node: The "options.histogram" property must be an instance of RecordableHistogram. Received type number (1)
createHistogram().add(1)
  bun:  The "argument" argument must be of type Histogram. Received type number (1)
  node: The "other" argument must be an instance of RecordableHistogram. Received type number (1)
events.once(1, "x")  (rejects)
  bun:  The "emitter" argument must be of type EventEmitter. Received type number (1)
  node: The "emitter" argument must be an instance of EventEmitter. Received type number (1)
new inspector.Session().post("Runtime.evaluate", 1)
  bun:  The "params" argument must be of type Object. Received type number (1)
  node: The "params" argument must be of type object. Received type number (1)
cert.checkPrivateKey(1) / cert.verify(1)
  bun:  The "pkey" argument must be of type KeyObject. Received type number (1)
  node: The "pkey" argument must be an instance of KeyObject. Received type number (1)
cert.checkIssued({})
  bun:  issuer must be a JSX509Certificate
  node: The "otherCert" argument must be an instance of X509Certificate. Received an instance of Object
unchanged (already identical): process.chdir(1), createHash("sha256").update(1), new vm.SyntheticModule(1, fn)

With this change every bun line above is identical to the node line.

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 6 minutes

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: 89f645e6-bd82-41ad-9d2b-1ac733c17340

📥 Commits

Reviewing files that changed from the base of the PR and between 9805144 and 17f01b7.

📒 Files selected for processing (8)

• src/jsc/bindings/ErrorCode.cpp
• src/jsc/bindings/ErrorCode.h
• src/jsc/bindings/JSNodePerformanceHooksHistogramPrototype.cpp
• src/jsc/bindings/JSX509CertificatePrototype.cpp
• test/js/bun/perf_hooks/histogram.test.ts
• test/js/node/crypto/x509-subclass.test.ts
• test/js/node/errors/invalid-arg-type-expected.test.ts
• test/js/web/streams/streams.test.js

---

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

[robobun]

Status: fix pushed, self-review in progress.

Reproduced on bun 1.4.0 / main with, for example:

$ bun -e 'require("events").on(new (require("events"))(), "x", { signal: 1 })'
TypeError: The "options.signal" property must be of type AbortSignal. Received type number (1)
$ node -e '...same...'
TypeError: The "options.signal" property must be an instance of AbortSignal. Received type number (1)

Same divergence for every caller that passes one class name (Buffer.copyBytesFrom, stream.addAbortSignal, Readable.fromWeb, timerify, Histogram#add, X509Certificate#checkPrivateKey/#verify, ...), all routed through the single-string formatter in ErrorCode.cpp. The PR description has the full list and the node v26.3.0 output for each asserted call.

Tests: test/js/node/errors/invalid-arg-type-expected.test.ts (new), plus additions to histogram.test.ts, x509-subclass.test.ts and streams.test.js; all fail on the unfixed build and pass with the fix.

[claude]

LGTM — focused Node.js error-message compat fix.

What was reviewed:

• The single-string ERR_INVALID_ARG_TYPE formatter now classifies kTypes vs class name vs phrase using the same isPrimitiveTypeName/isClassName helpers the list overload already uses; checked ordering so "Object"/"Function" hit kTypes before the class-name regex and lower-case correctly.
• Verified the pre-rendered-list fallback ("string or an instance of Buffer, ...") and the free-form phrase branch are byte-identical to before, and that the WTF::String& → StringView param widening on the two static helpers is safe for the existing list-overload callers.
• Two call-site name fixes (Histogram#add → "other"/RecordableHistogram, X509Certificate#checkIssued → structured "otherCert"/X509Certificate) match node v26.3.0; the 17-case test file plus per-API tests pin exact strings.

Extended reasoning...

Overview

This PR fixes the single-string overload of Message::ERR_INVALID_ARG_TYPE in src/jsc/bindings/ErrorCode.cpp so that a bare class name renders as "an instance of X" (matching Node's lib/internal/errors.js) instead of "of type X". It reuses the existing isPrimitiveTypeName/isClassName helpers from the list overload, forward-declares them, and widens their parameter from const WTF::String& to StringView so both overloads can share them. Two call sites with wrong argument/class names are corrected: Histogram#add ("argument"/"Histogram" → "other"/"RecordableHistogram") and X509Certificate#checkIssued (hand-written message → structured ERR::INVALID_ARG_TYPE with "otherCert"/"X509Certificate"). A header comment documents the classification. Four test files add exact-string assertions verified against node v26.3.0.

Security risks

None. This is pure error-message text formatting on the throw path; no parsing, allocation sizing, auth, or crypto logic changes. The X509 change only replaces one throw call with another that formats the same rejection differently.

Level of scrutiny

Medium-low. The formatter is shared across ~50 call sites, so the important thing is that the three unchanged rendering classes (lower-case primitive, pre-rendered list, free-form phrase) stay byte-identical. I traced each: primitives hit isPrimitiveTypeName and lower-case to themselves; pre-rendered lists contain " or " so isPhrase is false and they get the verbatim "of type " prefix; phrases fall through unchanged. The kTypes-before-classRegExp ordering matches Node so "Object"/"Function" correctly render "of type object"/"of type function" rather than "an instance of Object". The StringView widening is safe — the list overload passes a WTF::String, which implicitly converts.

Other factors

No CODEOWNERS on touched files. No prior human review comments. Test coverage is thorough: a new 17-case file exercises all three entry routes (C++ literal name, JS-supplied dotted name via validateAbortSignal, builtin $ERR_INVALID_ARG_TYPE) plus an async rejection, the "Object" lower-casing, and the three unchanged-rendering controls; per-API tests cover the two renamed call sites and the seven Bun.readableStreamTo* consumers. The PR description records that 14/17 fail on the unfixed build and all pass after, and that a 20-call diff against real node v26.3.0 is identical.