node:test: support the (t, done) callback signature in tests and hooks

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/js/node/net.ts

@@ -2510,7 +2510,7 @@ function lookupAndConnect(self, options) {
     if (!self.connecting) return;
     if (err) {
       process.nextTick(destroyNT, self, err);
-    } else if (!isIP(ip)) {
+    } else if (typeof ip !== "string" || !isIP(ip)) {
       err = $ERR_INVALID_IP_ADDRESS(ip);
       process.nextTick(destroyNT, self, err);
     } else if (addressType !== 4 && addressType !== 6) {

src/js/node/test.ts

@@ -659,6 +659,73 @@ function parseTestOptions(arg0: unknown, arg1: unknown, arg2: unknown) {
   return { name, options: options as TestOptions, fn };
 }
 
+// Runs a user test or hook function and completes via `finish`, a single-shot
+// finalizer (safe to call more than once). A function declaring exactly two
+// parameters (`(context, done)`) uses Node's error-first callback style: it
+// receives a `done` callback and only completes once `done` is called (a
+// truthy argument fails, a falsy one passes). Like Node, a synchronous throw
+// or a returned Promise takes precedence over a `done` call made before the
+// function returned, and a second `done` call throws. Any other arity
+// completes synchronously or when its returned Promise settles.
+function runWithDone(fn: TestFn | HookFn, context: TestContext, finish: DoneCallback) {
+  if (fn.length === 2) {
+    let returned = false;
+    let doneCalls = 0;
+    let donePending = false;
+    let doneFailure: unknown;
+    const done = (error?: unknown) => {
+      doneCalls += 1;
+      if (doneCalls > 1) {
+        if (doneCalls === 2) {
+          throw new Error("callback invoked multiple times");
+        }
+        return;
+      }
+      const failure = error ? error : undefined;
+      if (returned) {
+        finish(failure);
+      } else {
+        donePending = true;
+        doneFailure = failure;
+      }
+    };
+
+    let result: unknown;
+    try {
+      result = fn(context, done);
+    } catch (error) {
+      finish(error);
+      return;
+    }
+    returned = true;
+    if (result instanceof Promise) {
+      // Node reports this misuse right away; the promise's own outcome no
+      // longer decides the test, so don't leave its rejection unhandled.
+      (result as Promise<unknown>).then(kDefaultFunction, kDefaultFunction);
+      finish(new Error("passed a callback but also returned a Promise"));
+    } else if (donePending) {
+      finish(doneFailure);
+    }
+    return;
+  }
+
+  let result: unknown;
+  try {
+    result = fn(context);
+  } catch (error) {
+    finish(error);
+    return;
+  }
+  if (result instanceof Promise) {
+    (result as Promise<unknown>).then(
+      () => finish(),
+      error => finish(error),
+    );
+  } else {
+    finish();
+  }
+}
+
 function createTest(arg0: unknown, arg1: unknown, arg2: unknown) {
   const { name, options, fn } = parseTestOptions(arg0, arg1, arg2);
 
@@ -668,26 +735,18 @@ function createTest(arg0: unknown, arg1: unknown, arg2: unknown) {
   const runTest = (done: (error?: unknown) => void) => {
     const originalContext = ctx;
     ctx = context;
+    let finished = false;
     const endTest = (error?: unknown) => {
+      if (finished) return;
+      finished = true;
       try {
         done(error);
       } finally {
         ctx = originalContext;
       }
     };
 
-    let result: unknown;
-    try {
-      result = fn(context);
-    } catch (error) {
-      endTest(error);
-      return;
-    }
-    if (result instanceof Promise) {
-      (result as Promise<unknown>).then(() => endTest()).catch(error => endTest(error));
-    } else {
-      endTest();
-    }
+    runWithDone(fn, context, endTest);
   };
 
   return { name, options, fn: runTest };
@@ -739,25 +798,23 @@ function createHook(arg0: unknown, arg1: unknown) {
   const { fn, options } = parseHookOptions(arg0, arg1);
 
   const runHook = (done: (error?: unknown) => void) => {
-    let result: unknown;
-    try {
-      result = fn();
-    } catch (error) {
+    let finished = false;
+    const endHook = (error?: unknown) => {
+      if (finished) return;
+      finished = true;
       done(error);
-      return;
-    }
-    if (result instanceof Promise) {
-      (result as Promise<unknown>).then(() => done()).catch(error => done(error));
-    } else {
-      done();
-    }
+    };
+
+    const context = new TestContext(false, undefined, Bun.main, ctx);
+    runWithDone(fn, context, endHook);
   };
 
   return { options, fn: runHook };
 }
 
-type TestFn = (ctx: TestContext) => unknown | Promise<unknown>;
-type HookFn = () => unknown | Promise<unknown>;
+type DoneCallback = (error?: unknown) => void;
+type TestFn = (ctx: TestContext, done?: DoneCallback) => unknown | Promise<unknown>;
+type HookFn = (ctx: TestContext, done?: DoneCallback) => unknown | Promise<unknown>;
 
 type TestOptions = {
   concurrency?: number | boolean | null;

test/expectations.txt

@@ -12,6 +12,14 @@
 # the spawned child needs process.binding('inspector'), which is not implemented
 test/js/node/test/parallel/test-inspector-enabled.js [ FAIL ]
 
+# Unmasked by the node:test (t, done) callback fix (PR #28502): this file uses the two-argument
+# test(function(_, cb){...}) signature, so before the fix its subtests never ran and it passed
+# vacuously. It spawns test-http-max-http-headers.js (removed in 8c2b7b65a1 as non-passing) and
+# asserts the child exits 0 when --max-http-header-size equals the header size; bun's HTTP server
+# instead emits clientError (HPE_HEADER_OVERFLOW) on request headers sized exactly at the limit,
+# where node accepts them. Pre-existing header-size boundary bug in the HTTP layer, out of scope.
+test/js/node/test/parallel/test-set-http-max-http-headers.js [ FAIL ]
+
 # Verbatim node v26.3.0 test asserting a FinalizationRegistry callback fires
 # within ONE globalThis.gc() + ONE setImmediate after the connect callback's
 # closure is unreferenced. The FinalizationRegistry spec gives no timing

test/js/node/net/net-connect-lookup.test.ts

@@ -0,0 +1,54 @@
+import { expect, it } from "bun:test";
+import { connect } from "node:net";
+
+it("emits ERR_INVALID_IP_ADDRESS when a custom lookup yields a non-string address", async () => {
+  // Node validates `typeof ip === "string"` before isIP(); an array like
+  // ["127.0.0.1"] stringifies into a valid IP, so it must not connect.
+  for (const family of [4, 6] as const) {
+    const { promise, resolve, reject } = Promise.withResolvers<Error & { code?: string }>();
+    const socket = connect({
+      host: "example.com",
+      port: 80,
+      family,
+      lookup: (_host, _options, callback) => {
+        callback(null, ["127.0.0.1"] as unknown as string, family);
+      },
+    });
+    socket.on("connect", () => reject(new Error("connected with an invalid lookup result")));
+    socket.on("error", resolve);
+    try {
+      const error = await promise;
+      expect(error.code).toBe("ERR_INVALID_IP_ADDRESS");
+    } finally {
+      socket.destroy();
+    }
+  }
+});
+
+it("passes a string address from a custom lookup through to the connection", async () => {
+  // A loopback server observes the connection even though the hostname never
+  // resolves, proving the lookup result is what gets connected to.
+  using server = Bun.listen({
+    hostname: "127.0.0.1",
+    port: 0,
+    socket: {
+      data() {},
+    },
+  });
+  const { promise, resolve, reject } = Promise.withResolvers<void>();
+  const socket = connect({
+    host: "definitely-not-resolvable.example.invalid",
+    port: server.port,
+    family: 4,
+    lookup: (_host, _options, callback) => {
+      callback(null, "127.0.0.1", 4);
+    },
+  });
+  socket.on("connect", () => resolve());
+  socket.on("error", reject);
+  try {
+    await promise;
+  } finally {
+    socket.destroy();
+  }
+});