test runner: fail dependent tests when a hook's done() receives an error

docs/guides/util/base64.mdx

@@ -40,10 +40,11 @@ const text = bytes.toString("utf8");
 <Warning>
   The older [`btoa()`](https://developer.mozilla.org/en-US/docs/Web/API/Window/btoa) and [`atob()`](https://developer.mozilla.org/en-US/docs/Web/API/Window/atob) APIs are still available for compatibility, but they operate on binary strings instead of byte arrays. Avoid them in new code, especially when handling arbitrary binary data or non-ASCII text.
 
-  ```ts
-  const encoded = btoa("bun"); // => "YnVu"
-  const decoded = atob(encoded); // => "bun"
-  ```
+```ts
+const encoded = btoa("bun"); // => "YnVu"
+const decoded = atob(encoded); // => "bun"
+```
+
 </Warning>
 
 ---

src/runtime/test_runner/DoneCallback.rs

@@ -8,6 +8,9 @@ pub struct DoneCallback {
     /// Some = not called yet. None = done already called, no-op.
     pub r#ref: Option<RefDataPtr>,
     pub called: bool, // = false
+    /// The owning body threw before `run_test_callback` could attach `ref`,
+    /// so a later done(error) from this callback has no attributable owner.
+    pub orphaned: bool, // = false
 }
 
 impl DoneCallback {
@@ -32,6 +35,7 @@ impl DoneCallback {
         let done_callback = DoneCallback {
             r#ref: None,
             called: false,
+            orphaned: false,
         };
 
         // `JsClass::to_js` boxes `self` and hands the raw pointer to the JS

src/runtime/test_runner/bun_test.rs

@@ -815,31 +815,59 @@
         let [value] = callframe.arguments_as_array::<1>();
 
         let was_error = !value.is_empty_or_undefined_or_null();
+        // A second done() is a no-op, matching Bun 1.2.20.
+        // In Jest it is "Expected done to be called once, but it was called multiple times."
+        // Vitest does not support done callbacks.
         // SAFETY: `this` is the live `*mut DoneCallback` returned by `from_js`;
         // single-threaded JS VM, GC keeps the wrapper alive for the call frame.
-        if unsafe { (*this).called } {
-            // in Bun 1.2.20, this is a no-op
-            // in Jest, this is "Expected done to be called once, but it was called multiple times."
-            // Vitest does not support done callbacks
-        } else {
-            // error is only reported for the first done() call
-            if was_error {
-                let _ = global_this.bun_vm().as_mut().uncaught_exception(global_this, value, false);
-            }
-        }
+        let first_call = !unsafe { (*this).called };
         // SAFETY: see above — `this` is a live `*mut DoneCallback`.
         let ref_in = unsafe {
             (*this).called = true;
             (*this).r#ref.take()
         };
-        let Some(ref_in) = ref_in else {
-            return Ok(JSValue::UNDEFINED);
-        };
         // `this.ref` was already taken above.
         // RefPtr<T> currently has NO Drop impl, so decrement the
         // intrusive count explicitly at scope exit. Without this the
         // paired promise then/catch path never sees has_one_ref()==true and the RefData leaks.
-        let ref_in = scopeguard::guard(ref_in, |r: RefDataPtr| r.deref());
+        let ref_in = ref_in.map(|r| scopeguard::guard(r, |r: RefDataPtr| r.deref()));
+
+        // error is only reported for the first done() call
+        if first_call && was_error {
+            // `done(error)` is a failure of the entry that owns this callback,
+            // not a stray unhandled exception. Attribute it there (like the
+            // promise-catch path) so a failing hook fails its dependent tests.
+            let strong = match ref_in.as_ref() {
+                Some(r) => r.buntest_weak.upgrade(),
+                // The body threw before a ref could be attached; whenever and
+                // however this done() fires, it has no attributable owner.
+                // SAFETY: `this` is the live `*mut DoneCallback`; see above.
+                None if unsafe { (*this).orphaned } => None,
+                // Not orphaned and no ref: `run_test_callback` attaches the ref
+                // only after the callback returns, so done() must be running
+                // synchronously inside it and the active entry is the owner.
+                None => clone_active_strong(),
+            };
+            match strong {
+                Some(strong) => {
+                    let phase = match ref_in.as_ref() {
+                        Some(r) => r.phase.clone(),
+                        None => strong.get().get_current_state_data(),
+                    };
+                    // `strong.get()` is re-derived; the `get_current_state_data` borrow ended above.
+                    strong.get().on_uncaught_exception(global_this, Some(value), false, &phase);
+                }
+                None => {
+                    // Orphaned, or no live BunTest: report it generically, keeping
+                    // the hook-demotion guard in `jest::on_unhandled_rejection`.
+                    let _ = global_this.bun_vm().as_mut().uncaught_exception(global_this, value, false);
+                }
+            }
+        }
+
+        let Some(ref_in) = ref_in else {
+            return Ok(JSValue::UNDEFINED);
+        };
 
         // dupe the ref and enqueue a task to call the done callback.
         // this makes it so if you do something else after calling done(), the next test doesn't start running until the next tick.
@@ -1198,11 +1226,16 @@
         // second `RefDataPtr` here would over-count and the done-callback path
         // would never observe `has_one_ref()`.
         let mut dcb_ref: Option<NonNull<RefData>> = None;
-        if !done_callback.is_empty() && !result.is_empty() {
+        if !done_callback.is_empty() {
             if let Some(dcb_data) = DoneCallback::from_js(done_callback) {
                 // SAFETY: `dcb_data` is the live `*mut DoneCallback` from `from_js`;
                 // single-threaded JS VM, GC roots `done_callback` for this frame.
-                if unsafe { (*dcb_data).called } {
+                if result.is_empty() {
+                    // The body threw, so no ref is attached. A later done(error)
+                    // from this callback (the body may have handed it to a timer
+                    // or microtask before throwing) has no attributable owner.
+                    unsafe { (*dcb_data).orphaned = true };
+                } else if unsafe { (*dcb_data).called } {
                     // done callback already called or the callback errored; add result immediately
                 } else {
                     let r = Self::ref_(this_strong, cfg_data.clone());

test/js/bun/test/test-error-code-done-callback.test.ts

@@ -1,5 +1,5 @@
-import { expect, test } from "bun:test";
-import { bunEnv, bunExe } from "harness";
+import { describe, expect, test } from "bun:test";
+import { bunEnv, bunExe, tempDir } from "harness";
 import path from "path";
 
 test("verify we print error messages passed to done callbacks", () => {
@@ -80,7 +80,6 @@ test("verify we print error messages passed to done callbacks", () => {
     ^
     error: you should see this(async)
     at <anonymous> (<dir>/test-error-done-callback-fixture.ts:42:14)
-    at <anonymous> (<dir>/test-error-done-callback-fixture.ts:37:3)
     (fail) error done callback (async)
     43 |   });
     44 | });
@@ -111,7 +110,6 @@ test("verify we print error messages passed to done callbacks", () => {
     ^
     error: you should see this(async, nextTick)
     at <anonymous> (<dir>/test-error-done-callback-fixture.ts:60:14)
-    at <anonymous> (<dir>/test-error-done-callback-fixture.ts:54:5)
     (fail) error done callback (async, nextTick)
     62 | });
     63 |
@@ -140,3 +138,95 @@ test("verify we print error messages passed to done callbacks", () => {
     "
   `);
 });
+
+// A `done(error)` in a lifecycle hook must fail the hook's dependent tests,
+// exactly like a synchronous throw in the same hook does. `node:test` routes
+// every hook through the done-callback form.
+describe.concurrent("done(error) in a lifecycle hook", () => {
+  // One describe block containing 2 tests; expected counts match the
+  // synchronous-throw variant of each hook.
+  const expected = {
+    beforeAll: { pass: 0, fail: 1 },
+    beforeEach: { pass: 0, fail: 2 },
+    afterEach: { pass: 0, fail: 2 },
+    afterAll: { pass: 2, fail: 1 },
+  } as const;
+
+  test.each(Object.keys(expected) as (keyof typeof expected)[])(
+    "%s(done => done(err)) matches the synchronous-throw counts",
+    async hook => {
+      using dir = tempDir(`done-error-${hook}`, {
+        "hook.test.ts": `
+          import { describe, ${hook}, test } from "bun:test";
+          describe("suite", () => {
+            ${hook}(done => { done(new Error("hook failed")); });
+            test("t1", () => {});
+            test("t2", () => {});
+          });
+        `,
+      });
+      await using proc = Bun.spawn({
+        cmd: [bunExe(), "test", "./hook.test.ts"],
+        env: bunEnv,
+        cwd: String(dir),
+        stdout: "pipe",
+        stderr: "pipe",
+      });
+      const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+      const out = stdout + stderr;
+      // The hook's error is still reported, attributed to the failing entry.
+      expect(out).toContain("error: hook failed");
+      expect(summaryCounts(out)).toEqual(expected[hook]);
+      expect(exitCode).toBe(1);
+    },
+  );
+});
+
+// A done callback orphaned by its body throwing must stay an "Unhandled error
+// between tests" and never be blamed on whatever entry is active when it fires.
+// A macrotask fires on a later turn; a microtask drains inside the next entry.
+describe.concurrent("a late done(err) from a test whose body threw", () => {
+  test.each([
+    ["setImmediate", "setImmediate(fire)"],
+    ["a microtask", "Promise.resolve().then(fire)"],
+  ])("scheduled via %s does not fail an unrelated test", async (_name, schedule) => {
+    using dir = tempDir("orphaned-done", {
+      "orphan.test.ts": `
+        import { test, describe, beforeEach } from "bun:test";
+        const { promise: orphanFired, resolve: markOrphanFired } = Promise.withResolvers<void>();
+        test("a", done => {
+          const fire = () => { done(new Error("late orphan")); markOrphanFired(); };
+          ${schedule};
+          throw new Error("immediate");
+        });
+        describe("suite", () => {
+          // The orphan's done(err) lands while this hook is the active entry.
+          beforeEach(done => { orphanFired.then(() => done()); });
+          test("b still passes", () => {});
+        });
+      `,
+    });
+    await using proc = Bun.spawn({
+      cmd: [bunExe(), "test", "./orphan.test.ts"],
+      env: bunEnv,
+      cwd: String(dir),
+      stdout: "pipe",
+      stderr: "pipe",
+    });
+    const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+    const out = stdout + stderr;
+    expect(out).toContain("(pass) suite > b still passes");
+    expect(out).toContain("Unhandled error between tests");
+    expect(summaryCounts(out)).toEqual({ pass: 1, fail: 1, error: 1 });
+    expect(exitCode).toBe(1);
+  });
+});
+
+/** `" 2 pass\n 0 fail\n 1 error\n"` -> `{ pass: 2, fail: 0, error: 1 }` */
+function summaryCounts(out: string): Record<string, number> {
+  const counts: Record<string, number> = {};
+  for (const [, n, label] of out.matchAll(/^ (\d+) (pass|fail|skip|todo|error)s?$/gm)) {
+    counts[label] = Number(n);
+  }
+  return counts;
+}

test/js/node/test_runner/fixtures/06-failing-before-hook.js

@@ -0,0 +1,10 @@
+const { describe, before, test } = require("node:test");
+
+describe("db suite", () => {
+  before(() => {
+    throw new Error("DB connection failed");
+  });
+  test("reads row", () => {
+    // would touch the DB; must not be reported as a pass
+  });
+});

test/js/node/test_runner/node-test.test.ts

@@ -52,6 +52,15 @@ describe("node:test", () => {
       stderr: expect.stringContaining("0 fail"),
     });
   });
+
+  test("should not report a test as passing when its before() hook threw", async () => {
+    const { exitCode, stderr } = await runTests(["06-failing-before-hook.js"]);
+    // node fails every test under a suite whose before() hook failed.
+    expect(stderr).toContain("error: DB connection failed");
+    expect(stderr).toContain(" 0 pass\n 1 fail\n");
+    expect(stderr).not.toContain("Unhandled error between tests");
+    expect(exitCode).toBe(1);
+  });
 });
 
 async function runTests(filenames: string[]) {