ci: recover test-bun from Buildkite artifact-download failures

.buildkite/ci.mjs

@@ -298,25 +298,34 @@ function getImageName(platform, options) {
 }
 
 /**
- * @param {number} [limit]
+ * @param {{ artifactDownload?: boolean }} [options]
  * @link https://buildkite.com/docs/pipelines/command-step#retry-attributes
  */
-function getRetry() {
+function getRetry(options = {}) {
+  // Self-heal infra deaths once instead of leaving the build failed until a
+  // human notices and clicks retry:
+  //   -1  = agent lost / process killed (box died, agent restarted)
+  //   255 = step timeout kill (timeout_in_minutes SIGTERM cascade)
+  // User-canceled jobs are state=canceled, which never triggers automatic
+  // retry, so this cannot resurrect deliberately canceled builds. limit: 1
+  // caps the cost when a suite genuinely crashes with these statuses.
+  const automatic = [
+    { exit_status: -1, limit: 1 },
+    { exit_status: 255, limit: 1 },
+  ];
+  // Test steps download the build artifact before running any test;
+  // runner.node.mjs exits 4 when that download fails after its own retries
+  // (e.g. an agent with broken artifact-store connectivity). Reschedule the
+  // whole job so it can land on a healthy agent instead of going red on a
+  // lane that ran zero tests, which otherwise needs a manual retry click.
+  if (options.artifactDownload) {
+    automatic.push({ exit_status: 4, limit: 2 });
+  }
   return {
     manual: {
       permit_on_passed: true,
     },
-    // Self-heal infra deaths once instead of leaving the build failed until a
-    // human notices and clicks retry:
-    //   -1  = agent lost / process killed (box died, agent restarted)
-    //   255 = step timeout kill (timeout_in_minutes SIGTERM cascade)
-    // User-canceled jobs are state=canceled, which never triggers automatic
-    // retry, so this cannot resurrect deliberately canceled builds. limit: 1
-    // caps the cost when a suite genuinely crashes with these statuses.
-    automatic: [
-      { exit_status: -1, limit: 1 },
-      { exit_status: 255, limit: 1 },
-    ],
+    automatic,
   };
 }
 
@@ -831,7 +840,7 @@ function getTestBunStep(platform, options, testOptions = {}) {
     label: `${getPlatformLabel(platform)} - test-bun`,
     depends_on: depends,
     agents: getTestAgent(platform, options),
-    retry: getRetry(),
+    retry: getRetry({ artifactDownload: true }),
     cancel_on_build_failing: isMergeQueue(),
     parallelism: os === "darwin" ? 2 : os === "windows" ? 8 : 20,
     timeout_in_minutes: profile === "asan" || os === "windows" || os === "darwin" ? 45 : 30,

scripts/download-artifact.mjs

@@ -0,0 +1,86 @@
+// Downloads a build artifact from Buildkite with retries. Lives in its own
+// module (instead of inline in runner.node.mjs) so the retry policy can be
+// unit-tested without booting the whole test runner.
+
+import { mkdirSync, readdirSync, rmSync } from "node:fs";
+import { join } from "node:path";
+
+/** @typedef {(options: { command: string, args: string[], timeout: number }) => Promise<{ error?: string }>} SpawnFn */
+
+/**
+ * Download the bun build artifact for `target` into `releasePath` and return
+ * the path to the downloaded `bun*.zip`.
+ *
+ * Retries transient failures: a slow download killed at the per-attempt
+ * timeout, or the artifact not being uploaded yet (the download succeeds but
+ * no zip is present). It throws only after exhausting every attempt, so a
+ * single slow download no longer fails the job on the first try.
+ *
+ * Each attempt starts from an empty `releasePath`: a download killed at the
+ * timeout can leave a truncated zip behind, and a successful retry must never
+ * pick up that partial file (or a stale binary) instead of the real artifact.
+ *
+ * @param {object} params
+ * @param {string} params.target Buildkite step name to download from.
+ * @param {string} [params.buildId] Buildkite build id (defaults to the current build).
+ * @param {string} params.releasePath Directory to download into.
+ * @param {SpawnFn} params.spawn Spawns a command and resolves with `{ error }`.
+ * @param {number} [params.attempts] Max download attempts.
+ * @param {number} [params.timeout] Per-attempt timeout, in milliseconds.
+ * @param {(ms: number) => Promise<void>} [params.sleep] Backoff between attempts.
+ * @returns {Promise<string>}
+ */
+export async function downloadArtifactZip({
+  target,
+  buildId,
+  releasePath,
+  spawn,
+  attempts = 10,
+  timeout = 120_000,
+  sleep = ms => new Promise(resolve => setTimeout(resolve, ms)),
+}) {
+  let lastError;
+  for (let i = 0; i < attempts; i++) {
+    rmSync(releasePath, { recursive: true, force: true });
+    mkdirSync(releasePath, { recursive: true });
+
+    const args = ["artifact", "download", "**", releasePath, "--step", target];
+    if (buildId) {
+      args.push("--build", buildId);
+    }
+
+    const { error } = await spawn({ command: "buildkite-agent", args, timeout });
+    if (error) {
+      lastError = error;
+      console.warn(`buildkite-agent artifact download failed for step '${target}' (${error}), retrying...`);
+    } else {
+      // When both are present, prefer bun-profile: it keeps its symbol table
+      // (the release bun is stripped), so CI crash backtraces symbolicate.
+      const zipPath = readdirSync(releasePath, { recursive: true, encoding: "utf-8" })
+        .filter(filename => /^bun.*\.zip$/i.test(filename))
+        .map(filename => join(releasePath, filename))
+        .sort((a, b) => b.includes("profile") - a.includes("profile"))
+        .at(0);
+
+      if (zipPath) {
+        return zipPath;
+      }
+
+      console.warn(`Waiting for ${target}.zip to be available...`);
+    }
+
+    if (i < attempts - 1) {
+      await sleep((i + 1) * 1000);
+    }
+  }
+
+  // Tag the failure so the runner can exit with an infra status Buildkite
+  // auto-retries (getExitCode/main in runner.node.mjs, getRetry in ci.mjs)
+  // instead of a plain exit 1 that sits red until a human clicks Retry.
+  const error = new Error(
+    `Could not download ${target}.zip from Buildkite after ${attempts} attempts: ${releasePath}` +
+      (lastError ? ` (last download error: ${lastError})` : ""),
+  );
+  error.code = "ARTIFACT_DOWNLOAD_FAILED";
+  throw error;
+}

scripts/runner.node.mjs

@@ -34,6 +34,7 @@ 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";
+import { downloadArtifactZip } from "./download-artifact.mjs";
 import pLimit from "./p-limit.mjs";
 import {
   getAbi,
@@ -2250,44 +2251,7 @@ async function getExecPathFromBuildKite(target, buildId) {
   }
 
   const releasePath = join(cwd, "release");
-  mkdirSync(releasePath, { recursive: true });
-
-  let zipPath;
-  downloadLoop: for (let i = 0; i < 10; i++) {
-    const args = ["artifact", "download", "**", releasePath, "--step", target];
-    if (buildId) {
-      args.push("--build", buildId);
-    }
-
-    const { error } = await spawnSafe({
-      command: "buildkite-agent",
-      args,
-      timeout: 120000,
-    });
-    if (error === "timeout") {
-      throw new Error(
-        `buildkite-agent artifact download timed out after 120s for step '${target}'. ` +
-          `Refusing to continue with a partial download (would silently fall back to the wrong binary).`,
-      );
-    }
-
-    zipPath = readdirSync(releasePath, { recursive: true, encoding: "utf-8" })
-      .filter(filename => /^bun.*\.zip$/i.test(filename))
-      .map(filename => join(releasePath, filename))
-      .sort((a, b) => b.includes("profile") - a.includes("profile"))
-      .at(0);
-
-    if (zipPath) {
-      break downloadLoop;
-    }
-
-    console.warn(`Waiting for ${target}.zip to be available...`);
-    await new Promise(resolve => setTimeout(resolve, i * 1000));
-  }
-
-  if (!zipPath) {
-    throw new Error(`Could not find ${target}.zip from Buildkite: ${releasePath}`);
-  }
+  const zipPath = await downloadArtifactZip({ target, buildId, releasePath, spawn: spawnSafe });
 
   await unzip(zipPath, releasePath);
 
@@ -2576,7 +2540,7 @@ function isExecutable(execPath) {
 }
 
 /**
- * @param {"pass" | "fail" | "cancel"} [outcome]
+ * @param {"pass" | "fail" | "cancel" | "infra"} [outcome]
  */
 function getExitCode(outcome) {
   if (outcome === "pass") {
@@ -2593,6 +2557,12 @@ function getExitCode(outcome) {
   if (outcome === "cancel") {
     return 3;
   }
+  // Infra failure (e.g. the build artifact could not be downloaded). Distinct
+  // from a test failure so Buildkite can auto-retry it (see getRetry in
+  // .buildkite/ci.mjs) without also retrying genuine test failures (exit 2).
+  if (outcome === "infra") {
+    return 4;
+  }
   return 1;
 }
 
@@ -2978,4 +2948,10 @@ export async function main() {
   process.exit(getExitCode(ok ? "pass" : "fail"));
 }
 
-await main();
+await main().catch(error => {
+  console.error(error);
+  // The runner could not obtain the build artifact after retrying. Exit with an
+  // infra status Buildkite auto-retries so the job reschedules (ideally onto an
+  // agent with working artifact connectivity) instead of needing a manual retry.
+  process.exit(error?.code === "ARTIFACT_DOWNLOAD_FAILED" ? getExitCode("infra") : 1);
+});

test/internal/download-artifact.test.ts

@@ -0,0 +1,159 @@
+// Regression tests for the Buildkite artifact download retry policy
+// (scripts/download-artifact.mjs). The runner used to throw on the first
+// download timeout, failing the whole test job on a single slow download; the
+// retry loop only retried the "artifact not uploaded yet" case. See
+// https://github.com/oven-sh/bun/issues/33116
+
+import { describe, expect, test } from "bun:test";
+import { tempDir } from "harness";
+import { existsSync, writeFileSync } from "node:fs";
+import { join } from "node:path";
+
+import { downloadArtifactZip } from "../../scripts/download-artifact.mjs";
+
+type Behavior = { error?: string; writeZip?: string };
+
+/**
+ * Fake `buildkite-agent` spawn. Each call consumes the next behavior (the last
+ * one repeats): it optionally writes a zip into the download directory (which
+ * the real agent derives from args[3]) and resolves with `{ error }`.
+ */
+function fakeSpawn(behaviors: Behavior[]) {
+  const calls: { args: string[]; timeout: number }[] = [];
+  const spawn = async ({ args, timeout }: { command: string; args: string[]; timeout: number }) => {
+    const behavior = behaviors[Math.min(calls.length, behaviors.length - 1)];
+    calls.push({ args, timeout });
+    if (behavior.writeZip) {
+      const releasePath = args[3];
+      writeFileSync(join(releasePath, behavior.writeZip), "PK\u0003\u0004fake");
+    }
+    return { error: behavior.error };
+  };
+  return { spawn, calls };
+}
+
+const noSleep = async () => {};
+
+describe("downloadArtifactZip", () => {
+  test("retries a timed-out download and recovers", async () => {
+    using dir = tempDir("dl-artifact", {});
+    const releasePath = join(String(dir), "release");
+    const { spawn, calls } = fakeSpawn([{ error: "timeout" }, { writeZip: "bun-linux-x64.zip" }]);
+
+    const zipPath = await downloadArtifactZip({
+      target: "darwin-aarch64-build-bun",
+      releasePath,
+      spawn,
+      sleep: noSleep,
+    });
+
+    expect(zipPath).toBe(join(releasePath, "bun-linux-x64.zip"));
+    expect(calls.length).toBe(2);
+  });
+
+  test("retries while the artifact is still uploading", async () => {
+    using dir = tempDir("dl-artifact", {});
+    const releasePath = join(String(dir), "release");
+    // First download succeeds but the zip is not present yet.
+    const { spawn, calls } = fakeSpawn([{}, { writeZip: "bun-linux-x64.zip" }]);
+
+    const zipPath = await downloadArtifactZip({
+      target: "darwin-aarch64-build-bun",
+      releasePath,
+      spawn,
+      sleep: noSleep,
+    });
+
+    expect(zipPath).toBe(join(releasePath, "bun-linux-x64.zip"));
+    expect(calls.length).toBe(2);
+  });
+
+  test("prefers the profile build when several zips are present", async () => {
+    using dir = tempDir("dl-artifact", {});
+    const releasePath = join(String(dir), "release");
+    const spawn = async ({ args }: { command: string; args: string[]; timeout: number }) => {
+      const downloadPath = args[3];
+      writeFileSync(join(downloadPath, "bun-linux-x64.zip"), "PK\u0003\u0004fake");
+      writeFileSync(join(downloadPath, "bun-profile.zip"), "PK\u0003\u0004fake");
+      return {};
+    };
+
+    const zipPath = await downloadArtifactZip({
+      target: "darwin-aarch64-build-bun",
+      releasePath,
+      spawn,
+      sleep: noSleep,
+    });
+
+    expect(zipPath).toBe(join(releasePath, "bun-profile.zip"));
+  });
+
+  test("throws after exhausting every attempt and surfaces the last error", async () => {
+    using dir = tempDir("dl-artifact", {});
+    const releasePath = join(String(dir), "release");
+    const { spawn, calls } = fakeSpawn([{ error: "timeout" }]);
+
+    const error = await downloadArtifactZip({
+      target: "darwin-aarch64-build-bun",
+      releasePath,
+      spawn,
+      attempts: 3,
+      sleep: noSleep,
+    }).catch(e => e);
+
+    expect(error).toBeInstanceOf(Error);
+    expect(error.message).toMatch(/after 3 attempts.*last download error: timeout/s);
+    // Tagged so the runner exits an infra status Buildkite auto-retries.
+    expect(error.code).toBe("ARTIFACT_DOWNLOAD_FAILED");
+    expect(calls.length).toBe(3);
+  });
+
+  test("never reuses a partial zip left behind by a killed download", async () => {
+    using dir = tempDir("dl-artifact", {});
+    const releasePath = join(String(dir), "release");
+    // Attempt 1: the killed download leaves a truncated zip, then reports the
+    // timeout. Attempt 2: the download succeeds but produces no zip. The fix
+    // must start attempt 2 from a clean directory, so the stale zip is gone
+    // and we throw instead of handing back the partial artifact.
+    const { spawn, calls } = fakeSpawn([{ writeZip: "bun-stale.zip", error: "timeout" }, {}]);
+
+    await expect(
+      downloadArtifactZip({
+        target: "darwin-aarch64-build-bun",
+        releasePath,
+        spawn,
+        attempts: 2,
+        sleep: noSleep,
+      }),
+    ).rejects.toThrow(/after 2 attempts/);
+    expect(calls.length).toBe(2);
+    expect(existsSync(join(releasePath, "bun-stale.zip"))).toBe(false);
+  });
+
+  test("passes the build id through to buildkite-agent when provided", async () => {
+    using dir = tempDir("dl-artifact", {});
+    const releasePath = join(String(dir), "release");
+    const { spawn, calls } = fakeSpawn([{ writeZip: "bun-linux-x64.zip" }]);
+
+    await downloadArtifactZip({
+      target: "darwin-aarch64-build-bun",
+      buildId: "01234567-89ab-cdef-0123-456789abcdef",
+      releasePath,
+      spawn,
+      sleep: noSleep,
+    });
+
+    expect(calls[0].args).toContain("--build");
+    expect(calls[0].args).toContain("01234567-89ab-cdef-0123-456789abcdef");
+    expect(calls[0].args).toEqual([
+      "artifact",
+      "download",
+      "**",
+      releasePath,
+      "--step",
+      "darwin-aarch64-build-bun",
+      "--build",
+      "01234567-89ab-cdef-0123-456789abcdef",
+    ]);
+  });
+});