node: Node-style uncaught-error output — Error: prefix, quoted props, ResolveMessage surface (+4 tests) - #35527
Draft
cirospaciari wants to merge 22 commits into
Draft
cirospaciari wants to merge 22 commits into
cirospaciari wants to merge 22 commits into
Conversation
…ion dropped by branch merges The branch merge kept the bun_clap side of the Node-style CLI work but dropped its Arguments.rs consumers, leaving -e/-p/--eval/--print dead (every invocation fell through to help), --check/-c unregistered (a panic in ComptimeClap::find), --inspect-port/--debug-port unregistered, and the NODE_OPTIONS validation + node-style missing-value errors gone. Same story for node_fs.rs: types.rs kept from_js_required and BUFFER_EXPECTED_TYPES but the call sites converting fs argument validation to Node's error messages were dropped (dead-code errors). Also adds Node's ERR_MISSING_OPTION guard: any --allow-* flag without --permission exits 1 with 'TypeError [ERR_MISSING_OPTION]: --permission is required' (initializePermission, pre_execution.js), and updates the now-obsolete '--permission is not supported by Bun' test to assert the model is actually enabled.
…ng properties
Bun's uncaught-exception printer wrote a lowercase 'error:' prefix for
plain Errors (and promoted a 'CODE: '-prefixed message's code into the
name slot), double-quoted string properties, and lost the creation
stack and property dump when the uncaughtException handler rethrew.
Node prints the error's own name ('Error: boom', 'TypeError: x'),
single-quotes inspect strings (code: 'ENOENT'), and reports the
rethrown error's own stack.
- Formatter grows node_uncaught_style, set only on the uncaught/
unhandled print path (print_exception + the jsc_hooks funnel) and off
under bun test, so test-runner failure rendering is unchanged.
- print_error_name_and_message prints the name verbatim in that mode.
- String properties (and the code property) print single-quoted for
simple ASCII strings, falling back to the JSON writer otherwise.
- The rethrow-inside-uncaughtException path unwraps the JSC::Exception
wrapper before printing so the Error's own stack and properties
render (previously: rethrow-site frame only, no properties), and
util.callbackify's falsy-rejection message drops the stray 'a' to
match Node ('Promise was rejected with falsy value').
Unlocks the vendored test-unhandled-exception-rethrow-error.js and (in
the following commits) the permission tests that assert /Error: .../ on
child stderr. Bun-side assertions on the old rendering are updated in
the same commit.
ResolveMessage reported name 'ResolveMessage' and stringified as
'ResolveMessage: ...'. Node throws module-not-found as a plain Error:
String(err) is 'Error: Cannot find module 'x'' for CJS,
'Error [ERR_MODULE_NOT_FOUND]: ...' for ESM, and
'Error [ERR_UNKNOWN_BUILTIN_MODULE]: ...' for a missing node: builtin.
- name becomes a getter: 'Error' for the require/import module-not-found
shapes, 'ResolveMessage' otherwise (Bun.build logs etc. unchanged).
- toString()/.stack/toJSON use the same display name.
- The uncaught printer renders these as 'Error: Cannot find module 'x'
[newline] Require stack: ...' instead of the transpiler-log form.
- is_bare_esm_specifier no longer classifies URL-scheme specifiers
('file://...', 'pkg:bar') as bare packages; Node reports those as
URL/scheme errors, never "Cannot find package 'file:'", and the
module-form message keeps the full specifier.
This lets fixtures/require-resolve.js go back to byte-verbatim upstream
(its /^Error: Cannot find module/ anchors now match) and vendors
test-internal-modules.js.
test-permission-inspector.js, test-permission-inspector-brk.js, and test-permission-sqlite-load-extension.js all assert /Error: .../ (and code: 'ERR_...') on child stderr. The inspector gating they exercise (silently skip --inspect without --allow-inspector; uncaught ERR_ACCESS_DENIED for --inspect-brk, per inspector_agent.cc) ships in the ensure_debugger change of the printer commit.
Collaborator
This was referenced Jul 25, 2026
…aude/node-uncaught-printer-parity # Conflicts: # src/jsc/ConsoleObject.rs # src/runtime/cli/Arguments.rs
…aude/node-uncaught-printer-parity # Conflicts: # src/runtime/cli/Arguments.rs
…c/bun_runtime - src/jsc/bindings/BunHeapProfiler.h: restored (deleted by #36500 on main, still needed for $newCppFunction in src/js/node/v8.ts -> GeneratedJS2Native.h) - src/jsc/web_worker.rs: parent_ref binding was removed by the &self-only refactor merge; use the surrounding unsafe { (*parent).field } pattern - src/runtime/node/types.rs: define BUFFER_EXPECTED_TYPES imported by node_fs.rs - src/clap/lib.rs: Diagnostic fields pub (read by bun_runtime::cli::Arguments) - src/runtime/node/path.rs: resolve_{posix,windows}_t pub(crate) for permission.rs - src/runtime/permission.rs: std::sync::RwLock -> bun_threading::RwLock, std::env::var -> bun_core::env_var::NODE_OPTIONS, manual_contains lint - clippy: unreachable_pub (run_command.rs), undocumented_unsafe_blocks (Timer.rs), unnecessary_lazy_evaluations (BunHeapProfiler.rs)
…aude/node-uncaught-printer-parity # Conflicts: # src/jsc/bindings/BunHeapProfiler.h # src/runtime/permission.rs
…aude/node-uncaught-printer-parity
…aude/node-uncaught-printer-parity # Conflicts: # src/runtime/node/types.rs
…aude/node-uncaught-printer-parity # Conflicts: # src/runtime/cli/Arguments.rs # test/cli/run/run-eval.test.ts
…now that uncaught errors print Node-style
…ls via BUN_FEATURE_FLAG_INTERNAL_FOR_TESTING
…mes to the Node-style rendering
Collaborator
|
Heads-up for a rebase: #38278 makes |
This branch has not been deployed
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
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.
Important
Deliberate user-visible output change, shipped for explicit review/veto. Bun's uncaught-exception printer historically wrote a lowercase
error:prefix. This PR makes the uncaught/unhandled path print the error's ownnamethe way Node does (Error: boom,TypeError: x,Error: ENOENT: no such file…) and single-quotes string properties in the property dump (code: 'ENOENT').bun testfailure rendering andconsole.log(err)/Bun.inspect(err)are unchanged. This one prefix structurally blocked ~17+ reachable vendored Node tests that spawn a child and regex/Error: .../on its stderr — no amount of feature work reaches them.Stacked on
claude/node-v26-fix-tls. +5 upstream tests (4 newly vendored, 1 pre-existing fixture family restored to verbatim).test-internal-modules.jswas vendored at first and dropped again: the test harness setsBUN_FEATURE_FLAG_INTERNAL_FOR_TESTING, which since the quic work exposes internals, sorequire('internal/freelist')resolves under the harness and the test cannot pass in CI.What this does
1. Node-style uncaught-error rendering (
src/jsc/VirtualMachine.rs,src/jsc/ConsoleObject.rs,src/runtime/jsc_hooks.rs)A new
Formatter.node_uncaught_styleflag is set only on the uncaught/unhandled print funnel (print_exception+ the jsc_hooks funnel), and explicitly not underbun test, so the runner's classic failure output is untouched. In that mode:nameprints verbatim — no lowercasing ofError, no promoting aCODE:-prefixed message's code into the name slot (Error: ENOENT: no such file or directory, open 'x', matching Node);code: 'ERR_LOAD_SQLITE_EXTENSION',permission: 'FileSystemWrite'), falling back to the JSON writer for anything needing escapes;uncaughtExceptionhandler now unwraps theJSC::Exceptionwrapper before printing, so the error's own creation stack (at throwException (...)) and property dump render — previously only the rethrow-site frame printed, with no properties (exit code 7 was already correct).2. ResolveMessage gets Node's
name/toString()/.stacksurface (src/jsc/ResolveMessage.rs)String(err)on a module-not-found wasResolveMessage: ...; Node needsError: Cannot find module 'x'(CJS),Error [ERR_MODULE_NOT_FOUND]: ...(ESM),Error [ERR_UNKNOWN_BUILTIN_MODULE]: ...(missingnode:builtin) — all witherr.name === 'Error'(.codewas already right).namebecomes a getter soBun.buildlogs and non-module-resolution uses keep reportingResolveMessage. The uncaught printer renders these Node-style too (Error: Cannot find module 'x'+Require stack:).is_bare_esm_specifierno longer classifies URL-scheme specifiers (file://…,pkg:bar) as bare packages — Node never saysCannot find package 'file:', and the module-form message keeps the full specifier (all message shapes oracled against node v26.3.0).3. Node's inspector permission gate (
src/runtime/jsc_hooks.rs::ensure_debugger)Under
--permissionwithout--allow-inspector,--inspect/--inspect-waitare silently skipped and--inspect-brkraises uncaughtERR_ACCESS_DENIED(resourcePauseOnNextJavascriptStatement), exit 1 — verified against node v26.3.0's observed behavior (plain--inspectruns fine there; only the pause request throws).4. Node's
ERR_MISSING_OPTIONguard: any--allow-*flag without--permissionexits 1 withTypeError [ERR_MISSING_OPTION]: --permission is required(initializePermission, pre_execution.js).5.
util.callbackifyfalsy-rejection message drops the stray "a":Promise was rejected with falsy value, Node's exact wording.Base repairs (the branch did not compile)
claude/node-v26-fix-tls's merges dropped whole generations of already-merged work (kept one side of each conflict):reject_bad_negationsinitializer missing fromArguments.rs→ the base does not build;Arguments.rsconsumers of the Node-style CLI work were dropped while thebun_claphalf survived:-e/-p/--eval/--printwere completely dead (every invocation printed help),-c/--checkpanicked (--check is not a parameter),--inspect-port/--debug-portunregistered, NODE_OPTIONS validation and node-style missing-value errors gone;node_fs.rscall sites offrom_js_required/BUFFER_EXPECTED_TYPESdropped whiletypes.rskept the helpers (dead-code hard errors).These are restored as the first two commits (re-applied from the original commits, conflicts resolved by hand;
test/cli/run/run-eval.test.ts88→91 passing again, the fivetest-fs-*files from the fs-validation commit re-verified green). Every other agent branching from this base is currently broken the same way.Tests
Newly vendored (byte-verbatim, preflight-verified; each fails on the unfixed build, passes with this PR, and fails again with a throw appended):
parallel/test-internal-modules.js— anchors/^Error: Cannot find module 'internal\/freelist'/againstString(err)(no longer included, see above)parallel/test-permission-inspector.js— in-processSession.connect()denial + child/Error: Access to this API has been restricted/parallel/test-permission-inspector-brk.js—--inspect-brkunder--permissionexits 1 with the same lineparallel/test-permission-sqlite-load-extension.js—/Error: Cannot load SQLite extensions…/+/code: 'ERR_LOAD_SQLITE_EXTENSION'/parallel/test-unhandled-exception-rethrow-error.js— exit 7,/Error: boom/,/at throwException/,/rethrow: true/Restored to byte-verbatim upstream:
fixtures/require-resolve.js(its/^Error: Cannot find module/anchors now hold;test-require-resolve.jsre-verified green, and fails with the old binary).test-util-callbackify.js(pre-existing adapted vendored file on the base): updated itserror:-prefix and falsy-message assertions to the upstream spellings; still not fully verbatim — upstream additionally assertserrLines.length === 7(Bun's multi-line source excerpt makes this 10) andat process.processTicksAndRejectionsframe naming. This is the single remaining preflight drift, inherited from the base and reduced by this PR.Not vendored, with reasons:
test-node-output-{errors,console,vm,sourcemaps}.mjs— byte-exact snapshot comparisons requiring V8 frame naming (at Object.<anonymous> (...)),node:internalframes in stacks, V8 column numbers, and aNode.js <version>footer; all 30 error fixtures still fail. Unreachable without stack-shimming JSC to V8's frame model.test-dotenv-node-options.js— needsNODE_OPTIONSfrom--env-filefiles applied at startup (permission grants included); separate feature.Bun-side assertions on the old rendering updated in the same PR (each verified to fail on the old build):
hot.test.ts,process.test.js,sourcemap.test.js,reportError.test.ts,stack.test.ts,serve.test.ts,serve-direct-readable-stream.test.ts,async-iterator-stream.test.ts,run-autoinstall.test.ts,tsconfig-override.test.ts,run-eval.test.ts,resolve.test.ts,resolve-error.test.ts,resolve-test.js,import-meta.test.js. Suites re-run green: resolve/ (37 files), http server suites, worker_threads, web/workers, hot, process, run-eval, regression set.Known pre-existing issue surfaced while sweeping (not touched here): worker
terminate()leaks the per-workernode_fs_binding::Bindingunder LeakSanitizer (worker-terminate-lifetime.test.tsdns case) — unrelated to this diff.