bun test: keep the result of a callback that throws after its timeout

src/runtime/api/bun/js_bun_spawn_bindings.rs

@@ -293,7 +293,7 @@ pub(crate) fn spawn_sync(
         options.to_js(),
         &mut bun_test_deadline,
     );
-    // A bun:test deadline that passed while the isolated loop was blocking is reported only now: the loop is torn down and the child reaped, so the runner's callback re-enters nothing that is mid-flight. With an exception pending (spawn failure, termination) the file timer is left armed and reports it from the main loop instead.
+    // A bun:test deadline that passed while the isolated loop was blocking is reported only now: the loop is torn down and the child reaped, so the runner's callback re-enters nothing that is mid-flight. With an exception pending (spawn failure, termination) the file timer is left armed: the runner reports the deadline when the callback ends, or when that timer fires.
     if let Some(deadline) = bun_test_deadline
         && result.is_ok()
         && !cx.global().has_exception()
@@ -1912,10 +1912,10 @@ fn spawn_maybe_sync(
                             .active_file
                             .clone()
                         {
-                            active_file
-                                .get()
-                                .execution
-                                .kill_dangling_processes_on_timeout(cx.global());
+                            let execution = &mut active_file.get().execution;
+                            // `kill_dangling_processes_on_timeout` skips a concurrent group. The wait of this call ends below in any group.
+                            execution.killed_on_timeout_at = now;
+                            execution.kill_dangling_processes_on_timeout(cx.global());
                         }
                         let _ = subprocess.try_kill(subprocess.kill_signal);
                     }

src/runtime/test_runner/Collection.rs

@@ -6,7 +6,7 @@ use bun_jsc::{DeprecatedStrong, JSGlobalObject, JSValue, JsResult};
 use bun_core::Timespec;
 
 use crate::test_runner::bun_test::{
-    self, BunTest, BunTestPtr, DescribeScope, HandleUncaughtExceptionResult,
+    self, BunTest, BunTestPtr, CallbackEnd, DescribeScope, HandleUncaughtExceptionResult,
     RefDataValue, StepResult,
 };
 use crate::test_runner::bun_test::debug::group;
@@ -220,18 +220,20 @@ impl Collection {
                 bstr::BStr::new(this.active_scope().base.name.as_deref().unwrap_or(b"undefined")),
             ));
 
-            if let Some(cfg_data) = BunTest::run_test_callback(
+            let callback_data = RefDataValue::Collection { active_scope: previous_scope };
+            let end = BunTest::run_test_callback(
                 buntest_strong,
                 global_this,
                 callback.get(),
                 &[],
                 false,
-                RefDataValue::Collection { active_scope: previous_scope },
+                callback_data,
                 &Timespec::EPOCH,
-            ) {
+            );
+            if end != CallbackEnd::Running {
                 // the result is available immediately; queue
                 // Re-derive after re-entrant call per BunTestCell::get aliasing contract.
-                buntest_strong.get().add_result(cfg_data);
+                buntest_strong.get().add_result(callback_data);
             }
 
             return Ok(StepResult::Waiting { timeout: Timespec::EPOCH });

src/runtime/test_runner/DoneCallback.rs

@@ -10,6 +10,8 @@ pub(crate) struct DoneCallback {
     /// Some = not called yet. None = done already called, no-op.
     pub(crate) r#ref: JsCell<Option<RefPtr<RefData>>>,
     pub(crate) called: Cell<bool>, // = false
+    /// `done(error)` failed the running test after the deadline of that test.
+    pub(crate) failed_late: Cell<bool>,
     pub(crate) entry: RefDataValue,
 }
 
@@ -20,6 +22,7 @@ impl DoneCallback {
         let done_callback = DoneCallback {
             r#ref: JsCell::new(None),
             called: Cell::new(false),
+            failed_late: Cell::new(false),
             entry,
         };
 

src/runtime/test_runner/Execution.rs

@@ -45,7 +45,7 @@ use bun_core::scoped_log;
 
 use super::debug::group as group_log; // bun_test.debug.group
 use super::bun_test::{
-    group_begin, AddedInPhase, Around, AroundPhase, BunTest, BunTestPtr, Calling, EntryData, ExecutionEntry,
+    group_begin, AddedInPhase, Around, AroundPhase, BunTest, BunTestPtr, CallbackEnd, Calling, EntryData, ExecutionEntry,
     HandleUncaughtExceptionResult, Order, RefDataValue, ScopeMode, StepResult,
 };
 use super::expect::expect_deferred::ExpectDeferred;
@@ -97,6 +97,8 @@ pub(crate) struct Execution {
     /// The entry whose callback is on the stack, which `get_current_state_data()` can't name in a concurrent group.
     /// Cleared before the call's microtasks drain, because those can resume other tests in the group.
     pub(crate) on_stack: core::cell::Cell<Option<EntryData>>,
+    /// When bun last killed a process or ended a `Bun.spawnSync` wait for a timeout that had passed. `EPOCH` = never.
+    pub(crate) killed_on_timeout_at: Timespec,
 }
 
 pub(crate) struct ConcurrentGroup {
@@ -284,6 +286,16 @@ impl ExecutionSequence {
         false
     }
 
+    /// Whether `handle_uncaught_exception` prints the error of a hook and leaves `result`, so the test doesn't fail.
+    fn result_survives_hook_error(&self) -> bool {
+        match self.result {
+            Result::Pass | Result::Todo => true,
+            // `skip()` of a test context hides later errors on purpose.
+            Result::Skip => self.context.is_none(),
+            _ => false,
+        }
+    }
+
     /// For `test.failing`, `Result::Pass` means the callback threw, which is a failed attempt here.
     fn attempt_failed(&self) -> bool {
         match self.entry_mode() {
@@ -425,6 +437,7 @@ impl Execution {
             sequences: Box::default(),
             group_index: 0,
             on_stack: core::cell::Cell::new(None),
+            killed_on_timeout_at: Timespec::EPOCH,
         }
     }
 
@@ -457,6 +470,7 @@ impl Execution {
                         // SAFETY: bun_vm() returns the live per-thread VM.
                         let kill_count = global_this.bun_vm().as_mut().auto_killer.kill();
                         if kill_count.processes > 0 {
+                            self.killed_on_timeout_at = now;
                             bun_core::pretty_errorln!(
                                 "<d>killed {} dangling process{}<r>",
                                 kill_count.processes,
@@ -1330,22 +1344,25 @@ fn step_sequence_one(
         // The callback re-enters and reaches the sequence on its own, so no `&mut` is held across it.
         let sequence_ptr = NonNull::from(sequence);
         global_this.bun_vm().as_mut().is_running_vitest_entry = next_item.calling.is_vitest();
-        if BunTest::run_test_callback(
+        let end = BunTest::run_test_callback(
             buntest_strong,
             global_this,
             callback,
             args,
             next_item.calling == Calling::Jest { done: true },
             callback_data,
             &next_item.timespec,
-        )
-        .is_some()
-        {
+        );
+        if end != CallbackEnd::Running {
             *now = Timespec::now_force_real_time();
             // SAFETY: re-deref after run_test_callback; sequence_ptr still valid (sequences is a
             // Box<[ExecutionSequence]>, never reallocated during execution).
             let sequence = unsafe { &mut *sequence_ptr.as_ptr() };
-            if next_item.evaluate_timeout(sequence, now) {
+            // A failed callback keeps its result, unless bun killed a process for it or it is a hook whose error can't fail the test.
+            let checks_deadline = end == CallbackEnd::Returned
+                || buntest_strong.get().execution.killed_on_timeout_at.order(&next_item.timespec) != core::cmp::Ordering::Less
+                || (Some(next_item_ptr) != sequence.test_entry && sequence.result_survives_hook_error());
+            if checks_deadline && next_item.evaluate_timeout(sequence, now) {
                 if let Some(context) = context {
                     TestContext::abort_signal(context, global_this);
                 }

src/runtime/test_runner/bun_test.rs

@@ -986,24 +986,31 @@ impl BunTest {
         Self::bun_test_then_or_catch(cx.global(), value.to_js(), context, true)
     }
 
-    fn on_done_error(global_this: &JSGlobalObject, entry: &RefDataValue, error: JSValue) {
+    /// Whether the error failed the running test after the deadline of that test.
+    fn on_done_error(global_this: &JSGlobalObject, entry: &RefDataValue, error: JSValue) -> bool {
         if let Some(strong) = clone_active_strong() {
             let this = strong.get();
-            let is_running_test = this
+            // `Some` if `entry` is the running test, with whether its deadline has passed.
+            let running_test_is_late = this
                 .execution
                 .get_current_and_valid_execution_sequence(entry)
-                .is_some_and(|(sequence, _)| sequence.active_entry == sequence.test_entry);
-            if is_running_test {
+                .filter(|(sequence, _)| sequence.active_entry == sequence.test_entry)
+                .map(|(sequence, _)| {
+                    // SAFETY: arena-owned entry, alive for the lifetime of BunTest
+                    sequence.active_entry.is_some_and(|test| unsafe { test.as_ref() }.is_past_deadline(&Timespec::now_force_real_time()))
+                });
+            if let Some(is_late) = running_test_is_late {
                 this.on_uncaught_exception(global_this, Some(error), false, entry);
                 // As for any other uncaught exception: the test is over whatever its callback returns,
                 // and `on_before_exit()` doesn't drain the event loop of a file that failed.
                 this.add_result(*entry);
                 Self::run_next_tick(&Rc::downgrade(&strong), global_this, *entry);
                 global_this.bun_vm().as_mut().unhandled_error_counter += 1;
-                return;
+                return is_late;
             }
         }
         let _ = global_this.bun_vm().as_mut().uncaught_exception(global_this, error, false);
+        false
     }
 
     pub(crate) fn bun_test_done_callback(
@@ -1028,7 +1035,7 @@ impl BunTest {
         } else {
             // error is only reported for the first done() call
             if was_error {
-                Self::on_done_error(global_this, &this.entry, value);
+                this.failed_late.set(Self::on_done_error(global_this, &this.entry, value));
             }
         }
         this.called.set(true);
@@ -1330,7 +1337,6 @@ impl BunTest {
         }
     }
 
-    /// if sync, the result is returned. if async, None is returned.
     #[optimize(size)]
     pub(crate) fn run_test_callback(
         this_strong: &BunTestPtr,
@@ -1340,7 +1346,7 @@ impl BunTest {
         cfg_done_parameter: bool,
         cfg_data: RefDataValue,
         timeout: &Timespec,
-    ) -> Option<RefDataValue> {
+    ) -> CallbackEnd {
         let _g = group_begin!();
         // Raw `*mut` (via `UnsafeCell`) — the JS event-loop call below can
         // re-enter `bun_test_then_or_catch` / `bun_test_done_callback` /
@@ -1376,6 +1382,7 @@ impl BunTest {
 
         // SAFETY: `UnsafeCell`-derived; sole `&mut` at this point (before JS re-entry).
         unsafe { (*this).update_min_timeout(global_this, timeout) };
+        let mut end = CallbackEnd::Returned;
         let args_slice: &[JSValue] = if !done_arg.is_empty() { core::slice::from_ref(&done_arg) } else { cfg_args };
         // SAFETY: `this_strong` keeps the cell alive and in place for the whole call.
         let on_stack = unsafe { OnStack::enter(&raw const (*this).execution.on_stack, &cfg_data) };
@@ -1393,6 +1400,7 @@ impl BunTest {
         {
             Ok(v) => v,
             Err(_) => {
+                end = CallbackEnd::Failed;
                 global_this.clear_termination_exception();
                 // SAFETY: re-derive after JS callback returned; no outer `&mut` was held across it.
                 unsafe { (*this).on_uncaught_exception(global_this, global_this.try_take_exception(), false, &cfg_data) };
@@ -1436,6 +1444,9 @@ impl BunTest {
             if let Some(dcb_data) = done_callback.as_class_ref::<DoneCallback>() {
                 if dcb_data.called.get() {
                     // done callback already called or the callback errored; add result immediately
+                    if dcb_data.failed_late.get() {
+                        end = CallbackEnd::Failed;
+                    }
                 } else {
                     let r = Self::ref_(this_strong, cfg_data);
                     let alias = NonNull::new(r.as_ptr())
@@ -1483,14 +1494,14 @@ impl BunTest {
                             Bun__TestScope__Describe2__bunTestCatch,
                         );
                         // TODO: properly propagate exception upwards
-                        return None;
+                        return CallbackEnd::Running;
                     }
                     PromiseStatus::Fulfilled => {
                         // Do not register a then callback when it's already fulfilled.
                         let value = bun_jsc::JSPromise::opaque_mut(promise).result(global_this.vm());
                         // SAFETY: as in the `Rejected` arm.
                         unsafe { (*this).adopt_teardown(&cfg_data, value) };
-                        return Some(cfg_data);
+                        return end;
                     }
                     PromiseStatus::Rejected => {
                         let value = bun_jsc::JSPromise::opaque_mut(promise).result(global_this.vm());
@@ -1500,7 +1511,7 @@ impl BunTest {
 
                         // We previously marked it as handled above.
 
-                        return Some(cfg_data);
+                        return CallbackEnd::Failed;
                     }
                 }
             }
@@ -1509,15 +1520,15 @@ impl BunTest {
         if dcb_ref.is_some() {
             // completed asynchronously
             bun_core::scoped_log!(bun_test_group, "callTestCallback -> wait for done callback");
-            return None;
+            return CallbackEnd::Running;
         }
 
         bun_core::scoped_log!(bun_test_group, "callTestCallback -> sync");
         if !result.is_empty() {
             // SAFETY: re-derived after the JS callback returned; sole `&mut` at this point.
             unsafe { (*this).adopt_teardown(&cfg_data, result) };
         }
-        Some(cfg_data)
+        end
     }
 
     /// A promise that follows `returned` if it's a thenable other than a promise. Otherwise `returned`.
@@ -1920,6 +1931,16 @@ pub enum HandleUncaughtExceptionResult {
 pub(crate) type ResultQueue = LinearFifo<RefDataValue, bun_collections::linear_fifo::DynamicBuffer<RefDataValue>>;
 // bun.LinearFifo(.Dynamic) → second generic is the buffer strategy.
 
+/// How a callback that `run_test_callback` called ended.
+#[derive(Copy, Clone, PartialEq, Eq)]
+pub(crate) enum CallbackEnd {
+    /// Its promise or its `done` is still pending.
+    Running,
+    Returned,
+    /// It threw, its promise rejected, or its `done(error)` failed the test after the deadline of the test.
+    Failed,
+}
+
 pub(crate) enum StepResult {
     Waiting { timeout: Timespec },
     Complete,
@@ -2418,12 +2439,17 @@ impl ExecutionEntry {
         entry
     }
 
+    /// An entry without a timeout has no deadline.
+    pub(crate) fn is_past_deadline(&self, now: &Timespec) -> bool {
+        !self.timespec.eql(&Timespec::EPOCH) && self.timespec.order(now) == core::cmp::Ordering::Less
+    }
+
     pub(crate) fn evaluate_timeout(
         &self,
         sequence: &mut Execution::ExecutionSequence,
         now: &Timespec,
     ) -> bool {
-        if !self.timespec.eql(&Timespec::EPOCH) && self.timespec.order(now) == core::cmp::Ordering::Less {
+        if self.is_past_deadline(now) {
             // timed out
             // SAFETY: pointer-identity comparison only — no deref, no provenance laundering.
             let is_test_entry = sequence