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 29152e29c5c..5dc7ff18dec 100644 --- a/src/jsc/ConsoleObject.rs +++ b/src/jsc/ConsoleObject.rs @@ -6011,18 +6011,23 @@ 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. // TODO: this does an extra JSONStringify and we don't need it to! - let snapshot: [JSValue; 1] = [global_this.generate_heap_snapshot()]; - // 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, - ); + let snapshot: [JSValue; 1] = match global_this.generate_heap_snapshot() { + Ok(snapshot) => [snapshot], + 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 + MessageType::Log, + MessageLevel::Debug, + global_this, + snapshot.as_ptr(), + 1, + ) { + global_this.report_active_exception_as_unhandled(err); } } diff --git a/src/jsc/JSGlobalObject.rs b/src/jsc/JSGlobalObject.rs index 8760daf7abb..7f05a6f3d1b 100644 --- a/src/jsc/JSGlobalObject.rs +++ b/src/jsc/JSGlobalObject.rs @@ -966,8 +966,20 @@ impl JSGlobalObject { }) } - pub(crate) fn generate_heap_snapshot(&self) -> JSValue { - JSC__JSGlobalObject__generateHeapSnapshot(self) + 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)) } /// DEPRECATED — use [`TopExceptionScope`](crate::TopExceptionScope) to check for exceptions 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/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..b74986ac17e --- /dev/null +++ b/test/js/bun/console/console-take-heap-snapshot.test.ts @@ -0,0 +1,97 @@ +import { describe, expect, it } from "bun:test"; +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 () => { + // 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. + 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]); + expect(stdout).toContain(`type: "Inspector"`); + expect(stdout.endsWith("done\n")).toBe(true); + expect(uncheckedScopes(stderr)).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); + }); + + // 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); + }); +});