diff --git a/src/jsc/bindings/ZigException.cpp b/src/jsc/bindings/ZigException.cpp index fe293087a1da..6e388ee55250 100644 --- a/src/jsc/bindings/ZigException.cpp +++ b/src/jsc/bindings/ZigException.cpp @@ -481,13 +481,19 @@ static void fromErrorInstance(ZigException& except, JSC::JSGlobalObject* global, auto& vm = JSC::getVM(global); auto scope = DECLARE_TOP_EXCEPTION_SCOPE(vm); + // Prefer the Error instance's own stack (captured at construction) over + // the outer JSC::Exception wrapper's stack. When a user rethrows an + // existing Error — e.g. inside `process.on('uncaughtException')` — JSC + // wraps the value in a fresh Exception whose stack points at the + // `throw err` site. The interesting data is on the Error itself, not on + // the rethrow wrapper. This matches Node's behavior of using `err.stack`. bool getFromSourceURL = false; - if (stackTrace != nullptr && stackTrace->size() > 0) { - populateStackTrace(vm, *stackTrace, except.stack, global, flags); - - } else if (err->stackTrace() != nullptr && err->stackTrace()->size() > 0) { + if (err->stackTrace() != nullptr && err->stackTrace()->size() > 0) { populateStackTrace(vm, *err->stackTrace(), except.stack, global, flags, FinalizerSafety::MustNotTriggerGC); + } else if (stackTrace != nullptr && stackTrace->size() > 0) { + populateStackTrace(vm, *stackTrace, except.stack, global, flags); + } else { getFromSourceURL = true; } @@ -878,7 +884,15 @@ extern "C" void ZigException__collectSourceLines(JSC::EncodedJSValue jsException auto* jscException = uncheckedDowncast(value); JSValue unwrapped = jscException->value(); - if (jscException->stack().size() > 0) { + // Must mirror the stack-source selection used by `fromErrorInstance` + // above: OnlySourceLines indexes into the same frame vector recorded + // during OnlyPosition (via ZigStackFrame::jsc_stack_frame_index). If + // the Error has its own stack, use it; otherwise fall back to the + // wrapper's stack. + if (auto* error = dynamicDowncast(unwrapped); + error && error->stackTrace() != nullptr && error->stackTrace()->size() > 0) { + populateStackTrace(global->vm(), *error->stackTrace(), exception->stack, global, PopulateStackTraceFlags::OnlySourceLines, FinalizerSafety::MustNotTriggerGC); + } else if (jscException->stack().size() > 0) { populateStackTrace(global->vm(), jscException->stack(), exception->stack, global, PopulateStackTraceFlags::OnlySourceLines); } diff --git a/test/js/bun/test/dots.test.ts b/test/js/bun/test/dots.test.ts index 3cb4027199f0..12a5d71b5d30 100644 --- a/test/js/bun/test/dots.test.ts +++ b/test/js/bun/test/dots.test.ts @@ -87,7 +87,7 @@ test("dots 2", async () => { 6 | setTimeout(() => { 7 | resolve(); 8 | throw new Error("unhandled error"); - ^ + ^ error: unhandled error at (file:NN:NN) (fail) failure diff --git a/test/js/bun/test/only-failures.test.ts b/test/js/bun/test/only-failures.test.ts index 25429f3288a1..f6fe527f544b 100644 --- a/test/js/bun/test/only-failures.test.ts +++ b/test/js/bun/test/only-failures.test.ts @@ -39,7 +39,7 @@ test.concurrent("only-failures flag should show only failures", async () => { 24 | 25 | test("another failing test", () => { 26 | throw new Error("This test fails"); - ^ + ^ error: This test fails at (file:NN:NN) (fail) another failing test diff --git a/test/js/node/process/process.test.js b/test/js/node/process/process.test.js index c95af9bc8eee..7a527ab89a73 100644 --- a/test/js/node/process/process.test.js +++ b/test/js/node/process/process.test.js @@ -821,6 +821,34 @@ describe.concurrent(() => { expect(await proc.exited).toBe(1); expect(await proc.stderr.text()).toContain("bar"); }); + + // https://github.com/oven-sh/bun/issues/30504 + it("preserves the original Error stack when the uncaughtException handler rethrows", async () => { + using dir = tempDir("rethrow-uncaught", { + "index.cjs": ` + 'use strict'; + process.on('uncaughtException', err => { + throw err; + }); + function throwUncaughtError() { + throw new Error('Boom'); + } + throwUncaughtError(); + `, + }); + await using proc = Bun.spawn({ + cmd: [bunExe(), join(String(dir), "index.cjs")], + env: bunEnv, + stderr: "pipe", + stdout: "pipe", + }); + const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + // Stack must reference the original throw site inside throwUncaughtError, + // not only the `throw err` rethrow site inside the handler. + expect(stderr).toContain("throwUncaughtError"); + expect(stderr).toContain("Boom"); + expect(exitCode).toBe(7); + }); }); it("process.hasUncaughtExceptionCaptureCallback", () => { diff --git a/test/regression/issue/12782.test.ts b/test/regression/issue/12782.test.ts index 9cf64acecf2e..29c2dca64abd 100644 --- a/test/regression/issue/12782.test.ts +++ b/test/regression/issue/12782.test.ts @@ -28,7 +28,7 @@ test("12782", async () => { 4 | 5 | beforeAll(() => { 6 | if (!FOO) throw new Error("Environment variable FOO is not set"); - ^ + ^ error: Environment variable FOO is not set at (file:NN:NN) (fail) (unnamed) diff --git a/test/regression/issue/19850/19850.test.ts b/test/regression/issue/19850/19850.test.ts index bd4a6b4e200d..c4486ce4a0a3 100644 --- a/test/regression/issue/19850/19850.test.ts +++ b/test/regression/issue/19850/19850.test.ts @@ -25,17 +25,17 @@ err-in-hook-and-multiple-tests.ts: 2 | 3 | beforeEach(() => { 4 | throw new Error("beforeEach"); - ^ + ^ error: beforeEach - at (/err-in-hook-and-multiple-tests.ts:4:31) + at (/err-in-hook-and-multiple-tests.ts:4:13) (fail) test 0 1 | import { beforeEach, test } from "bun:test"; 2 | 3 | beforeEach(() => { 4 | throw new Error("beforeEach"); - ^ + ^ error: beforeEach - at (/err-in-hook-and-multiple-tests.ts:4:31) + at (/err-in-hook-and-multiple-tests.ts:4:13) (fail) test 1 0 pass