runner: start the CI crash remap server in the background and wait for it at the first test

scripts/runner.node.mjs

@@ -30,7 +30,6 @@ import {
 import { readFile } from "node:fs/promises";
 import { availableParallelism, userInfo } from "node:os";
 import { basename, dirname, extname, join, relative, sep } from "node:path";
-import { createInterface } from "node:readline";
 import { setTimeout as setTimeoutPromise } from "node:timers/promises";
 import { parseArgs } from "node:util";
 import { prestartMap as dockerPrestartMap } from "../test/docker/prestart-map.mjs";
@@ -65,6 +64,7 @@ import {
   markBuildkiteStepReported,
   printEnvironment,
   reportAnnotationToBuildKite,
+  spawnBackgroundServer,
   startGroup,
   tmpdir,
   unzip,
@@ -86,6 +86,12 @@ const spawnTimeout = 5_000;
 const spawnBunTimeout = 20_000; // when running with ASAN/LSAN bun can take a bit longer to exit, not a bug.
 const testTimeout = 3 * 60_000;
 const integrationTimeout = 5 * 60_000;
+// How long the first test waits for the crash report remap server. It counts from
+// the moment the server is needed, not from its start: the server is started
+// before the root and test/ installs and is normally listening once they are done.
+// A fresh CI agent takes 3.5s to well over 5s to start it (a 5s budget counted
+// from its start lost the server on 3 of 4 shards), so keep this far above that.
+const ciRemapServerTimeout = 30_000;
 
 const resolutionGatingFlags = new Set([
   "--expose-internals",
@@ -546,6 +552,11 @@ async function runTests() {
   const tests = getRelevantTests(testsPath, modifiers, expectations);
   !isQuiet && console.log("Running tests:", tests.length);
 
+  // The crash report remap server installs and starts in the background while
+  // the rest of the setup below runs. Its port is awaited right before the first
+  // test, the first thing that needs it.
+  const ciRemapServer = isCI && !isWindows ? startCiRemapServer(execPath) : undefined;
+
   // Start the docker-service coordinator (test/docker/coordinator.ts). It
   // owns every `docker compose` invocation for this shard — `compose up` is
   // not concurrency-safe, so exactly one process runs it — and prestarts the
@@ -793,44 +804,7 @@ async function runTests() {
   };
 
   if (!failedResults.length) {
-    // TODO: remove windows exclusion here
-    if (isCI && !isWindows && (await installCiRemapServer(execPath))) {
-      const { promise: portPromise, resolve: portResolve } = Promise.withResolvers();
-      const { promise: errorPromise, resolve: errorResolve } = Promise.withResolvers();
-      let exiting = false;
-
-      const server = spawn(execPath, ["run", "--silent", "ci-remap-server", execPath, cwd, getCommit()], {
-        stdio: ["ignore", "pipe", "inherit"],
-        cwd: ciRemapServerPath,
-        env: { ...process.env, BUN_DEBUG_QUIET_LOGS: "1", NO_COLOR: "1" },
-      });
-      server.unref();
-      server.on("error", errorResolve);
-      server.on("exit", (code, signal) => {
-        if (!exiting && (code !== 0 || signal !== null)) errorResolve(signal ? signal : "code " + code);
-      });
-      function onBeforeExit() {
-        exiting = true;
-        server.off("error");
-        server.off("exit");
-        server.kill?.();
-      }
-      process.once("beforeExit", onBeforeExit);
-      const lines = createInterface(server.stdout);
-      lines.on("line", line => {
-        portResolve({ port: parseInt(line) });
-      });
-
-      const result = await Promise.race([portPromise, errorPromise.catch(e => e), setTimeoutPromise(5000, "timeout")]);
-      if (typeof result?.port != "number") {
-        process.off("beforeExit", onBeforeExit);
-        server.kill?.();
-        console.warn("ci-remap server did not start:", result);
-      } else {
-        console.log("crash reports parsed on port", result.port);
-        remapPort = result.port;
-      }
-    }
+    if (ciRemapServer) remapPort = await ciRemapServer.port();
 
     const runOneTest = async (testPath, concurrent) => {
       await awaitNapiPrebuild(testPath);
@@ -2248,20 +2222,66 @@ async function spawnBunInstall(execPath, options) {
 }
 
 /**
- * Installs `ci-remap-server`, the bin of bun-tracestrings (github:oven-sh/bun.report).
- * It is pinned in scripts/ci-remap-server/package.json rather than the root
- * package.json because this runner is its only user, and as a github: dependency
- * it would otherwise put GitHub on the critical path of the root `bun install`
- * every GitHub Actions workflow and every build runs. Best-effort, like starting
- * the server itself: without it crash reports are not remapped, the tests still run.
+ * Installs and starts `ci-remap-server`, the bin of bun-tracestrings
+ * (github:oven-sh/bun.report), which remaps the crash reports of the tests
+ * (spawnBun points them at it with BUN_CRASH_REPORT_URL). It is pinned in
+ * scripts/ci-remap-server/package.json rather than the root package.json because
+ * this runner is its only user, and as a github: dependency it would otherwise put
+ * GitHub on the critical path of the root `bun install` every GitHub Actions
+ * workflow and every build runs.
+ *
+ * Nothing is awaited here. The install and the server's startup (it loads octokit
+ * and opens a sqlite database) take seconds each on a fresh CI agent, so they run
+ * while the runner does the rest of its setup, and `port()` is called right before
+ * the first test. `port()` prints the install output and the outcome as one group.
+ * Best-effort throughout: without the server crash reports are not remapped, the
+ * tests still run.
  * @param {string} execPath
- * @returns {Promise<boolean>}
+ * @returns {{ port: () => Promise<number | undefined> }}
  */
-async function installCiRemapServer(execPath) {
+function startCiRemapServer(execPath) {
   const title = relative(cwd, join(ciRemapServerPath, "package.json")).replaceAll(sep, "/");
-  const { ok, error } = await startGroup(title, () => spawnBunInstall(execPath, { cwd: ciRemapServerPath }));
-  if (!ok) console.warn(`ci-remap server not installed (${title}: ${error}), crash reports will not be remapped`);
-  return ok;
+  const startedAt = Date.now();
+  let installOutput = "";
+  /** @type {Promise<import("./utils.mjs").BackgroundServer | { error: string }>} */
+  const server = (async () => {
+    const { ok, error } = await spawnBunInstall(execPath, {
+      cwd: ciRemapServerPath,
+      stdout: chunk => (installOutput += chunk),
+      stderr: chunk => (installOutput += chunk),
+    });
+    if (!ok) return { error: `not installed (${error})` };
+    return spawnBackgroundServer([execPath, "run", "--silent", "ci-remap-server", execPath, cwd, getCommit()], {
+      cwd: ciRemapServerPath,
+      env: {
+        ...process.env,
+        BUN_DEBUG_QUIET_LOGS: "1",
+        NO_COLOR: "1",
+        // The server is killed when this process exits. This also takes it down
+        // when this process is killed instead, which runs no exit handlers.
+        BUN_FEATURE_FLAG_NO_ORPHANS: "1",
+      },
+    });
+  })().catch(error => ({ error: String(error) }));
+
+  return {
+    async port() {
+      const neededAt = Date.now();
+      const seconds = since => `${((Date.now() - since) / 1000).toFixed(1)}s`;
+      const started = await server;
+      startGroup(title);
+      process.stdout.write(installOutput);
+      const result = "port" in started ? await started.port(ciRemapServerTimeout) : started;
+      if ("error" in result) {
+        console.warn(
+          `ci-remap server did not start: ${result.error}, crash reports will not be remapped (${seconds(startedAt)} after it was started)`,
+        );
+        return;
+      }
+      console.log(`crash reports parsed on port ${result.port} (waited ${seconds(neededAt)} for it)`);
+      return result.port;
+    },
+  };
 }
 
 /**

scripts/utils.mjs

@@ -18,6 +18,7 @@ import { connect } from "node:net";
 import { hostname, homedir as nodeHomedir, tmpdir as nodeTmpdir, release, userInfo } from "node:os";
 import { basename, dirname, join, relative, resolve } from "node:path";
 import { normalize as normalizeWindows } from "node:path/win32";
+import { createInterface } from "node:readline";
 
 export const isWindows = process.platform === "win32";
 export const isMacOS = process.platform === "darwin";
@@ -439,6 +440,78 @@ export function spawnSyncSafe(command, options = {}) {
   return spawnSync(command, { throwOnError: true, ...options });
 }
 
+/**
+ * @typedef {object} BackgroundServer
+ * @property {import("node:child_process").ChildProcess} subprocess
+ * @property {(timeout: number) => Promise<{ port: number } | { error: string }>} port
+ */
+
+/**
+ * Starts a server without waiting for it. The server prints the port it listens
+ * on as its first line of stdout (the ci-remap server does), then keeps running
+ * until this process exits, which kills it. Start it as early as possible and call
+ * `port()` right before the server is needed, so that its startup overlaps with
+ * the work in between.
+ *
+ * `port(timeout)` resolves with the port, also when it was printed before the
+ * call. It resolves with an error as soon as the server fails to spawn or ends
+ * without printing a line, whatever its exit code, when its first line is not a
+ * port, or when `timeout` ms pass after the call. In the last two cases the server
+ * is killed as well. It never rejects. The server does not keep this process
+ * alive, and it inherits this process's stderr.
+ * @param {string[]} command
+ * @param {{ cwd?: string, env?: Record<string, string | undefined> }} [options]
+ * @returns {BackgroundServer}
+ */
+export function spawnBackgroundServer(command, options = {}) {
+  const [cmd, ...args] = command;
+  debugLog("$", cmd, ...args);
+
+  const { promise: firstLine, resolve: settle } = Promise.withResolvers();
+  const subprocess = nodeSpawn(cmd, args, {
+    cwd: options["cwd"],
+    env: options["env"],
+    stdio: ["ignore", "pipe", "inherit"],
+  });
+  // kill() sends SIGTERM, on purpose. `bun run <bin>` stays alive as the parent of
+  // the script it runs and forwards SIGTERM to it. SIGKILL would end the wrapper
+  // alone and leave the script running, holding this process's stderr open.
+  const kill = () => subprocess.kill();
+  process.once("exit", kill);
+  subprocess.on("error", error => settle({ error: error.message }));
+  // "close" and not "exit": a server that prints its line and exits at once can
+  // emit "exit" before its stdout was read. "close" comes after the end of stdout,
+  // and readline emits the line before that.
+  subprocess.on("close", (exitCode, signalCode) => settle({ error: signalCode ?? `code ${exitCode}` }));
+  createInterface(subprocess.stdout).once("line", line => settle({ line }));
+  subprocess.unref();
+  subprocess.stdout.unref?.();
+
+  return {
+    subprocess,
+    async port(timeout) {
+      const timer = setTimeout(() => {
+        settle({ error: "timeout" });
+        kill();
+      }, timeout);
+      let result;
+      try {
+        result = await firstLine;
+      } finally {
+        clearTimeout(timer);
+      }
+      if ("error" in result) {
+        return result;
+      }
+      if (!/^\d+$/.test(result.line)) {
+        kill();
+        return { error: `printed ${JSON.stringify(result.line)} instead of a port` };
+      }
+      return { port: parseInt(result.line) };
+    },
+  };
+}
+
 /**
  * @param {number} exitCode
  * @returns {string | undefined}