From e01f4546352fd2c3b6158a06600a4dddd91e1fb1 Mon Sep 17 00:00:00 2001 From: pfg Date: Wed, 5 Nov 2025 22:04:14 -0800 Subject: [PATCH] Fix #23865 (#24355) Fixes #23865, Fixes ENG-21446 Previously, a termination exception would be thrown. We didn't handle it properly and eventually it got caught by a `catch @panic()` handler. Now, no termination exception is thrown. ``` drainMicrotasksWithGlobal calls JSC__JSGlobalObject__drainMicrotasks JSC__JSGlobalObject__drainMicrotasks returns m_terminationException -> drainMicrotasksWithGlobal -> event_loop.zig:exit, which catches the error and discards it -> ... ``` For workers, we will need to handle termination exceptions in this codepath. ~~Previously, it would see the exception, call reportUncaughtExceptoinAtEventLoop, but the exception would still survive and return out from the catch scope. You're not supposed to still have an exception signaled at the exit of a catch scope. Exception checker may not have caught it because maybe the branch wasn't taken.~~ --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> --- src/bun.js/bindings/ZigGlobalObject.cpp | 5 ++++- src/bun.js/test/Execution.zig | 3 +-- test/regression/issue/23865.fixture.ts | 7 +++++++ test/regression/issue/23865.test.ts | 27 +++++++++++++++++++++++++ 4 files changed, 39 insertions(+), 3 deletions(-) create mode 100644 test/regression/issue/23865.fixture.ts create mode 100644 test/regression/issue/23865.test.ts diff --git a/src/bun.js/bindings/ZigGlobalObject.cpp b/src/bun.js/bindings/ZigGlobalObject.cpp index d53dce02e3..caf3c26aa2 100644 --- a/src/bun.js/bindings/ZigGlobalObject.cpp +++ b/src/bun.js/bindings/ZigGlobalObject.cpp @@ -2973,7 +2973,10 @@ void GlobalObject::handleRejectedPromises() continue; Bun__handleRejectedPromise(this, promise); - if (auto ex = scope.exception()) this->reportUncaughtExceptionAtEventLoop(this, ex); + if (auto ex = scope.exception()) { + scope.clearException(); + this->reportUncaughtExceptionAtEventLoop(this, ex); + } } } diff --git a/src/bun.js/test/Execution.zig b/src/bun.js/test/Execution.zig index 9f94729c11..be1d774f2d 100644 --- a/src/bun.js/test/Execution.zig +++ b/src/bun.js/test/Execution.zig @@ -177,7 +177,7 @@ pub fn handleTimeout(this: *Execution, globalThis: *jsc.JSGlobalObject) bun.JSEr defer groupLog.end(); // if the concurrent group has one sequence and the sequence has an active entry that has timed out, - // request a termination exception and kill any dangling processes + // kill any dangling processes // when using test.concurrent(), we can't do this because it could kill multiple tests at once. if (this.activeGroup()) |current_group| { const sequences = current_group.sequences(this); @@ -186,7 +186,6 @@ pub fn handleTimeout(this: *Execution, globalThis: *jsc.JSGlobalObject) bun.JSEr if (sequence.active_entry) |entry| { const now = bun.timespec.now(); if (entry.timespec.order(&now) == .lt) { - globalThis.requestTermination(); const kill_count = globalThis.bunVM().auto_killer.kill(); if (kill_count.processes > 0) { bun.Output.prettyErrorln("killed {d} dangling process{s}", .{ kill_count.processes, if (kill_count.processes != 1) "es" else "" }); diff --git a/test/regression/issue/23865.fixture.ts b/test/regression/issue/23865.fixture.ts new file mode 100644 index 0000000000..357910846d --- /dev/null +++ b/test/regression/issue/23865.fixture.ts @@ -0,0 +1,7 @@ +// Should not crash +test("abc", () => { + expect(async () => { + await Bun.sleep(100); + throw new Error("uh oh!"); + }).toThrow("uh oh!"); +}, 50); diff --git a/test/regression/issue/23865.test.ts b/test/regression/issue/23865.test.ts new file mode 100644 index 0000000000..b4f5dadea8 --- /dev/null +++ b/test/regression/issue/23865.test.ts @@ -0,0 +1,27 @@ +import { bunEnv, bunExe, normalizeBunSnapshot } from "harness"; + +// the test should time out, not crash +test("23865", async () => { + const proc = Bun.spawn({ + cmd: [bunExe(), "test", "./23865.fixture.ts"], + env: bunEnv, + cwd: import.meta.dir, + stdout: "pipe", + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + expect(exitCode).not.toBe(0); + expect(normalizeBunSnapshot(stdout)).toMatchInlineSnapshot(`"bun test ()"`); + expect(normalizeBunSnapshot(stderr)).toMatchInlineSnapshot(` + "23865.fixture.ts: + (fail) abc + ^ this test timed out after 50ms. + + 0 pass + 1 fail + 1 expect() calls + Ran 1 test across 1 file." + `); +});