diff --git a/src/jsc/bindings/FormatStackTraceForJS.cpp b/src/jsc/bindings/FormatStackTraceForJS.cpp index ccbfc31bef99..62367b05a274 100644 --- a/src/jsc/bindings/FormatStackTraceForJS.cpp +++ b/src/jsc/bindings/FormatStackTraceForJS.cpp @@ -669,8 +669,23 @@ JSC_DEFINE_HOST_FUNCTION(errorConstructorFuncAppendStackTrace, (JSC::JSGlobalObj return {}; } + // appendVector() is not self-append safe, and the clear() below would then drop the trace. + if (source == destination) { + return JSC::JSValue::encode(jsUndefined()); + } + + // 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()); + } + if (!destination->stackTrace()) { - destination->captureStackTrace(vm, globalObject, 1); + // captureStackTrace() unwraps stackTraceLimit(), which is empty when Error.stackTraceLimit is not a number. + 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..b16fa3151251 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,6 +993,198 @@ test("captureStackTrace does not crash when stackTraceLimit is non-numeric", () } }); +test("Error.appendStackTrace moves the source's frames behind the destination's own", () => { + function inner() { + try { + null(); + } catch (e) { + return e; + } + } + function makeDestination() { + const error = new Error("destination"); + error.name = "DestinationError"; + return error; + } + const source = inner(); + const destination = makeDestination(); + Error.appendStackTrace(source, destination); + + 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 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 { + 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")); + + 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({ + 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 () => { + // 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 = ` + 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, + printedOwnFrame: printed.includes("at makeDestination"), + printedAppendFrame: printed.includes("at appendAll"), + sourceIntact: typeof lastSource.stack === "string", + })); + `; + 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, printedOwnFrame: true, printedAppendFrame: false, sourceIntact: true }), + stderr: "", + exitCode: 0, + }); +}); + +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. + // 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(); + 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(d); + sources.push(c); + })();\`); + } + Bun.gc(true); + Bun.gc(true); + process.stdout.write(JSON.stringify({ sourcesIntact: sources.every(error => typeof error.stack === "string") })); + `; + 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({ sourcesIntact: true }), + 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 = `