Bun.serve: fix crash and cross-request 500 when inspecting a non-Response return value throws

[robobun]

Problem

• A fetch handler returns a non-Response value whose inspection throws, such as a throwing [util.inspect.custom]. Bun 1.4.2 aborts (panic(main thread): abort() called) at the next new timer callback, or answers another connection's request with a 500 that carries this exception. Debug builds: panic: a task returned Ok with a JS exception pending.
• Cause: render_missing_invalid_response (src/runtime/server/RequestContext.rs:804) prints the value through a Display adapter that can only return fmt::Error. The JS exception stays pending on the VM until other JavaScript runs.

Fix

• Format the value with Formatter::format_value, which returns the JsResult. On failure, print Expected a Response object and pass the exception to run_error_handler, like an exception the handler threw.
• The console hook (src/runtime/jsc_hooks.rs:2026) returned Ok with the exception pending for Request, Response and four more types. It now returns the error. A returned Request with a throwing value in params reached it.
• Behavior change: a handler whose promise was still pending reported this exception as uncaught before (1.3.13, 1.4.2). It now gets the same 500.
• Verified: three new tests in test/js/bun/http/serve.test.ts fail on canary c6b7fcb and on a debug build of main. Also ran the inspect, console and routes suites. Self-reviewed: 6 concerns raised, 6 addressed.

Background

• For a non-Response return value, Bun.serve prints a diagnostic and answers 204.
• In Rust, Err(JsError) means an exception is pending on the VM. JavaScriptCore must not enter JavaScript while one is pending.
• run_error_handler is the path of an exception the fetch handler throws. It calls error() if there is one, and answers 500.

Notes

Repro, process abort (1.4.2 and canary c6b7fcb, 3 of 3). The client must be another process, so that the timer callback is the next JavaScript the server runs.

import { inspect } from "node:util";
process.on("uncaughtException", err => console.error("LISTENER: " + err.message));
let calls = 0;
const server = Bun.serve({
  port: 0,
  development: false,
  fetch() {
    if (calls++ === 0) return { [inspect.custom]() { throw new Error("boom"); } };
    return new Response("ok");
  },
});
setInterval(() => console.error("tick"), 400);
console.log(server.port);

curl it once. 1.4.2 answers 204, then:

error: Expected a Response object, but received '============================================================
Bun v1.4.2 (744846f84) Linux x64
panic(main thread): abort() called

Repro, error delivered to another request (1.4.2, 3 of 3). Open two connections, then write GET /bad and GET /good in the same tick. /bad returns the value above. Result: /bad gets 204, /good gets 500 error-handler: inspector threw for /bad, and the fetch handler never runs for /good. With this change /bad gets the 500 and /good gets 200.

What each build does with the throwing value, by the kind of handler:

handler	1.3.13	1.4.2, canary c6b7fcb	this branch
sync	204, no diagnostic, nothing thrown	204, exception left pending	500 through error()
async, already settled when the server looks	204, exception left pending	204, exception left pending	500 through error()
async, still pending (on_resolve)	204, then uncaughtException	204, then uncaughtException	500 through error()

"Exception left pending" means that the next JavaScript meets it. On 1.4.2 that is an abort at a timer callback that never ran before, or a 500 for another request, or an uncaughtException if other JS runs first. On 1.3.13 another request gets the 500 (3 of 3), and a timer callback reports the exception as uncaught without an abort. The sync path prints this diagnostic since 1.4.0 (#33120, commit 73dad76 is in the bun-v1.4.0 tag). I ran 1.4.2 and 1.3.13. On every build the diagnostic line was cut off after but received '.

Why error() and not uncaught. The first version of this PR reported the exception with crate::dispatch::fold, which is what the pending-promise path did by accident. A review pointed out that this ends a process that has no uncaughtException listener, and that only the pending-promise path had that behavior before. The exception belongs to one request, and this file already sends such errors to run_error_handler: a handler throw, a rejected promise, and an unsendable status (reject_unsendable_response, called from the same two places as this function). Without an error() hook the request gets the default 500 page and the error is printed, as for a handler throw.

A realistic trigger: a handler that returns a Mongoose document by mistake. With toObject: { virtuals: true } and a virtual getter that throws, util.inspect(doc) throws (checked with mongoose 9.10.1; on 1.4.2 the exception leaks the same way).

Values tried against the unfixed debug build:

• throwing [util.inspect.custom], sync handler, settled promise, pending promise: panic.
• Object.assign(new String("q"), { toString() { throw ... }, [Symbol.toPrimitive]() { throw ... } }): panic. The same fix covers it.
• a returned BunRequest with req.params.x = { [inspect.custom]() { throw ... } }: panic, and still a panic with the RequestContext.rs change alone. Request::write_format returns fmt::Error for the JS exception and the console hook dropped it. test_runner/pretty_format.rs already turns that result into Err(JsError::Thrown). The hook now does the same. A sink error with no exception behind it is still ignored, so console.log to a closed pipe does not start to throw.
• a getter that throws: the formatter prints [Getter] and does not call it.
• a Proxy whose ownKeys throws: the formatter clears that exception itself and prints {}.
• a throwing Error.prepareStackTrace during write_trace: no leak.

console.log(req) with the poisoned params throws the inspector's error on 1.4.2 and on this branch. Release behavior does not change there. JavaScriptCore found the pending exception when the host function returned.

Also run on this branch (debug build with ASAN, and again with BUN_JSC_validateExceptionChecks=1):

• no error() hook and no uncaughtException listener: 500 Something went wrong!, the error is printed, the server keeps serving.
• error() throws: default 500. error() returns another throwing value, directly or from a pending promise: default 500, error() is not called twice.
• error() calls server.stop(true): the client gets ECONNRESET, no ASAN report. render_missing_invalid_response already checks is_aborted_or_ended() after the diagnostic, because the formatter can run user code.
• development: true: 500 with the development error page.
• each new test five times: stable.
• test/js/bun/http/serve.test.ts whole file: 326 pass, 2 fail. The two fail in my container for reasons that do not involve this change: "should use correct error when using a root range port(error message with Bun.serve for EACCESS is shown as EADDRINUSE #7187)" (the container runs as root, so the bind succeeds) and "only serves /bun:info to loopback clients in development mode" (the egress proxy of the container answers the request to the external address).
• test/js/web/fetch/response.test.ts: "handle stack overflow" times out at 5 s on the debug build. It does not print or inspect anything.

Thrown values take another path. This PR covers a value that the handler returns. A value that a route throws with no error() handler is printed by the error printer (print_error_instance_body, src/jsc/VirtualMachine.rs). On main that printer discards the formatter's error. #36921 changes that line.

• For a plain object that holds such a value (throw { status: 400, doc }), error printer: clear the pending exception when formatting a non-Error uncaught value throws #36921 alone is enough.
• For throw req and Promise.reject(req) with such a value in req.params, both PRs are needed. This PR makes the Request printer return the error, and error printer: clear the pending exception when formatting a non-Error uncaught value throws #36921 makes the error printer take it.
• Checked on debug builds with ASAN of main bf42a52. With this PR alone, each thrown shape still hits ASSERTION FAILED: Unexpected exception observed. With error printer: clear the pending exception when formatting a non-Error uncaught value throws #36921 alone, throw req, Promise.reject(req) and throw [req] still hit it. With both PRs, each shape is clean.
• The tests for the thrown shapes need both changes, so they are not in this PR.
• One value formatter for console, Bun.inspect, the error printer and bun:test #44268 (head 8865f2c) also makes these printers return the error, with another signature. It keeps the Display adapter in render_missing_invalid_response, so the RequestContext.rs change here is still needed with it.

Not in this PR: a check in the request dispatcher that folds or asserts on an exception that is still pending when a handler's frame returns. It would stop any later leak of this kind from reaching another request. It is a wider change than this fix. The write_format family returns fmt::Error for a JS exception. A typed error there would remove the has_exception() check in the console hook and the four copies in pretty_format.rs. That touches eight files and is a separate refactor.

---

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

[robobun]

Status: ready for review.

How I reproduced it:

• Bun 1.4.2 and canary c6b7fcb: a sync fetch handler returns { [util.inspect.custom]() { throw new Error("boom") } }. With a client in another process, the server aborts (panic(main thread): abort() called) when the next new timer callback runs. With two requests in one event loop turn, the second one skips its handler and gets a 500 with the first one's exception.
• Debug build of main (630e921): the same request panics with a task returned Ok with a JS exception pending.
• USE_SYSTEM_BUN=1 bun test test/js/bun/http/serve.test.ts -t "whose inspection throws": 3 fail. bun bd test with base src/: 3 fail. bun bd test with this branch: 3 pass.

The scripts are in the Notes block of the description.

Since 716dcfe the exception goes to the request's error path (error() and a 500) and is no longer reported as uncaught. The description explains why.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Essentials

Run ID: c091df2f-628a-4c64-8655-18985a191d59

📥 Commits

Reviewing files that changed from the base of the PR and between a91a12d and 716dcfe.

📒 Files selected for processing (2)

• src/runtime/server/RequestContext.rs
• test/js/bun/http/serve.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 0 remain after this review.

---

Walkthrough

The change propagates pending JavaScript exceptions from runtime value formatting, adds fallback reporting for invalid responses, and tests synchronous, asynchronous, concurrent, and timer-ordered request scenarios.

Changes

Runtime inspection error handling

Layer / File(s)	Summary
Formatting error propagation src/runtime/jsc_hooks.rs, src/runtime/server/RequestContext.rs	Supported runtime-object formatters now propagate pending JsError::Thrown values. Invalid response reporting uses an owned buffer, emits inspected output after successful inspection, and routes captured exceptions through the request error handler.
Server regression coverage test/js/bun/http/serve.test.ts	Regression tests cover synchronous and asynchronous inspection failures, concurrent connections, timer ordering, diagnostic handling, fallback responses, subsequent successful requests, and normal process exit.

Suggested reviewers: jarred-sumner

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 716dc

No concrete merge-blocking risk remains in the changed invalid-response handling.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly identifies the main change: fixing Bun.serve crashes and cross-request 500 responses caused by thrown inspection of non-Response return values. It is specific and concise enough for …
Description check	✅ Passed	The description provides detailed problem, cause, fix, verification results, regression coverage, and scope limitations. It does not use the template headings "### What does this PR do?" and "### How …

---

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

[claude]

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Beyond the inline findings, I also checked the teardown path where an uncaughtException listener invoked by fold at src/runtime/server/RequestContext.rs:824 calls server.stop(true) — the existing RequestContextRef guard and the is_aborted_or_ended() re-check at line 832 already cover that re-entry — and the remaining FetchHeaders branch in console_print_runtime_object_inner, which propagates via ? and does not share the swallowed-write_format pattern.

Extended reasoning...

Two findings were posted inline (the behavioral change to exit 1 for listener-less servers on the sync path, and the serial subprocess tests). Separately I traced the re-entry from crate::dispatch::fold into a user uncaughtException listener that tears down the server: the request-context liveness guard and the post-diagnostic is_aborted_or_ended() check predate this PR and handle synchronous abort/end, so the new fold call does not introduce a use-after-free there. I also confirmed the formatted helper in src/runtime/jsc_hooks.rs covers all six write_format branches and that the seventh (FetchHeaders) already uses ?-propagation, so no sibling site in that function is left with the old pattern.

One verified lower-impact observation (a convention, logging or cleanup point) was not posted.

[claude]

Code review found no issues

No high-confidence issues detected in this change.

Still open from earlier reviews (2):

• Unresolved: 2 minor or pre-existing.

[claude]

The latest push addresses both earlier inline comments (the inspection error now goes to this request's error() via run_error_handler, and the new block is describe.concurrent); I found no bugs in this revision, but since it changes request-lifecycle exception routing in Bun.serve, a human look is still worthwhile.

What was reviewed:

• RequestContext.rs: take_error clears the pending exception before run_error_handler enters JS; the is_aborted_or_ended() early return drops the already-cleared error, so nothing stays pending on the VM.
• jsc_hooks.rs: rethrow_pending only propagates when has_exception() is true, so sink-only fmt::Errors (closed pipe) keep the old swallow behavior.
• Note the PR description's "Decision" paragraph (report as uncaught via fold, exit 1 without a listener) no longer matches the code, which answers 500 through error(); worth updating before merge.

Extended reasoning...

Overview

The PR touches three files: src/runtime/server/RequestContext.rs (render_missing_invalid_response now formats the returned value explicitly, captures a thrown inspection exception with take_error, and routes it to run_error_handler after the abort check), src/runtime/jsc_hooks.rs (the console hook for Response/Request/BuildArtifact/Blob/S3Client/Archive now returns Err(JsError::Thrown) when write_format failed with an exception pending instead of returning Ok(true)), and test/js/bun/http/serve.test.ts (a describe.concurrent block with three spawned-process tests covering sync/settled/pending/Request-return variants, cross-connection isolation, and the timer-callback crash).

Security risks

None specific to this change. The code paths run user JS during formatting, which was already the case; the fix makes an exception raised there attributable to the originating request rather than leaking into unrelated JS. No new input parsing, no credential or auth handling.

Level of scrutiny

Moderate. The diff is small and self-contained, but it changes observable behavior of the HTTP server (a request whose return value cannot be inspected now gets a 500 through error() rather than 204 plus a leaked exception) and modifies exception propagation in a shared console hook used by console.log/util.inspect. I checked that take_error consumes the pending exception before any JS runs, that the early is_aborted_or_ended() return leaves the VM clean, and that rethrow_pending is gated on has_exception() so pure sink failures are unchanged. The run_error_handler path mirrors what handle_reject already does for a rejected handler promise, so it follows an established pattern.

Other factors

Both inline comments from the earlier review were addressed in commit 716dcfe. The PR description still describes the earlier design (crate::dispatch::fold, uncaught, exit 1) and should be updated to match the merged behavior. The robobun evidence block reports no local test proof, deferring to CI, so a maintainer should confirm the new tests pass on the CI lanes before merging.

[robobun]

The description is current as of 716dcfe. It has no "Decision" paragraph anymore: the Fix section describes the run_error_handler path, and the Notes explain why the exception goes to error() and is not reported as uncaught. The new tests pass locally on a debug build with ASAN (3 of 3, five runs), fail with the base src/, and fail on canary c6b7fcb.