From 3cc5220929adcd7a21f715768b4fa625795195a4 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 11 Aug 2026 01:18:22 +0000 Subject: [PATCH 1/5] Error.appendStackTrace: don't abort when Error.stackTraceLimit is not a number ErrorInstance::captureStackTrace() calls .value() on the global's stackTraceLimit optional, which is empty once Error.stackTraceLimit has been set to a non-number or deleted. With -fno-exceptions that is a plain abort(). Give the destination an empty frame list in that state instead of capturing. Also return early when the destination's error info has already been materialized: its frames were discarded and are never read again, and installing new ones trips ASSERT(!m_errorInfoMaterialized) in computeErrorInfo during GC. --- src/jsc/bindings/FormatStackTraceForJS.cpp | 15 +++++- test/js/node/v8/capture-stack-trace.test.js | 53 +++++++++++++++++++++ 2 files changed, 67 insertions(+), 1 deletion(-) diff --git a/src/jsc/bindings/FormatStackTraceForJS.cpp b/src/jsc/bindings/FormatStackTraceForJS.cpp index ccbfc31bef99..92b475577a82 100644 --- a/src/jsc/bindings/FormatStackTraceForJS.cpp +++ b/src/jsc/bindings/FormatStackTraceForJS.cpp @@ -669,8 +669,21 @@ JSC_DEFINE_HOST_FUNCTION(errorConstructorFuncAppendStackTrace, (JSC::JSGlobalObj return {}; } + // Once .stack is materialized the frames are discarded and never read again; + // installing new ones only trips ASSERT(!m_errorInfoMaterialized) in + // computeErrorInfo when GC finalizes the error. + if (destination->hasMaterializedErrorInfo()) { + return JSC::JSValue::encode(jsUndefined()); + } + if (!destination->stackTrace()) { - destination->captureStackTrace(vm, globalObject, 1); + // ErrorInstance::captureStackTrace() unwraps stackTraceLimit(), which is + // empty once Error.stackTraceLimit has been set to a non-number or deleted. + if (globalObject->stackTraceLimit()) { + destination->captureStackTrace(vm, globalObject, 1); + } else { + destination->setStackFrames(vm, {}); + } } if (source->stackTrace()) { diff --git a/test/js/node/v8/capture-stack-trace.test.js b/test/js/node/v8/capture-stack-trace.test.js index 21b1ddd39fbd..68986564158f 100644 --- a/test/js/node/v8/capture-stack-trace.test.js +++ b/test/js/node/v8/capture-stack-trace.test.js @@ -993,6 +993,59 @@ test("captureStackTrace does not crash when stackTraceLimit is non-numeric", () } }); +// Both of these abort the process when they fail, so they run in a child. +test.concurrent("Error.appendStackTrace does not abort when stackTraceLimit is non-numeric or deleted", async () => { + const src = ` + class Source { + constructor() { + this.error = new Error("source"); + } + } + const source = new Source().error; + + Error.stackTraceLimit = "foo"; + const destination = new Error("destination"); + Error.appendStackTrace(source, destination); + Error.appendStackTrace(new Error("a"), new Error("b")); + + delete Error.stackTraceLimit; + Error.appendStackTrace(new Error("c"), new Error("d")); + + process.stdout.write(JSON.stringify({ appended: destination.stack.includes("at new Source") })); + `; + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", src], + env: bunEnv, + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout, stderr, exitCode }).toEqual({ stdout: JSON.stringify({ appended: true }), stderr: "", exitCode: 0 }); +}); + +test.concurrent("Error.appendStackTrace is a no-op once the destination's .stack has been materialized", async () => { + const src = ` + const destination = new Error("destination"); + const stack = destination.stack; + for (let i = 0; i < 100; i++) { + Error.appendStackTrace(new Function("return new Error('source')")(), destination); + } + Bun.gc(true); + Bun.gc(true); + process.stdout.write(JSON.stringify({ unchanged: destination.stack === stack })); + `; + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", src], + env: bunEnv, + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout, stderr, exitCode }).toEqual({ + stdout: JSON.stringify({ unchanged: true }), + stderr: "", + exitCode: 0, + }); +}); + test("Error.stackTraceLimit default matches the limit captureStackTrace applies", async () => { // Run in a fresh process so nothing has written to Error.stackTraceLimit yet. const src = ` From 56a25d52cdaf631d626fd53ad8cab0d868e7320a Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 13 Aug 2026 00:43:16 +0000 Subject: [PATCH 2/5] Error.appendStackTrace: make self-append a no-op, cover materialized and unset-limit cases Appending an error's trace to itself made Vector::appendVector copy out of the buffer it had just reallocated and then clear() wiped the trace. Adds tests for the self-append, for destinations materialized through .sourceURL, and for Error.stackTraceLimit = undefined. --- src/jsc/bindings/FormatStackTraceForJS.cpp | 7 ++ test/js/node/v8/capture-stack-trace.test.js | 102 +++++++++++++++++++- 2 files changed, 106 insertions(+), 3 deletions(-) diff --git a/src/jsc/bindings/FormatStackTraceForJS.cpp b/src/jsc/bindings/FormatStackTraceForJS.cpp index 92b475577a82..f8a7c0097ac6 100644 --- a/src/jsc/bindings/FormatStackTraceForJS.cpp +++ b/src/jsc/bindings/FormatStackTraceForJS.cpp @@ -669,6 +669,13 @@ JSC_DEFINE_HOST_FUNCTION(errorConstructorFuncAppendStackTrace, (JSC::JSGlobalObj return {}; } + // Appending a trace to itself would make appendVector copy out of the buffer + // it just reallocated (the span overload does not rebase the source + // pointer), and the clear() below would then wipe the trace. + if (source == destination) { + return JSC::JSValue::encode(jsUndefined()); + } + // Once .stack is materialized the frames are discarded and never read again; // installing new ones only trips ASSERT(!m_errorInfoMaterialized) in // computeErrorInfo when GC finalizes the error. diff --git a/test/js/node/v8/capture-stack-trace.test.js b/test/js/node/v8/capture-stack-trace.test.js index 68986564158f..9401120dcfe9 100644 --- a/test/js/node/v8/capture-stack-trace.test.js +++ b/test/js/node/v8/capture-stack-trace.test.js @@ -1,7 +1,7 @@ import { nativeFrameForTesting } from "bun:internal-for-testing"; import { noInline } from "bun:jsc"; import { afterEach, expect, mock, test } from "bun:test"; -import { bunEnv, bunExe, tempDir } from "harness"; +import { bunEnv, bunExe, isASAN, tempDir } from "harness"; const origPrepareStackTrace = Error.prepareStackTrace; afterEach(() => { Error.prepareStackTrace = origPrepareStackTrace; @@ -993,7 +993,22 @@ test("captureStackTrace does not crash when stackTraceLimit is non-numeric", () } }); -// Both of these abort the process when they fail, so they run in a child. +test("Error.appendStackTrace moves the source's frames into the destination", () => { + function inner() { + try { + null(); + } catch (e) { + return e; + } + } + const source = inner(); + const destination = new Error("destination"); + Error.appendStackTrace(source, destination); + expect(destination.stack).toContain("at inner"); +}); + +// The rest of these abort the process (or trip ASAN) when they fail, so each +// runs its scenario in a child. test.concurrent("Error.appendStackTrace does not abort when stackTraceLimit is non-numeric or deleted", async () => { const src = ` class Source { @@ -1008,9 +1023,12 @@ test.concurrent("Error.appendStackTrace does not abort when stackTraceLimit is n Error.appendStackTrace(source, destination); Error.appendStackTrace(new Error("a"), new Error("b")); - delete Error.stackTraceLimit; + Error.stackTraceLimit = undefined; Error.appendStackTrace(new Error("c"), new Error("d")); + delete Error.stackTraceLimit; + Error.appendStackTrace(new Error("e"), new Error("f")); + process.stdout.write(JSON.stringify({ appended: destination.stack.includes("at new Source") })); `; await using proc = Bun.spawn({ @@ -1046,6 +1064,84 @@ test.concurrent("Error.appendStackTrace is a no-op once the destination's .stack }); }); +test.concurrent( + "Error.appendStackTrace onto errors materialized through .sourceURL does not assert when GC finalizes them", + async () => { + // Reading any of the lazily materialized properties (.stack, .line, + // .column, .sourceURL) discards the native frames. The errors are created + // inside eval'd functions so that each iteration's frames point at code GC + // can reclaim, which is what makes finalizeUnconditionally look at them. + const src = ` + const keep = []; + for (let i = 0; i < 200; i++) { + eval(\`(function inner\${i}() { + const a = new Error(); + const b = new Error(); + a.sourceURL; + b.sourceURL; + Error.appendStackTrace(a, b); + keep.push(b); + const c = new Error(); + const d = new Error(); + d.sourceURL; + Error.appendStackTrace(c, d); + keep.push(c, d); + })();\`); + } + Bun.gc(true); + Bun.gc(true); + process.stdout.write("ok"); + `; + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", src], + env: bunEnv, + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout, stderr, exitCode }).toEqual({ stdout: "ok", stderr: "", exitCode: 0 }); + }, +); + +test.concurrent( + "Error.appendStackTrace with the same error as source and destination leaves its trace alone", + async () => { + // The trace needs enough frames that appending it to itself reallocates the + // vector; the default stackTraceLimit of 10 is plenty once f() has recursed. + const src = ` + function f(n) { + if (n > 0) return f(n - 1) + 1; + try { + null(); + } catch (e) { + Error.appendStackTrace(e, e); + // Without the guard the trace ends up empty and .stack is undefined. + process.stdout.write(JSON.stringify({ frames: String(e.stack).split("\\n").filter(line => line.includes("at f ")).length })); + } + return 0; + } + f(64); + `; + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", src], + // WTF allocations normally come from bmalloc, where ASAN cannot see the + // freed buffer; Malloc=1 routes them through the system allocator. + // detect_leaks=0 keeps LeakSanitizer from reporting JSC's exit-time + // allocations under that allocator, and symbolize=0 keeps a failing child + // from spending seconds symbolizing the report. + env: isASAN + ? { + ...bunEnv, + Malloc: "1", + ASAN_OPTIONS: [bunEnv.ASAN_OPTIONS, "detect_leaks=0", "symbolize=0"].filter(Boolean).join(":"), + } + : bunEnv, + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout, stderr, exitCode }).toEqual({ stdout: JSON.stringify({ frames: 10 }), stderr: "", exitCode: 0 }); + }, +); + test("Error.stackTraceLimit default matches the limit captureStackTrace applies", async () => { // Run in a fresh process so nothing has written to Error.stackTraceLimit yet. const src = ` From 9981b500a7d432f007c0d8978d066061bb2b71c7 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 13 Aug 2026 02:49:11 +0000 Subject: [PATCH 3/5] Error.appendStackTrace: shorten the guard comments --- src/jsc/bindings/FormatStackTraceForJS.cpp | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/jsc/bindings/FormatStackTraceForJS.cpp b/src/jsc/bindings/FormatStackTraceForJS.cpp index f8a7c0097ac6..9c4cd6448b17 100644 --- a/src/jsc/bindings/FormatStackTraceForJS.cpp +++ b/src/jsc/bindings/FormatStackTraceForJS.cpp @@ -669,23 +669,18 @@ JSC_DEFINE_HOST_FUNCTION(errorConstructorFuncAppendStackTrace, (JSC::JSGlobalObj return {}; } - // Appending a trace to itself would make appendVector copy out of the buffer - // it just reallocated (the span overload does not rebase the source - // pointer), and the clear() below would then wipe the trace. + // appendVector() is not self-append safe, and the clear() below would then drop the trace. if (source == destination) { return JSC::JSValue::encode(jsUndefined()); } - // Once .stack is materialized the frames are discarded and never read again; - // installing new ones only trips ASSERT(!m_errorInfoMaterialized) in - // computeErrorInfo when GC finalizes the error. + // A materialized error never reads its frames again (see errorConstructorFuncCaptureStackTrace). if (destination->hasMaterializedErrorInfo()) { return JSC::JSValue::encode(jsUndefined()); } if (!destination->stackTrace()) { - // ErrorInstance::captureStackTrace() unwraps stackTraceLimit(), which is - // empty once Error.stackTraceLimit has been set to a non-number or deleted. + // captureStackTrace() unwraps stackTraceLimit(), which is empty when Error.stackTraceLimit is not a number. if (globalObject->stackTraceLimit()) { destination->captureStackTrace(vm, globalObject, 1); } else { From 1125138839c2bd516632c795632be9a9ef85fc17 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 13 Aug 2026 07:55:47 +0000 Subject: [PATCH 4/5] Error.appendStackTrace: assert the release-visible side of the materialized no-op The native error printer prefers an error's frames over its .stack string, so appending onto a materialized destination used to make it print the appendStackTrace call site and empty the source on every build, not only trip the assertion on debug builds. The materialized tests now check the printed frames and that the source keeps its trace, the basic test pins the frame order and that the source is consumed, and the guard's comment says what it is actually protecting. --- src/jsc/bindings/FormatStackTraceForJS.cpp | 5 +- test/js/node/v8/capture-stack-trace.test.js | 71 +++++++++++++++++---- 2 files changed, 61 insertions(+), 15 deletions(-) diff --git a/src/jsc/bindings/FormatStackTraceForJS.cpp b/src/jsc/bindings/FormatStackTraceForJS.cpp index 9c4cd6448b17..5e1d08d4439c 100644 --- a/src/jsc/bindings/FormatStackTraceForJS.cpp +++ b/src/jsc/bindings/FormatStackTraceForJS.cpp @@ -674,7 +674,10 @@ JSC_DEFINE_HOST_FUNCTION(errorConstructorFuncAppendStackTrace, (JSC::JSGlobalObj return JSC::JSValue::encode(jsUndefined()); } - // A materialized error never reads its frames again (see errorConstructorFuncCaptureStackTrace). + // Once .stack is materialized the frames are gone. Installing new ones would make the native error + // printer (which prefers frames over .stack) show this call site plus the source's frames, and trip + // computeErrorInfo's !m_errorInfoMaterialized assertion when GC finalizes the error. Leave both + // errors alone instead, as Bun__attachAsyncStackFromPromise does. if (destination->hasMaterializedErrorInfo()) { return JSC::JSValue::encode(jsUndefined()); } diff --git a/test/js/node/v8/capture-stack-trace.test.js b/test/js/node/v8/capture-stack-trace.test.js index 9401120dcfe9..b16fa3151251 100644 --- a/test/js/node/v8/capture-stack-trace.test.js +++ b/test/js/node/v8/capture-stack-trace.test.js @@ -993,7 +993,7 @@ test("captureStackTrace does not crash when stackTraceLimit is non-numeric", () } }); -test("Error.appendStackTrace moves the source's frames into the destination", () => { +test("Error.appendStackTrace moves the source's frames behind the destination's own", () => { function inner() { try { null(); @@ -1001,14 +1001,27 @@ test("Error.appendStackTrace moves the source's frames into the destination", () return e; } } + function makeDestination() { + const error = new Error("destination"); + error.name = "DestinationError"; + return error; + } const source = inner(); - const destination = new Error("destination"); + const destination = makeDestination(); Error.appendStackTrace(source, destination); - expect(destination.stack).toContain("at inner"); + + const lines = destination.stack.split("\n"); + const destinationFrame = lines.findIndex(line => line.includes("at makeDestination")); + const sourceFrame = lines.findIndex(line => line.includes("at inner")); + expect(lines[0]).toBe("DestinationError: destination"); + expect(destinationFrame).toBeGreaterThan(0); + expect(sourceFrame).toBeGreaterThan(destinationFrame); + // The frames are moved, not copied: the source has none left. + expect(source.stack).toBeUndefined(); }); -// The rest of these abort the process (or trip ASAN) when they fail, so each -// runs its scenario in a child. +// The rest of these can abort the process (or trip ASAN) when they fail, so +// each runs its scenario in a child. test.concurrent("Error.appendStackTrace does not abort when stackTraceLimit is non-numeric or deleted", async () => { const src = ` class Source { @@ -1041,15 +1054,37 @@ test.concurrent("Error.appendStackTrace does not abort when stackTraceLimit is n }); test.concurrent("Error.appendStackTrace is a no-op once the destination's .stack has been materialized", async () => { + // Without the guard, release builds install new frames on the destination + // (so the native printer shows the appendStackTrace call site instead of the + // frames .stack reports) and empty the source; debug builds also assert when + // GC finalizes the destination, which is what the new Function sources and + // the Bun.gc() calls provoke. const src = ` - const destination = new Error("destination"); - const stack = destination.stack; - for (let i = 0; i < 100; i++) { - Error.appendStackTrace(new Function("return new Error('source')")(), destination); + function makeDestination() { + const error = new Error("destination"); + error.stack; + return error; } + function appendAll(destination) { + let source; + for (let i = 0; i < 100; i++) { + source = new Function("return new Error('source')")(); + Error.appendStackTrace(source, destination); + } + return source; + } + const destination = makeDestination(); + const stack = destination.stack; + const lastSource = appendAll(destination); + const printed = Bun.inspect(destination); Bun.gc(true); Bun.gc(true); - process.stdout.write(JSON.stringify({ unchanged: destination.stack === stack })); + process.stdout.write(JSON.stringify({ + unchanged: destination.stack === stack, + printedOwnFrame: printed.includes("at makeDestination"), + printedAppendFrame: printed.includes("at appendAll"), + sourceIntact: typeof lastSource.stack === "string", + })); `; await using proc = Bun.spawn({ cmd: [bunExe(), "-e", src], @@ -1058,7 +1093,7 @@ test.concurrent("Error.appendStackTrace is a no-op once the destination's .stack }); const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); expect({ stdout, stderr, exitCode }).toEqual({ - stdout: JSON.stringify({ unchanged: true }), + stdout: JSON.stringify({ unchanged: true, printedOwnFrame: true, printedAppendFrame: false, sourceIntact: true }), stderr: "", exitCode: 0, }); @@ -1071,8 +1106,11 @@ test.concurrent( // .column, .sourceURL) discards the native frames. The errors are created // inside eval'd functions so that each iteration's frames point at code GC // can reclaim, which is what makes finalizeUnconditionally look at them. + // The unmaterialized sources (c) keeping their frames is the part release + // builds can observe. const src = ` const keep = []; + const sources = []; for (let i = 0; i < 200; i++) { eval(\`(function inner\${i}() { const a = new Error(); @@ -1085,12 +1123,13 @@ test.concurrent( const d = new Error(); d.sourceURL; Error.appendStackTrace(c, d); - keep.push(c, d); + keep.push(d); + sources.push(c); })();\`); } Bun.gc(true); Bun.gc(true); - process.stdout.write("ok"); + process.stdout.write(JSON.stringify({ sourcesIntact: sources.every(error => typeof error.stack === "string") })); `; await using proc = Bun.spawn({ cmd: [bunExe(), "-e", src], @@ -1098,7 +1137,11 @@ test.concurrent( stderr: "pipe", }); const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); - expect({ stdout, stderr, exitCode }).toEqual({ stdout: "ok", stderr: "", exitCode: 0 }); + expect({ stdout, stderr, exitCode }).toEqual({ + stdout: JSON.stringify({ sourcesIntact: true }), + stderr: "", + exitCode: 0, + }); }, ); From 3e632a3ab0fe3699470632b713cb9a31d915b322 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 13 Aug 2026 09:11:46 +0000 Subject: [PATCH 5/5] Error.appendStackTrace: condense the materialized guard comment --- src/jsc/bindings/FormatStackTraceForJS.cpp | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/jsc/bindings/FormatStackTraceForJS.cpp b/src/jsc/bindings/FormatStackTraceForJS.cpp index 5e1d08d4439c..62367b05a274 100644 --- a/src/jsc/bindings/FormatStackTraceForJS.cpp +++ b/src/jsc/bindings/FormatStackTraceForJS.cpp @@ -674,10 +674,7 @@ JSC_DEFINE_HOST_FUNCTION(errorConstructorFuncAppendStackTrace, (JSC::JSGlobalObj return JSC::JSValue::encode(jsUndefined()); } - // Once .stack is materialized the frames are gone. Installing new ones would make the native error - // printer (which prefers frames over .stack) show this call site plus the source's frames, and trip - // computeErrorInfo's !m_errorInfoMaterialized assertion when GC finalizes the error. Leave both - // errors alone instead, as Bun__attachAsyncStackFromPromise does. + // Materializing .stack dropped the frames; frames installed now would replace it in the native printer (same no-op as Bun__attachAsyncStackFromPromise). if (destination->hasMaterializedErrorInfo()) { return JSC::JSValue::encode(jsUndefined()); }