event loop: report immediates' rejections before polling, and stop polling once a fatal error ended the run

src/jsc/Debugger.rs

@@ -191,7 +191,7 @@ impl Debugger {
     ///
     /// Aliasing: `this.debugger` is read through a raw pointer
     /// with fresh short-lived borrows because `event_loop().tick()` /
-    /// `auto_tick_active()` re-enter JS, which calls `VirtualMachine::get()`
+    /// `auto_tick()` re-enter JS, which calls `VirtualMachine::get()`
     /// and may form independent `&mut VirtualMachine` borrows. Holding a
     /// long-lived `&mut Debugger` (which borrows from `&mut VirtualMachine`)
     /// across those calls is UB.
@@ -318,7 +318,11 @@ impl Debugger {
             };
             match wait {
                 Wait::Forever => {
-                    this.event_loop_mut().auto_tick_active();
+                    // A condition wait, like `wait_for_promise`, not a run loop:
+                    // `auto_tick_active` stops polling once a counted error has
+                    // ended the run (see its contract), and under `bun test`,
+                    // where such errors end nothing, that would spin here.
+                    this.event_loop_mut().auto_tick();
 
                     if bun_core::Environment::ENABLE_LOGS {
                         bun_core::scoped_log!(

src/jsc/event_loop.rs

@@ -397,10 +397,23 @@ impl EventLoop {
         self.drain_microtasks_with_global(global, jsc_vm)
     }
 
+    /// Whether the callbacks this loop runs from the current frame get their
+    /// microtask checkpoint here (`exit()` drains at depth 1). False while the
+    /// loop is being ticked from inside JS (`wait_for_promise` under a
+    /// callback), from a deferred task, or with draining suppressed (spawnSync):
+    /// there the enclosing frame's `tick()` drains, and reports rejections,
+    /// afterwards, and a promise whose `.catch()` is still queued must not be
+    /// reported before it.
+    fn checkpoints_here(&self) -> bool {
+        let vm = self.vm_ref();
+        self.entered_event_loop_count == 0
+            && !vm.is_inside_deferred_task_queue.get()
+            && !vm.suppress_microtask_drain.get()
+    }
+
     // should be called after exit()
     pub fn maybe_drain_microtasks(&mut self) -> Result<(), Stopped> {
-        if self.entered_event_loop_count == 0 && !self.vm_ref().is_inside_deferred_task_queue.get()
-        {
+        if self.checkpoints_here() {
             return self.drain_microtasks();
         }
         Ok(())
@@ -880,8 +893,9 @@ impl EventLoop {
     }
 
     /// `tickImmediateTasks` — swaps the two
-    /// immediate queues, drains the now-current batch, then recycles the
-    /// drained Vec as the next-tick buffer.
+    /// immediate queues, drains the now-current batch, reports the promise
+    /// rejections it left unhandled (as `tick()` does for its tasks), then
+    /// recycles the drained Vec as the next-tick buffer.
     ///
     /// Note: the real `ImmediateObject` lives in `bun_runtime` (cycle), so
     /// the per-task body dispatches through `__bun_run_immediate_task` (link-
@@ -913,19 +927,36 @@ impl EventLoop {
 
         let mut exception_thrown = false;
         for task in to_run_now.iter() {
+            // `|=`: a callback that threw skipped its own checkpoint
+            // (`run_immediate_task`), which the batch tail below owes it even
+            // when a later task returns `false` (cleared, or unref'd and skipped).
             // SAFETY: ImmediateObject pointers are kept alive by the JS heap
             // until `__bun_run_immediate_task` consumes them; `virtual_machine` is the
             // live owning VM per caller contract.
-            exception_thrown = unsafe { __bun_run_immediate_task(*task, virtual_machine) };
+            exception_thrown |= unsafe { __bun_run_immediate_task(*task, virtual_machine) };
         }
         // Re-escape `this` after the re-entrant loop so nothing about `*this`
         // is carried across it.
         let this: *mut Self = core::hint::black_box(this);
 
-        // make sure microtasks are drained if the last task had an exception
-        if exception_thrown {
+        // SAFETY: as above.
+        if !to_run_now.is_empty() && unsafe { (*this).checkpoints_here() } {
             // SAFETY: as above.
-            let _ = unsafe { (*this).maybe_drain_microtasks() };
+            let stopped = exception_thrown && unsafe { (*this).drain_microtasks() }.is_err();
+            // Every callback of the batch has had its checkpoint, so what they
+            // left rejected is unhandled: report it before the caller polls.
+            // The poll parks until the next timer or I/O event, which can be far
+            // off or never come, and the places that otherwise report these
+            // (the caller's next `tick()`, `auto_tick`'s tail) come after it.
+            // Not with an exception pending (as in `tick()`: by now that is the
+            // termination, or one that escaped its fold), which the handlers
+            // would run on top of.
+            // SAFETY: as above; `global_ref()` is `'static`, so no borrow of
+            // `*this` is live while the handlers re-enter this loop.
+            let global = unsafe { (*this).global_ref() };
+            if !stopped && !global.has_exception() {
+                global.handle_rejected_promises();
+            }
         }
 
         // SAFETY: as above; this read MUST observe pushes JS made during the

src/runtime/jsc_hooks.rs

@@ -1121,6 +1121,16 @@ unsafe fn auto_tick(vm: *mut VirtualMachine) {
 /// `on_before_exit` drain loops where blocking when the loop is idle would
 /// hang shutdown.
 ///
+/// Contract: the callers are loops of the form `while is_event_loop_alive()
+/// { tick(); auto_tick_active(); }` (`Run::start`, `on_before_exit`,
+/// `WebWorker::spin`, the REPL, the debugger thread's own loop), so once a
+/// counted error has made that condition false this returns without polling.
+/// A wait for some other condition must use [`auto_tick`], as
+/// `wait_for_promise` and the debugger's attach wait do, or it would spin
+/// instead of parking while the counter is nonzero. (`AnyEventLoop::tick_once`
+/// also calls this, for a single pump between batches of install work; one
+/// skipped poll there changes nothing.)
+///
 /// # Safety
 /// `vm` is the live per-thread VM.
 unsafe fn auto_tick_active(vm: *mut VirtualMachine) {
@@ -1132,7 +1142,18 @@ unsafe fn auto_tick_active(vm: *mut VirtualMachine) {
 
     // SAFETY: `el` is the live per-thread event loop; `vm` per fn contract.
     unsafe { (*el).tick_immediate_tasks(vm) };
-    // SAFETY: as above.
+    // An error nothing handled (raised by those immediates, or reported by the
+    // caller's `tick()` just before this) ends the run: the caller's loop
+    // condition is now false (see the contract above; the counter fails it
+    // unless more immediates are queued) and it runs nothing further. Parking
+    // in the poll below would only delay that exit until some unrelated wakeup,
+    // or forever, and run whatever the wakeup brings first. (With immediates
+    // queued the loop comes back for them, and the poll does not block anyway.)
+    // SAFETY: per fn contract.
+    if unsafe { &*vm }.unhandled_error_counter > 0 && !unsafe { &*vm }.is_event_loop_alive() {
+        return;
+    }
+    // SAFETY: `el` is the live per-thread event loop.
     let has_yielded_tasks = unsafe { (*el).promote_yield_tasks() };
     #[cfg(windows)]
     if has_yielded_tasks || !unsafe { &*el }.immediate_tasks.is_empty() {

test/js/bun/test/test-test.test.ts

@@ -749,3 +749,41 @@ test("my-test", () => {
     });
   }
 });
+
+test("a rejection left by a setImmediate callback is reported before the test's due timer fires", async () => {
+  // The timer is due by the time the immediate returns (the immediate blocks
+  // past its deadline), so it fires in the runner's next poll of the event
+  // loop. The rejection must be reported when the immediate returns, not after
+  // that poll: with a test waiting on something slower than a due timer, the
+  // report used to wait along with it.
+  using dir = tempDir("unhandled-immediate", {
+    "my-test.test.js": /* js */ `
+      import { test } from "bun:test";
+      test("my-test", async () => {
+        const { promise, resolve } = Promise.withResolvers();
+        // Concatenated so the marker only appears in stderr when the timer
+        // fires, not in the source excerpt printed with the error.
+        setTimeout(() => { console.error("## timer " + "fired ##"); resolve(); }, 0);
+        setImmediate(() => { Promise.reject(new Error("## rejected in immediate ##")); Bun.sleepSync(5); });
+        await promise;
+      });
+    `,
+    "package.json": "{}",
+  });
+
+  await using proc = spawn({
+    cmd: [bunExe(), "test", "my-test.test.js"],
+    cwd: String(dir),
+    stdout: "ignore",
+    stderr: "pipe",
+    env: bunEnv,
+  });
+  const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]);
+
+  const reportedAt = stderr.indexOf("error: ## rejected in immediate ##");
+  const timerAt = stderr.indexOf("## timer fired ##");
+  expect(reportedAt).toBeGreaterThan(-1);
+  expect(timerAt).toBeGreaterThan(reportedAt);
+  expect(stderr).toContain("1 fail");
+  expect(exitCode).toBe(1);
+});

test/js/node/inspector/inspector.test.ts

@@ -1,5 +1,6 @@
 import { expect, test } from "bun:test";
-import { bunEnv, bunExe, tempDir } from "harness";
+import { bunEnv, bunExe, isLinux, tempDir } from "harness";
+import { readFileSync } from "node:fs";
 import inspector from "node:inspector";
 
 test("inspector.url()", () => {
@@ -551,6 +552,97 @@ test("inspector.waitForDebugger() blocks again on the second call after a fronte
   expect(exitCode).toBe(0);
 });
 
+// Under `bun test` an error nothing handled is tallied, not fatal, so a later
+// waitForDebugger() runs with the VM's unhandled-error count nonzero. The wait
+// must still park the thread: the run loops' turn of the event loop
+// (auto_tick_active) stops polling once that count has ended a run, so a wait
+// ticking with it would spin here instead. Measured from outside, on the
+// fixture's main thread, before any client connects, so that nothing but the
+// wait itself is in the window. /proc is what makes the per-thread reading
+// possible, hence Linux only.
+const waitForDebuggerAfterUnhandledErrorFixture = `
+import { test } from "bun:test";
+import inspector from "node:inspector";
+
+test("leaves an error nothing handles", () => {
+  Promise.reject(new Error("tallied by the runner"));
+});
+
+test("then waits for a debugger", () => {
+  inspector.open(0, "127.0.0.1", false);
+  process.stderr.write("WAITING_FOR_DEBUGGER\\n");
+  inspector.waitForDebugger();
+  inspector.close();
+});
+`;
+
+test.skipIf(!isLinux)(
+  "inspector.waitForDebugger() parks the thread after an earlier unhandled error under bun test",
+  async () => {
+    using dir = tempDir("inspector-wait-after-error", {
+      "wait.test.ts": waitForDebuggerAfterUnhandledErrorFixture,
+    });
+
+    await using proc = Bun.spawn({
+      cmd: [bunExe(), "test", "wait.test.ts"],
+      env: bunEnv,
+      cwd: String(dir),
+      stdout: "ignore",
+      stderr: "pipe",
+    });
+
+    const decoder = new TextDecoder();
+    const reader = proc.stderr.getReader();
+    let stderrText = "";
+    while (!stderrText.includes("WAITING_FOR_DEBUGGER")) {
+      const { value, done } = await reader.read();
+      if (done) throw new Error(`stderr closed before the fixture started waiting; got: ${stderrText}`);
+      stderrText += decoder.decode(value);
+    }
+    expect(stderrText).toContain("tallied by the runner");
+    const wsUrl = stderrText.match(/Debugger listening on (ws:\S+)/)?.[1];
+    expect(wsUrl).toBeDefined();
+
+    // utime + stime of the main thread, in clock ticks (100 per second), from
+    // /proc/<pid>/task/<pid>/stat; the fields follow the parenthesized name.
+    const mainThreadTicks = () => {
+      const stat = readFileSync(`/proc/${proc.pid}/task/${proc.pid}/stat`, "utf8");
+      const fields = stat.slice(stat.lastIndexOf(")") + 2).split(" ");
+      return Number(fields[11]) + Number(fields[12]);
+    };
+    const windowMs = 500;
+    const windowTicks = windowMs / 10;
+    const ticksBefore = mainThreadTicks();
+    // The measurement window: the fixture is waiting for a client the whole
+    // time, and the question is what that costs it.
+    await Bun.sleep(windowMs);
+    const ticksDuringWait = mainThreadTicks() - ticksBefore;
+
+    const ws = new WebSocket(wsUrl!);
+    const opened = Promise.withResolvers<void>();
+    ws.onopen = () => opened.resolve();
+    ws.onerror = error => opened.reject(error);
+    await opened.promise;
+    ws.send(JSON.stringify({ id: 1, method: "Runtime.runIfWaitingForDebugger", params: {} }));
+    const drained = (async () => {
+      for (;;) {
+        const { value, done } = await reader.read();
+        if (done) break;
+        stderrText += decoder.decode(value);
+      }
+    })();
+    const exitCode = await proc.exited;
+    await drained;
+    ws.close();
+
+    // Parked: the thread runs nothing in the window (0 ticks, 1 if the idle GC
+    // timer fires). Spinning: about the whole window.
+    expect(ticksDuringWait).toBeLessThan(windowTicks / 4);
+    // The tallied error fails the file; that it was tallied is the case under test.
+    expect(exitCode).toBe(1);
+  },
+);
+
 test("Runtime.consoleAPICalled is emitted while the Runtime domain is enabled", () => {
   const session = new inspector.Session();
   session.connect();

test/js/node/process/process.test.js

@@ -1516,6 +1516,101 @@ describe.concurrent(() => {
     expect({ stdout: stdout.trim(), stderr, exitCode }).toEqual({ stdout: "ok", stderr: "", exitCode: 0 });
   });
 
+  // Every case arms a 0ms timer and then blocks past its deadline inside the
+  // callback under test, so by the time the callback returns the timer is due:
+  // it fires in the very next poll of the event loop, if there is one. Its
+  // output therefore shows whether the loop went on to poll after the callback
+  // instead of acting on the error first. (A due timer is the wakeup here; in
+  // real programs the poll waits for whatever the program is waiting on, which
+  // used to delay these errors by up to the idle GC timer, or forever.)
+  describe.concurrent("errors nothing handled are acted on before the event loop polls again", () => {
+    const runPastTheTimer = `Bun.sleepSync(5);`;
+
+    async function run(script) {
+      await using proc = Bun.spawn({
+        cmd: [bunExe(), "-e", script],
+        env: bunEnv,
+        stdout: "pipe",
+        stderr: "pipe",
+      });
+      const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+      return { stdout: stdout.split("\n").filter(Boolean), stderr, exitCode };
+    }
+
+    it("a rejection left by a setImmediate callback is emitted before the due timer fires", async () => {
+      const result = await run(`
+        process.on("unhandledRejection", e => console.log("unhandledRejection", e.message));
+        setTimeout(() => console.log("timer"), 0);
+        setImmediate(() => { Promise.reject(new Error("x")); ${runPastTheTimer} });
+      `);
+      expect(result).toEqual({ stdout: ["unhandledRejection x", "timer"], stderr: "", exitCode: 0 });
+    });
+
+    it("a fatal rejection left by a setImmediate callback ends the run; the due timer never fires", async () => {
+      const { stdout, stderr, exitCode } = await run(`
+        setTimeout(() => console.log("timer"), 0);
+        setImmediate(() => { Promise.reject(new Error("boom")); ${runPastTheTimer} });
+      `);
+      expect(stdout).toEqual([]);
+      expect(stderr).toInclude("error: boom");
+      expect(exitCode).toBe(1);
+    });
+
+    it("a fatal throw from a setImmediate callback ends the run; the due timer never fires", async () => {
+      const { stdout, stderr, exitCode } = await run(`
+        setTimeout(() => console.log("timer"), 0);
+        setImmediate(() => { ${runPastTheTimer} throw new Error("boom"); });
+      `);
+      expect(stdout).toEqual([]);
+      expect(stderr).toInclude("error: boom");
+      expect(exitCode).toBe(1);
+    });
+
+    it("a fatal rejection left by a task callback ends the run; the timer it made due never fires", async () => {
+      // An fs callback runs from the task queue, so here the rejection is
+      // reported by the tick that ran the callback (that part always worked),
+      // and what is under test is that the loop then exits instead of polling
+      // once more. Started from a timer so that the callback runs from the run
+      // loop proper, not from the tick that follows the entry point.
+      const { stdout, stderr, exitCode } = await run(`
+        setTimeout(() => require("fs").stat(".", () => {
+          setTimeout(() => console.log("timer"), 0);
+          Promise.reject(new Error("boom"));
+          ${runPastTheTimer}
+        }), 0);
+      `);
+      expect(stdout).toEqual([]);
+      expect(stderr).toInclude("error: boom");
+      expect(exitCode).toBe(1);
+    });
+
+    it.each([
+      ["returns", ""],
+      // A callback that throws gets its microtask checkpoint at the end of the
+      // immediate batch instead of at its own exit; a sibling that was cleared
+      // in the meantime must not make the batch forget that.
+      ["throws, after clearing the immediate queued behind it", `clearImmediate(sibling); throw new Error("boom");`],
+    ])("a rejection handled by a microtask of the setImmediate callback that %s is not reported", async (_, tail) => {
+      const result = await run(`
+        process.on("uncaughtException", e => console.log("uncaughtException", e.message));
+        process.on("unhandledRejection", e => console.log("unhandledRejection", e.message));
+        setTimeout(() => console.log("timer"), 0);
+        setImmediate(() => {
+          const p = Promise.reject(new Error("x"));
+          queueMicrotask(() => p.catch(() => {}));
+          ${runPastTheTimer}
+          ${tail}
+        });
+        const sibling = setImmediate(() => {});
+      `);
+      expect(result).toEqual({
+        stdout: tail ? ["uncaughtException boom", "timer"] : ["timer"],
+        stderr: "",
+        exitCode: 0,
+      });
+    });
+  });
+
   it("aborts when the uncaughtException handler throws", async () => {
     const proc = Bun.spawn([bunExe(), join(import.meta.dir, "process-onUncaughtExceptionAbort.js")], {
       stderr: "pipe",