From 88419b22b5fe169eaf3e05fb84e9eded29a5693f Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 6 Aug 2026 20:19:43 +0000 Subject: [PATCH 1/4] console: propagate heap snapshot JSONParse failures instead of asserting JSC__JSGlobalObject__generateHeapSnapshot ended with JSONParse followed by scope.releaseAssertNoException(), so an exception from JSONParse (out of memory building the parse tree, or one left pending during snapshot generation) aborted the process. A null string from HeapSnapshotBuilder::json(), returned when the snapshot overflows the maximum string length, passed an empty JSValue into the console formatter with no exception at all. The binding now returns empty with the exception pending (throwing an out-of-memory error for the null-string case), and the console.takeHeapSnapshot hook reports that exception through the uncaught-exception path instead of printing the snapshot, because JSC's consoleProtoFuncTakeHeapSnapshot performs no exception check after the client call, so the exception must not stay pending there. --- src/jsc/ConsoleObject.rs | 12 ++++- src/jsc/JSGlobalObject.rs | 4 +- src/jsc/bindings/bindings.cpp | 11 ++-- .../console-take-heap-snapshot.test.ts | 54 +++++++++++++++++++ 4 files changed, 75 insertions(+), 6 deletions(-) create mode 100644 test/js/bun/console/console-take-heap-snapshot.test.ts diff --git a/src/jsc/ConsoleObject.rs b/src/jsc/ConsoleObject.rs index 29152e29c5c..229bb079a32 100644 --- a/src/jsc/ConsoleObject.rs +++ b/src/jsc/ConsoleObject.rs @@ -6012,7 +6012,17 @@ pub(crate) extern "C" fn Bun__ConsoleObject__takeHeapSnapshot( _len: usize, ) { // TODO: this does an extra JSONStringify and we don't need it to! - let snapshot: [JSValue; 1] = [global_this.generate_heap_snapshot()]; + let snapshot: [JSValue; 1] = match global_this.generate_heap_snapshot() { + Ok(snapshot) => [snapshot], + // The exception must not stay pending: JSC's + // `consoleProtoFuncTakeHeapSnapshot` performs no exception check after + // this client call, so report it as uncaught instead (termination just + // stops the print and keeps the VM unwinding). + Err(err) => { + let _ = crate::task::report_error_or_terminate(global_this, err); + return; + } + }; // SAFETY: re-entry into our own host shim with a stack-local args slice. unsafe { message_with_type_and_level( diff --git a/src/jsc/JSGlobalObject.rs b/src/jsc/JSGlobalObject.rs index 8760daf7abb..d0eefb0c5c6 100644 --- a/src/jsc/JSGlobalObject.rs +++ b/src/jsc/JSGlobalObject.rs @@ -966,8 +966,8 @@ impl JSGlobalObject { }) } - pub(crate) fn generate_heap_snapshot(&self) -> JSValue { - JSC__JSGlobalObject__generateHeapSnapshot(self) + pub(crate) fn generate_heap_snapshot(&self) -> JsResult { + crate::from_js_host_call(self, || JSC__JSGlobalObject__generateHeapSnapshot(self)) } /// DEPRECATED — use [`TopExceptionScope`](crate::TopExceptionScope) to check for exceptions diff --git a/src/jsc/bindings/bindings.cpp b/src/jsc/bindings/bindings.cpp index 48e9cad873e..571520f65bf 100644 --- a/src/jsc/bindings/bindings.cpp +++ b/src/jsc/bindings/bindings.cpp @@ -3930,9 +3930,14 @@ JSC::EncodedJSValue JSC__JSGlobalObject__generateHeapSnapshot(JSC::JSGlobalObjec snapshotBuilder.buildSnapshot(); WTF::String jsonString = snapshotBuilder.json(); - JSC::EncodedJSValue result = JSC::JSValue::encode(JSONParse(globalObject, jsonString)); - scope.releaseAssertNoException(); - return result; + RETURN_IF_EXCEPTION(scope, {}); + if (jsonString.isNull()) [[unlikely]] { + // HeapSnapshotBuilder::json() returns the null string when the snapshot + // overflowed the maximum string length or its allocation failed. + throwOutOfMemoryError(globalObject, scope); + return {}; + } + RELEASE_AND_RETURN(scope, JSC::JSValue::encode(JSONParseWithException(globalObject, jsonString))); } // One load. always_inline so ThinLTO importers and the inliner never leave diff --git a/test/js/bun/console/console-take-heap-snapshot.test.ts b/test/js/bun/console/console-take-heap-snapshot.test.ts new file mode 100644 index 00000000000..ba8ee10a3ca --- /dev/null +++ b/test/js/bun/console/console-take-heap-snapshot.test.ts @@ -0,0 +1,54 @@ +import { describe, expect, it } from "bun:test"; +import { bunEnv, bunExe } from "harness"; + +describe.concurrent("console.takeHeapSnapshot", () => { + it("prints the parsed snapshot and survives BUN_JSC_validateExceptionChecks", async () => { + // JSONParse of the snapshot JSON can throw (out of memory building the + // parse tree), so the binding must hand the exception back to the console + // hook instead of releaseAssertNoException()-crashing, and the hook must + // report it rather than leave it pending under JSC's + // consoleProtoFuncTakeHeapSnapshot, whose scope performs no exception + // check after the client call. validateExceptionChecks aborts on debug + // builds if any scope is left unchecked; on release builds the option is + // a no-op and this just exercises the snapshot path. + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", `console.takeHeapSnapshot(); console.takeHeapSnapshot("label"); console.log("done");`], + env: { ...bunEnv, BUN_JSC_validateExceptionChecks: "1" }, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + const uncheckedScopes = stderr + .split("\n") + .map(line => line.trim()) + .filter(line => line.startsWith("This scope can throw") || line.startsWith("But the exception was unchecked")); + expect(stdout).toContain(`type: "Inspector"`); + expect(stdout.endsWith("done\n")).toBe(true); + expect(uncheckedScopes).toEqual([]); + expect(exitCode).toBe(0); + }); + + it("propagates exceptions thrown while coercing the label", async () => { + // The label toString runs before the snapshot is taken; the console + // function checks for the exception and rethrows it. + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + `try { + console.takeHeapSnapshot({ toString() { throw new Error("boom"); } }); + console.log("did not throw"); + } catch (e) { + console.log("caught:", e.message); + }`, + ], + env: { ...bunEnv, BUN_JSC_validateExceptionChecks: "1" }, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stdout).toBe("caught: boom\n"); + expect(stderr).toBe(""); + expect(exitCode).toBe(0); + }); +}); From 3562d82f93d32f771eb33e2515dbd7afadd6d668 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 6 Aug 2026 20:43:58 +0000 Subject: [PATCH 2/4] console: report snapshot formatting failures instead of leaving them pending The takeHeapSnapshot hook called the message_with_type_and_level host shim, which leaves formatter errors pending on the VM for the C++ caller to check. consoleProtoFuncTakeHeapSnapshot performs no such check, so a formatter failure hit the same unchecked-exception state the previous commit fixed for the parse step. Call the inner JsResult-returning function directly and route its error through the same uncaught-exception reporting. --- src/jsc/ConsoleObject.rs | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/src/jsc/ConsoleObject.rs b/src/jsc/ConsoleObject.rs index 229bb079a32..59e0c071275 100644 --- a/src/jsc/ConsoleObject.rs +++ b/src/jsc/ConsoleObject.rs @@ -6011,28 +6011,27 @@ pub(crate) extern "C" fn Bun__ConsoleObject__takeHeapSnapshot( _chars: *const u8, _len: usize, ) { + // No exception may stay pending when this hook returns: JSC's + // `consoleProtoFuncTakeHeapSnapshot` performs no exception check after the + // client call, so report failures as uncaught instead (termination just + // stops the print and keeps the VM unwinding). // TODO: this does an extra JSONStringify and we don't need it to! let snapshot: [JSValue; 1] = match global_this.generate_heap_snapshot() { Ok(snapshot) => [snapshot], - // The exception must not stay pending: JSC's - // `consoleProtoFuncTakeHeapSnapshot` performs no exception check after - // this client call, so report it as uncaught instead (termination just - // stops the print and keeps the VM unwinding). Err(err) => { let _ = crate::task::report_error_or_terminate(global_this, err); return; } }; - // SAFETY: re-entry into our own host shim with a stack-local args slice. - unsafe { - message_with_type_and_level( - core::ptr::null_mut(), // unused by the callee - MessageType::Log, - MessageLevel::Debug, - global_this, - snapshot.as_ptr(), - 1, - ); + if let Err(err) = message_with_type_and_level_( + core::ptr::null_mut(), // unused by the callee + MessageType::Log, + MessageLevel::Debug, + global_this, + snapshot.as_ptr(), + 1, + ) { + let _ = crate::task::report_error_or_terminate(global_this, err); } } From 7a093dbb7631951b1a8a588ed68569c7e00d55f0 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 6 Aug 2026 21:16:41 +0000 Subject: [PATCH 3/4] console.takeHeapSnapshot: make the failure path testable and fix the same hole in Bun.generateHeapSnapshot Self-review follow-ups: - Report failures through JSGlobalObject::report_active_exception_as_unhandled, the method built for exceptions raised in a native context with nowhere to propagate, instead of borrowing the task dispatcher's helper and discarding its termination sentinel. - Add BUN_INTERNAL_FAIL_TAKE_HEAP_SNAPSHOT, a debug-only fault-injection flag (same convention as BUN_INTERNAL_FAIL_PIPE_READER_START) that makes generate_heap_snapshot throw out-of-memory at the seam where a real JSONParse failure would surface. Two new debug-only tests pin the reported behavior: uncaught report with exit 1 and execution continuing, and interception via process.on(uncaughtException), both clean under BUN_JSC_validateExceptionChecks=1. These fail on the previous code. - Give Bun.generateHeapSnapshot the same null-string guard: its JSC branch parsed builder.json() with no isNull() check, so an overflowed snapshot returned an empty value with no exception, which is not a valid host function result. --- src/bun_core/env_var.rs | 2 + src/jsc/ConsoleObject.rs | 10 +-- src/jsc/JSGlobalObject.rs | 12 ++++ src/jsc/bindings/BunObject.cpp | 8 +++ .../console-take-heap-snapshot.test.ts | 61 ++++++++++++++++--- 5 files changed, 77 insertions(+), 16 deletions(-) diff --git a/src/bun_core/env_var.rs b/src/bun_core/env_var.rs index 9d0cfef32b3..3b78d9b19ef 100644 --- a/src/bun_core/env_var.rs +++ b/src/bun_core/env_var.rs @@ -261,6 +261,8 @@ pub mod feature_flag { new_feature_flag!(pub BUN_INTERNAL_BUNX_INSTALL, "BUN_INTERNAL_BUNX_INSTALL", {}); // Debug-only fault injection for test/js/bun/spawn/spawn-pipe-start-error.test.ts. new_feature_flag!(pub BUN_INTERNAL_FAIL_PIPE_READER_START, "BUN_INTERNAL_FAIL_PIPE_READER_START", {}); + // Debug-only fault injection for test/js/bun/console/console-take-heap-snapshot.test.ts. + new_feature_flag!(pub BUN_INTERNAL_FAIL_TAKE_HEAP_SNAPSHOT, "BUN_INTERNAL_FAIL_TAKE_HEAP_SNAPSHOT", {}); // Test-only: bypass the stdin isatty gate in `bun update --interactive` so // tests can drive the multi-select by writing keystrokes to a pipe. new_feature_flag!(pub BUN_INTERNAL_INTERACTIVE_ASSUME_TTY, "BUN_INTERNAL_INTERACTIVE_ASSUME_TTY", {}); diff --git a/src/jsc/ConsoleObject.rs b/src/jsc/ConsoleObject.rs index 59e0c071275..5dc7ff18dec 100644 --- a/src/jsc/ConsoleObject.rs +++ b/src/jsc/ConsoleObject.rs @@ -6013,15 +6013,11 @@ pub(crate) extern "C" fn Bun__ConsoleObject__takeHeapSnapshot( ) { // No exception may stay pending when this hook returns: JSC's // `consoleProtoFuncTakeHeapSnapshot` performs no exception check after the - // client call, so report failures as uncaught instead (termination just - // stops the print and keeps the VM unwinding). + // client call, so report failures as uncaught instead. // TODO: this does an extra JSONStringify and we don't need it to! let snapshot: [JSValue; 1] = match global_this.generate_heap_snapshot() { Ok(snapshot) => [snapshot], - Err(err) => { - let _ = crate::task::report_error_or_terminate(global_this, err); - return; - } + Err(err) => return global_this.report_active_exception_as_unhandled(err), }; if let Err(err) = message_with_type_and_level_( core::ptr::null_mut(), // unused by the callee @@ -6031,7 +6027,7 @@ pub(crate) extern "C" fn Bun__ConsoleObject__takeHeapSnapshot( snapshot.as_ptr(), 1, ) { - let _ = crate::task::report_error_or_terminate(global_this, err); + global_this.report_active_exception_as_unhandled(err); } } diff --git a/src/jsc/JSGlobalObject.rs b/src/jsc/JSGlobalObject.rs index d0eefb0c5c6..7f05a6f3d1b 100644 --- a/src/jsc/JSGlobalObject.rs +++ b/src/jsc/JSGlobalObject.rs @@ -967,6 +967,18 @@ impl JSGlobalObject { } pub(crate) fn generate_heap_snapshot(&self) -> JsResult { + // Debug-only fault injection for + // test/js/bun/console/console-take-heap-snapshot.test.ts: a real + // out-of-memory inside the snapshot's JSONParse cannot be staged from + // JS (constrained memory fails the earlier WTF-side snapshot + // allocations non-recoverably), so the console hook's error path is + // exercised this way. + #[cfg(debug_assertions)] + if bun_core::env_var::feature_flag::BUN_INTERNAL_FAIL_TAKE_HEAP_SNAPSHOT.get() + == Some(true) + { + return Err(self.throw_out_of_memory()); + } crate::from_js_host_call(self, || JSC__JSGlobalObject__generateHeapSnapshot(self)) } diff --git a/src/jsc/bindings/BunObject.cpp b/src/jsc/bindings/BunObject.cpp index 696460b194a..1489d4851e6 100644 --- a/src/jsc/bindings/BunObject.cpp +++ b/src/jsc/bindings/BunObject.cpp @@ -840,6 +840,14 @@ JSC_DEFINE_HOST_FUNCTION(functionGenerateHeapSnapshot, (JSC::JSGlobalObject * gl JSC::HeapSnapshotBuilder builder(heapProfiler); builder.buildSnapshot(); auto json = builder.json(); + if (json.isNull()) [[unlikely]] { + // HeapSnapshotBuilder::json() returns the null string when the snapshot + // overflowed the maximum string length or its allocation failed; + // JSONParseWithException maps a null string to an empty value without + // throwing, which is not a valid host function result. + throwOutOfMemoryError(globalObject, throwScope); + return {}; + } // Returning an object was a bad idea but it's a breaking change // so we'll just keep it for now. JSC::JSValue jsonValue = JSONParseWithException(globalObject, json); diff --git a/test/js/bun/console/console-take-heap-snapshot.test.ts b/test/js/bun/console/console-take-heap-snapshot.test.ts index ba8ee10a3ca..b74986ac17e 100644 --- a/test/js/bun/console/console-take-heap-snapshot.test.ts +++ b/test/js/bun/console/console-take-heap-snapshot.test.ts @@ -1,5 +1,14 @@ import { describe, expect, it } from "bun:test"; -import { bunEnv, bunExe } from "harness"; +import { bunEnv, bunExe, isDebug } from "harness"; + +// Lines printed by BUN_JSC_validateExceptionChecks=1 when a throw scope is +// left unchecked (the option aborts on debug builds; release builds ignore it). +function uncheckedScopes(stderr: string): string[] { + return stderr + .split("\n") + .map(line => line.trim()) + .filter(line => line.startsWith("This scope can throw") || line.startsWith("But the exception was unchecked")); +} describe.concurrent("console.takeHeapSnapshot", () => { it("prints the parsed snapshot and survives BUN_JSC_validateExceptionChecks", async () => { @@ -8,9 +17,7 @@ describe.concurrent("console.takeHeapSnapshot", () => { // hook instead of releaseAssertNoException()-crashing, and the hook must // report it rather than leave it pending under JSC's // consoleProtoFuncTakeHeapSnapshot, whose scope performs no exception - // check after the client call. validateExceptionChecks aborts on debug - // builds if any scope is left unchecked; on release builds the option is - // a no-op and this just exercises the snapshot path. + // check after the client call. await using proc = Bun.spawn({ cmd: [bunExe(), "-e", `console.takeHeapSnapshot(); console.takeHeapSnapshot("label"); console.log("done");`], env: { ...bunEnv, BUN_JSC_validateExceptionChecks: "1" }, @@ -18,13 +25,9 @@ describe.concurrent("console.takeHeapSnapshot", () => { stderr: "pipe", }); const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); - const uncheckedScopes = stderr - .split("\n") - .map(line => line.trim()) - .filter(line => line.startsWith("This scope can throw") || line.startsWith("But the exception was unchecked")); expect(stdout).toContain(`type: "Inspector"`); expect(stdout.endsWith("done\n")).toBe(true); - expect(uncheckedScopes).toEqual([]); + expect(uncheckedScopes(stderr)).toEqual([]); expect(exitCode).toBe(0); }); @@ -51,4 +54,44 @@ describe.concurrent("console.takeHeapSnapshot", () => { expect(stderr).toBe(""); expect(exitCode).toBe(0); }); + + // A real out-of-memory inside the snapshot's JSONParse cannot be staged from + // JS (constrained memory fails the earlier WTF-side snapshot allocations + // non-recoverably), so a debug-only fault-injection env var forces + // generate_heap_snapshot to throw at the same seam; before the fix the same + // pending exception aborted the process in releaseAssertNoException(). + it.skipIf(!isDebug)("reports a failed snapshot as an uncaught exception", async () => { + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", `console.takeHeapSnapshot(); console.log("after");`], + env: { ...bunEnv, BUN_INTERNAL_FAIL_TAKE_HEAP_SNAPSHOT: "1", BUN_JSC_validateExceptionChecks: "1" }, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toContain("Out of memory"); + expect(uncheckedScopes(stderr)).toEqual([]); + // The console call does not abort or rethrow; execution continues and the + // process exits 1 because the error went through the uncaught path. + expect(stdout).toBe("after\n"); + expect(exitCode).toBe(1); + }); + + it.skipIf(!isDebug)("a failed snapshot error is interceptable via process.on(uncaughtException)", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + `process.on("uncaughtException", e => console.log("handled:", e.message)); + console.takeHeapSnapshot(); + console.log("after");`, + ], + env: { ...bunEnv, BUN_INTERNAL_FAIL_TAKE_HEAP_SNAPSHOT: "1", BUN_JSC_validateExceptionChecks: "1" }, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(uncheckedScopes(stderr)).toEqual([]); + expect(stdout).toBe("handled: Out of memory\nafter\n"); + expect(exitCode).toBe(0); + }); }); From 56cf03b9b8fde7f26969b93dbb06134611db73bd Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 6 Aug 2026 21:29:55 +0000 Subject: [PATCH 4/4] ci: retrigger