node: Node-accurate ERR_INVALID_ARG_TYPE / ERR_INVALID_THIS / ERR_MISSING_ARGS; resync test/common and 128 vendored tests with v26.3.0 - #35429
Draft
cirospaciari wants to merge 179 commits into
Draft
Conversation
…ughtException
These callbacks are dispatched from a promise reaction, so a throw inside one
rejected the derived promise and surfaced as an unhandledRejection instead of an
uncaughtException as it does in Node, where the callback runs off the libuv
request completion. process.on('uncaughtException') and node:domain never saw
errors thrown from fs, dns, or crypto.pbkdf2 callbacks.
Add guardCallback in internal/shared and apply it at the existing callback
chokepoints (ensureCallback in fs, validateResolve in dns) plus the few sites
that bypass them. The callback keeps its exact place in the event loop; only the
throw is rerouted, through the same native path timer callbacks already use,
which is why setTimeout was never affected.
opendir and glob already hop through process.nextTick for this and are left
alone, since routing them would change their timing.
Found by sweeping every upstream test absent from both main and all 47 open pull requests, so none of these overlap work in flight. No source change is needed for any of them. sequential/test-cpu-prof-default sequential/test-cpu-prof-dir-absolute sequential/test-cpu-prof-dir-and-name sequential/test-cpu-prof-dir-relative sequential/test-cpu-prof-drained parallel/test-disable-sigusr1 parallel/test-whatwg-webstreams-adapters-to-writablestream Each was run 6 times, checked for a skip marker, and tamper-checked: mutating a mustCall count or an assertion makes every one of them exit non-zero, so none is passing vacuously. No-Verification-Needed: test-only diff, no runtime surface to drive
test-pipe-stream
test-pipe-unref
test-queue-microtask-uncaught-asynchooks
All three bind unix or UDP sockets, so an earlier sweep that ran them under a
restricted sandbox recorded them as flaky and they were wrongly discarded. With
the sandbox off they are 6/6, verbatim. CI is unaffected by that restriction.
The two pipe tests are tamper-checked: mutating an assertion exits 1. The
queue-microtask one is not, because its only assertion runs inside a
process.on('exit') handler and a failure there does not change the exit code --
but that is equally true on the node v26.3.0 binary, so there is no
bun-versus-node differential. It still exercises async_hooks, queueMicrotask and
uncaughtException together and would catch a crash or a hang.
Upstream tests run with --expose-internals import node's internal modules
directly. Bun already has the mechanism (exposedInternals in
internal-for-testing.ts); this widens it to more of Bun's REAL modules under the
names node's tests use:
internal/validators -> src/js/internal/validators.ts
internal/util/inspect -> src/js/internal/util/inspect.js
internal/util -> { normalizeEncoding } from node_util_binding
internal/errors -> bun's internal/errors.ts (aggregateTwoErrors only)
internal/event_target -> native Event/CustomEvent/EventTarget + kWeakHandler
internal/freelist, internal/fixed_queue, internal/assert/myers_diff
internal/net is recovered in the harness by probing net._normalizeArgs([]) for
the module-private normalizedArgsSymbol rather than minting a look-alike.
hasTemporal and hasLocalStorage are feature-detected in common/index.js; both
are false here.
Nothing above is a test-only construct: each name maps to the same code users
hit. Modules with no Bun counterpart were deliberately not shimmed -- the rest
of internal/errors is native C++ factory functions rather than a JS codes table,
so E/SystemError/hideStackFrames would be a second error hierarchy and the tests
would measure the shim rather than bun.
Adds test-global-customevent, test-net-normalize-args and
test-whatwg-encoding-singlebyte (57 subtests) with its fixtures, verbatim.
Found by re-sweeping the 754 upstream tests still absent from main and every open pull request, against a build carrying this branch's callback-dispatch and internal-module changes. Passes as-is, no source change. 6/6 with no skip marker, byte-identical to upstream, and tamper-checked: a mutated assertion exits 1. No-Verification-Needed: test-only diff, no runtime surface to drive
A `git add -A test/js/node/test` on an earlier commit swept in files that were never meant to land here: a scratch file, a test previously rejected as vacuous (test-gc-net-timeout), one that belongs to another branch and fails `handle instanceof UDP` on Windows (test-dgram-create-socket-handle), one that times out on every Windows shard (test-pipe-unref), and seven files that duplicate the free-wins branch (test-cpu-prof-*, test-disable-sigusr1, test-whatwg-webstreams-adapters-to-writablestream). All removed; each lives on in the branch that owns it, or nowhere, as appropriate. Also fixes CI-visible test-order dependence in the callback-throw suite: every subtest shared one tempdir, so the symlink subtests' l3/l4/lnc entries could become the first entry the Dir subtest's dir.read() returns. The Dir subtest now uses its own directory.
Node exposes formatTime from internal/util/debuglog and its test imports it from there under --expose-internals. Bun's copy was a closure inside the ConsoleObject builtin, unreachable by name. It now lives in internal/util/debuglog, required by createConsoleConstructor the same way that function already requires internal/validators, and is exposed through exposedInternals. Output is unchanged: the algorithm is line-for-line the same and the module is loaded lazily by the console.Console getter, so global console.timeEnd, which is native, is untouched. Plain prototype methods replace the captured-$call style because that is the internal-module convention, per neighboring modules. Adds test-console-formatTime from Node v26.3.0, verbatim.
Deprecations, matching node's exact text and once-per-process semantics:
_tls_common and _tls_wrap emit DEP0192 (the latter previously aliased straight
to node:tls with nowhere to warn, so it is now node's re-export shim, and plain
require("tls")/require("https") stay silent). --pending-deprecation and
NODE_PENDING_DEPRECATION gate DEP0111 on process.binding() and DEP0119 on
binding("uv").errname. Bun's own builtins call process.binding() where node's
internals use internalBinding, so internal callers neither warn nor consume the
once-latch; a later user call still warns exactly once.
cpu-prof: --cpu-prof-name/-dir/-interval without --cpu-prof exit 9 with node's
message instead of warning and continuing, with an interval equal to the
default treated as a noop as node does. ${pid} is substituted in
--cpu-prof-name. The default filename is node's
CPU.<yyyymmdd>.<hhmmss>.<pid>.<tid>.<seq>.cpuprofile in local time; the old name
used epoch microseconds, and the first cut of this exposed that
bun_core::Timespec is not wall-clock (it produced 1970), so the timestamp comes
from SystemTime with localtime_r, or GetLocalTime on Windows. A self-directed
fatal signal via process.kill with no JS listener flushes the profile before
dying, as node's Kill binding does; the process still dies by the real signal.
Not converted, deliberately: DEP0169 on url.parse was removed on purpose in
PR #16641, so re-adding it would reverse a product decision; and SIGPROF is not
reserved by JSC, so node's "reserved while debugging" warning would claim a
behavior bun does not have.
Adds six upstream Node v26.3.0 tests, verbatim.
…G_SNAPSHOT getHeapStatistics gains total_allocated_bytes so the key set matches node's exactly. getHeapSpaceStatistics and getHeapCodeStatistics report JSC's real totals in node's shape instead of throwing: JSC has one undivided heap, so its used and capacity numbers appear under old_space and the other twelve V8 space names carry zeros; the names are required by shape-dependent consumers, the numbers are real. getCppHeapStatistics validates its argument with node's message and returns the empty statistics node itself reports when cppgc holds nothing. setFlagsFromString validates and records the flags instead of throwing, and cachedDataVersionTag derives a stable uint32 from the runtime version plus the recorded flags, so the tag changes across a setFlagsFromString call as node's does. startupSnapshot callbacks throw node's ERR_NOT_BUILDING_SNAPSHOT. writeHeapSnapshot path handling is deliberately untouched: PR #34084 already fixes it. queryObjects is not implemented; it needs real heap iteration by prototype chain, which JSC does not expose. Adds test-v8-stats, test-v8-version-tag, test-v8-startup-snapshot-api and test-cppheap-stats from Node v26.3.0, verbatim.
--heap-prof-name/-dir/-interval without --heap-prof now exit 9 with node's
"<argv0>: <flag> must be used with --heap-prof" instead of warning and
continuing; --heap-prof-interval was previously not registered at all, so
`bun --heap-prof-interval 128 x.js` parsed 128 as the script. An interval
equal to node's default (512 KiB) is a noop without --heap-prof, like node.
--heap-prof now writes node's V8 sampling-heap-profile JSON
({"head": {...}, "samples": [...]}) under node's filename format
Heap.<yyyymmdd>.<hhmmss>.<pid>.<tid>.<seq>.heapprofile in local time (the old
name also hit the same non-wall-clock Timespec bug the CPU side fixed).
JavaScriptCore has no allocation-site sampler, so the profile reports the
real live-heap size on the (root) frame with no per-function attribution;
the full .heapsnapshot remains available via v8.writeHeapSnapshot() and
Bun.generateHeapSnapshot("v8"). The markdown format (--heap-prof-md) is
unchanged. Heap profiles also flush on a self-directed fatal signal via
process.kill, sharing the CPU profiler's hook.
Writing to an absolute --heap-prof-dir or --heap-prof-name (and an absolute
--cpu-prof-name) previously hit a debug assertion in AutoAbsPath::append;
both writers now use resolve-style join.
Adds two upstream Node v26.3.0 tests, verbatim. The other nine
test-heap-prof-* files assert a runAllocation call frame inside the profile
tree (or per-worker profiles), which requires allocation-stack sampling JSC
does not provide, so they are not vendored.
The vendored harness now parses node's `// Env: A=1 B=2` comment convention: parseTestMetadata mirrors upstream, and the test re-spawns itself with the merged environment when any variable differs, as node's common does. Two gaps that respawning exposed are fixed with it: the --expose-internals interceptor now installs when the flag arrives via execArgv in a re-spawned child, and the internal/options shim resolves --test-isolation from execArgv. node:url exports URLPattern, which node v26 re-exports from the global. pathToFileURL is ported from node's own JS implementation: the windows option, UNC and extended-UNC edges, node's pre-encode character set, and posix backslashes kept as %5C, which bun dropped. fileURLToPath attaches err.input, the parsed URL object, on ERR_INVALID_FILE_URL_PATH. The Bun.pathToFileURL and Bun.fileURLToPath globals are untouched. The exposed internal/util/inspect.getStringWidth now implements node's per-code-point east-asian-width-first algorithm, so a family emoji measures 8 and a skin-tone emoji 4 as in node. Bun.stringWidth and bun's own console layout are deliberately unchanged; bun measures grapheme clusters and that is a product decision, not a test's to overturn. Adds seven upstream Node v26.3.0 tests, verbatim: the three Env-comment tests, test-urlpattern, test-icu-stringwidth, and the two file-URL conversion tests.
A getcwd failure at startup aborted every entry point with a generic
ENOENT. Node falls back to the executable's directory
(Environment::GetCwd in env.cc) and lets process.cwd() surface the
error later.
- add bun_core::getcwd_or_exe_dir and use it for the synthetic entry
paths (-e/-p, stdin, cron, node-emulation eval and relative scripts,
feedback) and for the parsed startup cwd of runtime commands only;
install/test/build keep the hard error so they never act on a tree
found above the executable
- an absolute --cwd no longer needs a live cwd; a relative one still
errors; the stored cwd is the post-chdir physical path so
process.cwd(), path.resolve, and the resolver agree
- route a bare --interactive to the REPL like node's --interactive
Enables test-cwd-enoent{,-preload,-repl}.js (verbatim from Node
v26.3.0).
process.cwd() returned the resolver's cached top_level_dir, so it kept
returning a stale path after the directory was rmdir'ed. Node re-runs
uv_cwd on every cache miss and clears its cache on chdir
(lib/internal/bootstrap/switches/does_own_process_state.js).
- Bun__Process__getCwd now does a real getcwd and on failure throws
Node's uv_cwd UVException (message, code, errno, syscall verified
byte-identical against node v26.3.0)
- Process_functionChdir clears the cached cwd instead of repopulating
it, so the next process.cwd() re-queries the OS
- WriteStream's internal $fastPath no longer path.resolve()s its
discarded path, so child_process spawn keeps working from a deleted
cwd (its stdin wiring constructs WriteStream(""))
- drop the now-unused node::path get_cwd alias
The hot path is unchanged: the cached JSString still serves repeated
process.cwd() calls; only the first call after startup or chdir pays
the syscall.
Enables test-cwd-enoent-improved-message.js (verbatim from Node
v26.3.0).
A failed require or import threw ResolveMessage with bun's own message and no code, so the ubiquitous userland pattern `err.code === 'MODULE_NOT_FOUND'` never matched. ResolveMessage now extends Error via a new prototypeBase class option in codegen -- the direction NodeUtilTypesModule.cpp's FIXME already sanctions -- and its message getter returns node's exact text for runtime import kinds: MODULE_NOT_FOUND with a Require stack and a requireStack array for CJS, ERR_MODULE_NOT_FOUND with the specifier truncated to the package name for bare ESM imports. The class itself is kept: Bun.build's log API documents name and constructor as ResolveMessage, and the getter is prefix-gated so invalid-URL, data-URL and ENAMETOOLONG texts are untouched, as is the CLI stderr display text. require.resolve now validates options.paths with node's exact ERR_INVALID_ARG_TYPE and ERR_INVALID_ARG_VALUE messages. Nine assertions across three bun-owned test files are updated to the node shapes; each is listed in the pull request for sign-off. Adds test-require-resolve-invalid-paths from Node v26.3.0, verbatim.
…o stack/callback-throw-dispatch
…stack/callback-throw-dispatch # Conflicts: # src/js/node/dns.ts
…nto stack/callback-throw-dispatch
…ack/callback-throw-dispatch
…ns' into stack/callback-throw-dispatch
- CommonJS.ts: read options.paths once into a local instead of three property reads (oxlint single-read rule; also tamper-resistant against getters) - BunCPUProfiler.rs: replace open-coded mem::zeroed with the audited bun_core::ffi::zeroed wrapper and use raw-pointer borrows for the localtime_r out-params - bun_core: add Zeroable impl for libc::tm
Implements node v26's Assert class: the namespace is built from Assert.prototype, options live behind a private symbol so destructured methods fall back to defaults, calling without new throws ERR_CONSTRUCT_CALL_REQUIRED, and the diff option is threaded through assertion_error.ts, where full disables truncation and partialDeepStrictEqual appends diffs to custom messages. Deep equality gains a third checkPrototypes template flag enabled ONLY by the new node entry point: node:assert's strict variants and util.isDeepStrictEqual now compare prototypes (Buffer vs Uint8Array, subclass vs base are unequal, as in node) while Bun.deepEquals and every expect() instantiation are verified byte-identical in behavior before and after. Also fixed on the node-gated path: own-enumerable-only property walks, boxed Symbol/BigInt values, boxed-string own properties, RegExp lastIndex, invalid dates comparing equal, own properties of Dates/Maps/Sets/arrays/typed arrays, and WeakMap/WeakSet/Promise never equal. All twelve documented strictBug entries in bun's own deep-equal tests now pass and their markers are removed. Standalone fix worth its own line: the Rust inspect newline splitter emitted a trailing empty chunk for strings ending in a newline, breaking assertion diff line counts everywhere. One bun-owned behavior change, flagged for sign-off: assert !== assert.ok now, matching node 25+, as a consequence of the prototype layout; assert.spec.ts is updated accordingly. Adds test-assert-class, test-assert-class-destructuring and test-util-isDeepStrictEqual from Node v26.3.0, verbatim.
…nto stack/callback-throw-dispatch
…mordial-safe Node's deprecate() closures live in each Environment's own JS, so every worker warns once itself. The function-local statics latched process-wide and raced across JS threads; move them onto the Process object. Also restore the captured prototype methods formatTime used before it moved out of ConsoleObject.
Node's ESM loader classifies specifiers platform-independently, but the module-not-found message shaping used the host-native package-path check, whose Windows arm accepts any byte as a drive letter. A specifier like ':://x' was therefore reported as "Cannot find module" on Windows and "Cannot find package '::'" on POSIX (Node prints the latter everywhere). Classify with a host-agnostic predicate that only treats relative forms, separator-led paths, and ASCII-letter drive forms as path-like.
…ual; keep the watch-kill-signal flag set through reload
bindings.cpp DataViewType: the tail `if constexpr (checkPrototypes) break;
return true;` was dead (the leading !checkPrototypes guard already broke
out of the non-node instantiations). Replaced with a direct
nonIndexOwnPropertiesEqual call, which also matches node v26.3.0: its
DataView compare uses getOwnNonIndexProperties, so an integer-index own
property is ignored (`{0:1}` is accepted) while a string-named one is
rejected. deep-equal.test.ts covers both cases.
PosixSignalHandle: emit_watch_kill_signal_before_reload no longer clears
IS_EMITTING_WATCH_KILL_SIGNAL on return. The caller proceeds straight
through persist_now() into reload_process() without yielding, and the
grace-timer thread reads this flag to extend its deadline; clearing it
left persist_now() invisible to the grace timer, so a slow first-reload
persist under NODE_COMPILE_CACHE could be forced mid-write. execve tears
down the flag, so nothing needs to reset it.
on_before_reload_process_linux sets CLOEXEC on every fd >= 3, but
NODE_CHANNEL_FD survives in environ; the reloaded image re-attaches IPC to
a fd that no longer exists, so the parent stops receiving 'message' events
after the first reload and the vendored test-watch-mode-kill-signal-*
cleanup (which waits for child.on('exit') after child.kill()) can hang when
the reloaded iteration's IPC setup is broken. Unset CLOEXEC on the
NODE_CHANNEL_FD after the close_range so the socket survives like node's
grandchild/watcher relay does.
New watch.test.ts case verifies a process.send() from both sides of a
--watch reload reaches the parent.
…ctive, dedupe watch.test.ts waitFor c-bindings: the SIG_DFL reset loop now queries the old disposition and skips SIG_IGN/SIG_DFL, so an inherited SIG_IGN (nohup SIGHUP, job-control SIGTTIN/SIGTTOU) survives the reload like it did before execve. The loop only needs to reset caught handlers (the queued-and-lost case); SIG_IGN never enters Bun__onPosixSignal. Arguments: main's #31827 added a visible --interactive entry at :182, so this PR's hidden one at :319 was dead after the merge. Removed. watch.test.ts: the three byte-identical reader/decoder/waitFor blocks are now a single stdoutWaiter() helper; stderr goes to inherit in the four tests that were piping-and-ignoring it.
…oint in the port stays invalid applyIDNADeltaToURLAuthority previously passed host[:port] to the delta. The mapping-class entries identity-preserve a port (non-digit in, non-digit out), but the ignored-class entries (U+180E, U+206A..U+206F) are stripped, turning `http://foo:8\u180E0/` into `http://foo:80/` where node and pre-PR bun reject it. Find the last ':' in the non-bracketed host span and leave the port verbatim so the WHATWG port state still sees the invalid code point. url.test.ts covers both a rejected port and an accepted host with :80.
…set atomically The pre-execve disposition reset is process-wide but other threads are not quiesced; JSC's SamplingProfiler and concurrent GC pthread_kill sigThreadSuspendResume (an RT signal) and the wasm trap handler catches SIGSEGV/SIGBUS. Resetting those to SIG_DFL while heap-allocating argv/envp turns a sampler tick or stop-the-world into a fatal signal under --watch --cpu-prof. Skip SIGSEGV/SIGBUS and s >= SIGRTMIN so execve resets them atomically (BunProcess.cpp already guards sigThreadSuspendResume for the same reason). DOMURL: note at the is8Bit early return that a percent-encoded delta source is intentionally excluded from this stopgap.
…e header matches node kMethodsWithCustomMessageDiff routed partialDeepStrictEqual failures through createErrDiff, but kReadableOperator had no matching entry, so the header rendered as the literal string 'undefined'. Added the entry with node v26.3.0's exact wording; assert.test.cjs asserts the header.
… fix stale isSpecialValue comment
…d url.host setter delta to the host span bindings.cpp: the array path's pre-existing symbol-only loop (prototype- walking get/getIfPropertyExists) ran before nonIndexOwnPropertiesEqual, which already enumerates own enumerable strings and symbols with own-slot lookups. Under checkPrototypes that meant each symbol getter fired twice per side where node fires once. Moved the nonIndexOwnPropertiesEqual tail-call above the symbol block so the node path bypasses the redundant pass; Bun.deepEquals (non-checkPrototypes) keeps the existing behavior. deep-equal.test.ts asserts the getter-call count. URLDecomposition.cpp setHost: same port-span bounding as c0e2643 applied to applyIDNADeltaToURLAuthority. The delta now runs on value.left(reverseFind (':')) only, so an ignored-class code point in the port is not stripped into a valid digit run. url.test.ts covers both a delta-in-port (port stops at the non-digit) and a delta-in-host (host mapped, port kept).
…LOG_ENABLED collect_persist_jobs and write_persist_job_locked built display_name() and format!() Strings per entry even with logging off; those are only read by cclog!() which already gates on LOG_ENABLED. Gated the bindings like read_cache_file already does and inlined the Phase-2 failure log args.
The NODE_CHANNEL_FD unset_cloexec and caught-signal SIG_DFL reset were inside on_before_reload_process_linux under an OS(LINUX) || OS(FREEBSD) gate, so macOS skipped the hook entirely: usockets CLOEXEC'd the IPC fd and execve closed it, and the reloaded image attached to a dead fd (the "IPC to the parent survives a --watch reload" test failed on darwin). Renamed to on_before_reload_process_posix under !OS(WINDOWS); the close_range sweep stays Linux/FreeBSD-only, and reload_process calls the hook on all unix.
formatWhatwgURL rebuilt the href from .search/.hash, which both return ""
for a null and an empty-string component, so url.format(new URL("http://x/?"),
{unicode: true}) dropped the marker where node keeps it. Derive presence
from the href (the only bare # is the fragment delimiter; the char before
it or end-of-href is ? iff the query is present-and-empty).
CachedBytecode::__bun_jsc_generate_cached_bytecode and RuntimeTranspilerCache::OutputCode::byte_slice are called from NodeCompileCache.rs / jsc_hooks.rs (different crate than the pub(crate) sweep narrowed them to); node_process get_cwd uses bun_sys::getcwd directly (Syscall alias removed); repl editor_mode bool renamed to input_mode enum.
…g does not trip hasInlineStorage
{...process.binding("uv")} (internal/test/binding.ts) asserts
hasInlineStorage() in JSObject::inlineStorage on a constructEmptyObject(...,
0) object under the merged WebKit. Use the default-capacity overload; the
binding carries ~85 properties anyway so zero inline was a non-optimization.
…ered Error.* in isInsideNodeModules isInsideNodeModules read Error.captureStackTrace/stackTraceLimit/ prepareStackTrace at call time inside a try/finally with no catch, so a deleted captureStackTrace or a non-writable stackTraceLimit made url.parse throw. Capture captureStackTrace at module scope (matching assertion_error.ts /inspect.js/quic.ts) and swallow any throw from the body and the restore so callers never observe it.
The hoisted deliverCallbackResult/Error + .bind(cb) pattern allocated one bound-function object per .bind call (same count as the arrows it replaced), so the 'no closure allocates per invocation' comment was wrong; and .bind resolves through user-mutable Function.prototype.bind where arrows do not.
…-uncaught # Conflicts: # src/runtime/dispatch.rs # src/runtime/node/node_fs.rs
…SIGPWR on Linux) in the pre-execve disposition reset ipc.rs: the 276e176 merge moved the cpp bridge out of crate:: scope; the two advanced-buffer wrappers now resolve through bun_jsc::cpp like their ipc_serialize/ipc_parse siblings. c-bindings.cpp: on Linux g_wtfConfig.sigThreadSuspendResume is SIGPWR (30), not an RT signal, so the s >= SIGRTMIN break did not skip it; resetting SIGPWR to SIG_DFL while the SamplingProfiler/concurrent GC are still pthread_kill'ing the JS thread with it terminates the process instead of reloading. Skip it explicitly under OS(LINUX), mirroring BunProcess.cpp's process.on guard.
…into claude/node-v26-gaps-wave4 # Conflicts: # src/jsc/bindings/ErrorCode.cpp # test/js/node/test/common/index.mjs
… the try so a throwing getter is swallowed too
…soned CallSite.prototype.getFileName degrades to false instead of escaping to url.parse
…into claude/node-v26-gaps-wave4
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #34434's branch
claude/callback-throw-uncaught— review that first.What this does
Two related fixes to how Bun reports
ERR_INVALID_ARG_TYPE, plus a resync of the vendored Node test harness.1. Rust error paths now use the same message renderer as the C++ ones
JSGlobalObject::throw_invalid_argument_type_value(111 call sites) hand-rolled its message instead of calling the C++ formatter that already ports Node'sERR_INVALID_ARG_TYPE. Two consequences:"name" argument. Node writes"name" propertywhen the name contains a dot.new net.SocketAddress({ address: 5 })said argument where Node says property.must be of type X. Node writesmust be an instance of XwhenXis a class name, and groups a list of accepted types intoof type a or an instance of B or C.The fix routes the argument name through the existing
Bun::addParameter, and addsthrow_invalid_argument_type_list, which hands a list of accepted types to the existing C++ list renderer (formatInvalidArgType, extracted from theArgListoverload so both entry points share it).Call sites that were passing a pre-flattened type list as a single string were converted to real lists:
fs.rename/fs.mkdtemppaths,fs.read/fs.writebuffers,pbkdf2/scryptpassword and salt,randomFillbuf,zlibdata and dictionary,AbortSignal,Uint32Array,SecureContext, andprocess.send.throw_invalid_argument_type_value_one_ofhad one caller and is subsumed by the list helper, so it is deleted.Every converted message was checked against
node v26.3.0output. Before / after, with the node output in the middle:new net.SocketAddress({address:5})The "options.address" argument must be of type string.The "options.address" property must be of type string.fs.mkdtempSync(5)must be of type string, Buffer, or URLmust be of type string or an instance of Buffer or URLfs.readSync(1,"x")must be of type TypedArraymust be an instance of Buffer, TypedArray, or DataViewfs.writeSync(1,5)must be of type string or TypedArraymust be of type string or an instance of Buffer, TypedArray, or DataViewcrypto.pbkdf2Sync(5,…)must be of type string or buffermust be of type string or an instance of ArrayBuffer, Buffer, TypedArray, or DataViewcrypto.randomFillSync(5)must be of type ArrayBuffer or ArrayBufferViewmust be an instance of ArrayBuffer or ArrayBufferViewzlib.deflateSync(5)must be of type string, Buffer, TypedArray, DataView, or ArrayBuffermust be of type string or an instance of Buffer, TypedArray, DataView, or ArrayBufferzlib.createDeflate({dictionary:5})The "options.dictionary" property must be of type Buffer, …The "options.dictionary" property must be an instance of Buffer, TypedArray, DataView, or ArrayBuffer2.
test/js/node/test/commonresynced with node v26.3.0The shared harness had drifted, which silently weakens every test that imports it.
fixtures.jsused spread where upstream deliberately usesReflect.apply— that difference is the whole point oftest-require-delete-array-iterator.js.debugger.js,inspector-helper.js,tmpdir.js,benchmark.js,child_process.jsandinternet.jswere older upstream revisions, missingwaitUntil, thestderrOutputgetter and the settabletmpdir.path.debugger-probe.js,test-error-reporter.js,v8-max-heap-size-option.js,websocket-server.js,README.md.All of the above are now byte-identical to upstream.
common/index.jsexports through aProxythat throws on an unknown property, so a missing helper aborts any test that reads it before it runs a single assertion. Eight upstream v26 members were absent and are added:hasFFI,hasFullICU,hasInspector,isRiscv64,skipIfFFIMissing,expectRequiredTLAError,resolveBuiltBinary,usesSharedLibrary.index.mjsre-exports them.Deliberate Bun adaptations were left alone:
crypto.js(opensslClidiscovery),net.js(hasMultiLocalhostwithoutinternal/test/binding),boringssl.js,gc.js(onGConFinalizationRegistry) and the Bun-specific parts ofindex.jsall keep their existing explanatory comments.3. Seven vendored tests un-drifted
These had their expected
ERR_INVALID_ARG_TYPEtext edited to match Bun's wrong message. They pass verbatim against upstream v26.3.0 now, so the upstream file is restored:test-zlib-not-string-or-buffer.js— the one that directly exercises fix 1test-zlib-deflate-constructors.js,test-zlib-zero-windowBits.jstest-crypto-keyobject-brand-check.js,test-dns-setservers-type-check.jstest-net-connect-options-invalid.js,test-url-format-whatwg.jsEach was canary-checked: appending a
throwto the restored file makes it fail, so they are really executing rather than skipping.How it was verified
Debug build, one process per file, the way CI runs them (
run --config=bunfig.node-test.toml,BUN_GARBAGE_COLLECTOR_LEVEL=1,BUN_FEATURE_FLAG_INTERNAL_FOR_TESTING=1,NO_COLOR=1, distinctTEST_SERIAL_ID).common/tmpdir,common/fixtures,common/child_processconsumers): 156 pass, 0 fail.test-zlib-*, all 20 vendoredtest-sqlite-*,test-socketaddress.js,test-require-delete-array-iterator.js, the 6common/debuggerconsumers and the 1common/inspector-helperconsumer: all pass.test/js/node/fs/fs.test.ts(453),test/js/node/zlib/zlib.test.js(386),test/js/node/crypto/node-crypto.test.js(202),test/js/node/crypto/crypto.hmac.test.ts(74),test/js/node/child_process/child_process-node.test.js(34): 0 failures.test/,packages/,docs/,bench/for each of the twelve old phrasings and for every dotted argument name reaching the changed helper. The only assertion on an old phrasing wastest-zlib-not-string-or-buffer.js, which is restored to upstream above. Thesqlitetests that assert"options.x" argumentare unaffected — those messages come from raw C++ strings innode_sqlite, not from this helper.git diff origin/main...HEAD -- test/expectations.txtis empty.4.
ERR_INVALID_THIS/ERR_MISSING_ARGSfrom WebCore bindingsSame bug class, different renderer:
makeThisTypeErrorMessageproducedCan only call X.y on instances of X. Node'sERR_INVALID_THISisValue of "this" must be of type X.next()producedCannot call next() on a non-Iterator object. Node names the iterator interface:Value of "this" must be of type URLSearchParamsIterator.URLSearchParams.get/getAll/has/delete/append/setthrew a bareNot enough arguments. Node names them:The "name" argument must be specified,The "name" and "value" arguments must be specified.That restores eleven more vendored files to upstream text:
test-whatwg-url-custom-searchparams-{append,delete,entries,foreach,get,getAll,has,keys,set,stringifier,values}.js.test-whatwg-url-invalidthis.jskeeps a local edit — upstream expects node's class-private-field wording (Receiver must be an instance of class), which a WebCore-backedURLcannot produce. Its one stale regex is updated to the new message so it does not silently pass on the old one.Verified:
test/js/web/url,test/js/web/fetch/headers.test.ts,test/js/web/encoding,test/js/web/abortandtest/js/web/urlpattern— 1109 pass, 0 fail. All 42 vendoredtest-url*/test-whatwg-url*files pass excepttest-url-parse-invalid-input.js, which times out on this branch and on the base branch alike (unmodified by this PR).5. 110 more vendored tests restored to upstream text
Separate mechanical commit. Every vendored file under
test/js/node/test/{parallel,sequential}was diffed against~/code/nodeatv26.3.0; 451 had drifted. Each was overwritten with the upstream file and run; the ones that passed verbatim are kept, the rest reverted untouched. 110 kept.The drift being undone is commented-out assertions, older revisions of the same test, and
process.binding('uv')in place ofrequire('internal/test/binding').Files whose drift encodes a platform guard, a timing tolerance, or a Bun-specific skip were excluded from this commit even when they passed here — this box is root-only Linux, so a guard for macOS or Windows cannot be validated locally. The filter drops any file whose diff touches
isWindows/isMacOS/isIBMi/isAIX/isLinux/process.platform/process.arch/common.skip/skipIf/platformTimeout/setTimeout/Date.now/TODO: BUN. 27 files were reverted on that basis.Known gaps
fs.read(fd, 4)reports the missing-callback error where node reports the buffer-type error first; the validation order innode_fs.rsdiffers from node's. Not fixed here —test-fs-read-type.js,test-fs-read.js,test-fs-buffer.jsandtest-fs-write-buffer-large.jsstill keep their commented-out message assertions because of it.ERR_INVALID_THISfrom WebCore bindings saysCan only call X.y on instances of Xwhere node saysValue of "this" must be of type X, andERR_MISSING_ARGSsaysNot enough argumentswhere node names the argument. Together those two shared messages block ~13 vendoredtest-whatwg-url-custom-searchparams-*andtest-eventtargettests from being restored verbatim. That is a change to a message shared by every WebCore binding, so it wants its own PR.