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>
This commit is contained in:
pfg
2025-11-05 22:04:14 -08:00
committed by GitHub
parent f56232a810
commit e01f454635
4 changed files with 39 additions and 3 deletions

View File

@@ -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);
}
}
}

View File

@@ -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("<d>killed {d} dangling process{s}<r>", .{ kill_count.processes, if (kill_count.processes != 1) "es" else "" });

View File

@@ -0,0 +1,7 @@
// Should not crash
test("abc", () => {
expect(async () => {
await Bun.sleep(100);
throw new Error("uh oh!");
}).toThrow("uh oh!");
}, 50);

View File

@@ -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 <version> (<revision>)"`);
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."
`);
});