Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 19 additions & 5 deletions src/jsc/bindings/ZigException.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -878,7 +884,15 @@ extern "C" void ZigException__collectSourceLines(JSC::EncodedJSValue jsException
auto* jscException = uncheckedDowncast<JSC::Exception>(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<JSC::ErrorInstance>(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);
}

Expand Down
2 changes: 1 addition & 1 deletion test/js/bun/test/dots.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ test("dots 2", async () => {
6 | setTimeout(() => {
7 | resolve();
8 | throw new Error("unhandled error");
^
^
error: unhandled error
at <anonymous> (file:NN:NN)
(fail) failure
Expand Down
2 changes: 1 addition & 1 deletion test/js/bun/test/only-failures.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 <anonymous> (file:NN:NN)
(fail) another failing test
Expand Down
28 changes: 28 additions & 0 deletions test/js/node/process/process.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Comment thread
coderabbitai[bot] marked this conversation as resolved.
expect(exitCode).toBe(7);
});
});

it("process.hasUncaughtExceptionCaptureCallback", () => {
Expand Down
2 changes: 1 addition & 1 deletion test/regression/issue/12782.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 <anonymous> (file:NN:NN)
(fail) (unnamed)
Expand Down
8 changes: 4 additions & 4 deletions test/regression/issue/19850/19850.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,17 +25,17 @@ err-in-hook-and-multiple-tests.ts:
2 |
3 | beforeEach(() => {
4 | throw new Error("beforeEach");
^
^
error: beforeEach
at <anonymous> (/err-in-hook-and-multiple-tests.ts:4:31)
at <anonymous> (/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 <anonymous> (/err-in-hook-and-multiple-tests.ts:4:31)
at <anonymous> (/err-in-hook-and-multiple-tests.ts:4:13)
(fail) test 1

0 pass
Expand Down
Loading