From 80f4e8fdf4f2018bc2d2e88ebe114aa3171e9b12 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 16 Sep 2026 20:16:04 +0000 Subject: [PATCH 1/3] scripts: make curl() report only the last attempt curl() in scripts/utils.mjs kept status, statusText, body and error outside its retry loop and never cleared them between attempts. A retry that succeeded still returned the error of the failed attempt, so every caller treated the request as failed and curlSafe() threw. Clear the four values at the start of each attempt. Add a retryDelay option so the test does not wait through the backoff. --- scripts/utils.mjs | 12 ++- test/internal/utils-curl-retry.test.ts | 125 +++++++++++++++++++++++++ 2 files changed, 134 insertions(+), 3 deletions(-) create mode 100644 test/internal/utils-curl-retry.test.ts diff --git a/scripts/utils.mjs b/scripts/utils.mjs index 666e2961f166..9afc4506d137 100755 --- a/scripts/utils.mjs +++ b/scripts/utils.mjs @@ -777,6 +777,8 @@ export function getGithubToken() { * @property {number} [timeout] * @property {boolean} [cache] * @property {number} [retries] + * @property {number} [retryDelay] Milliseconds. The wait before attempt N (N >= 2) is N times this. + * Default: 1000. Tests pass 0. * @property {boolean} [json] * @property {boolean} [arrayBuffer] * @property {string} [filename] @@ -784,8 +786,8 @@ export function getGithubToken() { /** * @typedef {object} CurlResult - * @property {number} status - * @property {string} statusText + * @property {number | undefined} status undefined when the last attempt got no response + * @property {string | undefined} statusText * @property {Error | undefined} error * @property {any} body */ @@ -804,6 +806,7 @@ export async function curl(url, options = {}) { let input = options["body"]; let headers = options["headers"] || {}; let retries = options["retries"] || 3; + let retryDelay = options["retryDelay"] ?? 1000; let json = options["json"]; let arrayBuffer = options["arrayBuffer"]; let filename = options["filename"]; @@ -832,9 +835,12 @@ export async function curl(url, options = {}) { let error; for (let i = 0; i < retries; i++) { if (i > 0) { - await new Promise(resolve => setTimeout(resolve, 1000 * (i + 1))); + await new Promise(resolve => setTimeout(resolve, retryDelay * (i + 1))); } + // The result describes the last attempt only. + status = statusText = body = error = undefined; + let response; try { response = await fetch(href, { method, headers, body: input }); diff --git a/test/internal/utils-curl-retry.test.ts b/test/internal/utils-curl-retry.test.ts new file mode 100644 index 000000000000..3609665738eb --- /dev/null +++ b/test/internal/utils-curl-retry.test.ts @@ -0,0 +1,125 @@ +/** + * curl() in scripts/utils.mjs is how the CI scripts read Buildkite, GitHub and the + * cloud metadata services. It retries a request that fails. Every caller reads + * `error` first and gives up when it is set, so a request that a retry saved has + * to return the same result as one that worked the first time. + * + * The fake server below scripts one outcome per request. `retryDelay: 0` removes + * the wait between attempts. + */ +import { describe, expect, test } from "bun:test"; +import { tempDir } from "harness"; +import { once } from "node:events"; +import { readFileSync } from "node:fs"; +import { createServer, type AddressInfo } from "node:net"; +import { join } from "node:path"; +import { curl, curlSafe } from "../../scripts/utils.mjs"; + +/** "drop" closes the connection without an answer. Anything else is a status line and a body. */ +type Outcome = "drop" | [status: string, body: string]; + +const ok: Outcome = ["200 OK", JSON.stringify({ ok: true })]; +const busy: Outcome = ["503 Service Unavailable", "busy"]; + +/** + * Answers request N with outcome N, and drops every request after the script + * runs out. Every response is Connection: close, so one attempt is one request. + */ +async function fakeServer(script: Outcome[]) { + let requests = 0; + const server = createServer(socket => { + socket.once("data", () => { + const outcome = script[requests++] ?? "drop"; + if (outcome === "drop") { + socket.destroy(); + return; + } + const [status, body] = outcome; + socket.end( + `HTTP/1.1 ${status}\r\nContent-Length: ${Buffer.byteLength(body)}\r\nConnection: close\r\n\r\n${body}`, + ); + }); + }); + server.listen(0, "127.0.0.1"); + await once(server, "listening"); + const { port } = server.address() as AddressInfo; + return { + url: `http://127.0.0.1:${port}/build.json`, + get requests() { + return requests; + }, + [Symbol.asyncDispose]: () => new Promise(resolve => server.close(() => resolve())), + }; +} + +describe("curl", () => { + const transientFailures: [string, Outcome][] = [ + ["answers 503", busy], + ["drops the connection", "drop"], + ["sends a body that does not parse", ["200 OK", ""]], + ]; + + test.each(transientFailures)( + "a retry that succeeds reports no error when the first attempt %s", + async (_, failure) => { + await using server = await fakeServer([failure, ok]); + + const result = await curl(server.url, { json: true, retryDelay: 0 }); + + expect(result).toEqual({ status: 200, statusText: "OK", error: undefined, body: { ok: true } }); + expect(server.requests).toBe(2); + }, + ); + + test("the cached result of a retry that succeeds has no error", async () => { + await using server = await fakeServer([busy, ok]); + const options = { json: true, cache: true, retryDelay: 0 }; + + await curl(server.url, options); + const cached = await curl(server.url, options); + + expect(cached).toEqual({ status: 200, statusText: "OK", error: undefined, body: { ok: true } }); + expect(server.requests).toBe(2); + }); + + test("a download that needed a retry writes the file and returns no body", async () => { + using dir = tempDir("utils-curl-retry", {}); + await using server = await fakeServer([busy, ["200 OK", "artifact bytes"]]); + const filename = join(String(dir), "artifact.bin"); + + const result = await curl(server.url, { filename, retryDelay: 0 }); + + expect(readFileSync(filename, "utf8")).toBe("artifact bytes"); + expect(result).toEqual({ status: 200, statusText: "OK", error: undefined, body: undefined }); + expect(server.requests).toBe(2); + }); + + test("a failure on the last attempt is reported with nothing from the attempt before it", async () => { + await using server = await fakeServer([busy, "drop"]); + + const result = await curl(server.url, { retries: 2, retryDelay: 0 }); + + expect(result.error?.message).toBe(`Fetch failed: GET ${server.url}`); + expect(result).toEqual({ status: undefined, statusText: undefined, error: expect.any(Error), body: undefined }); + expect(server.requests).toBe(2); + }); + + test("a request that fails on every attempt reports the last failure", async () => { + await using server = await fakeServer([busy, busy, busy]); + + const result = await curl(server.url, { retryDelay: 0 }); + + expect(result.error?.message).toBe(`Fetch failed: GET ${server.url}: 503 Service Unavailable`); + expect(result).toEqual({ status: 503, statusText: "Service Unavailable", error: expect.any(Error), body: "busy" }); + expect(server.requests).toBe(3); + }); +}); + +describe("curlSafe", () => { + test("returns the body of a retry that succeeds", async () => { + await using server = await fakeServer([busy, ok]); + + expect(await curlSafe(server.url, { json: true, retryDelay: 0 })).toEqual({ ok: true }); + expect(server.requests).toBe(2); + }); +}); From 32b1b60cdd41875b6e0992e16466d7b249ae9866 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 17 Sep 2026 00:46:34 +0000 Subject: [PATCH 2/3] test: drop the filename case from utils-curl-retry No caller of curl() passes `filename`, so the case covered a path that nothing uses. The file now has 7 tests. Without the reset line in curl(), 6 of them fail. The seventh checks that a request that fails on every attempt still reports the failure. --- test/internal/utils-curl-retry.test.ts | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/test/internal/utils-curl-retry.test.ts b/test/internal/utils-curl-retry.test.ts index 3609665738eb..f8f45f0b44a1 100644 --- a/test/internal/utils-curl-retry.test.ts +++ b/test/internal/utils-curl-retry.test.ts @@ -8,11 +8,8 @@ * the wait between attempts. */ import { describe, expect, test } from "bun:test"; -import { tempDir } from "harness"; import { once } from "node:events"; -import { readFileSync } from "node:fs"; import { createServer, type AddressInfo } from "node:net"; -import { join } from "node:path"; import { curl, curlSafe } from "../../scripts/utils.mjs"; /** "drop" closes the connection without an answer. Anything else is a status line and a body. */ @@ -82,18 +79,6 @@ describe("curl", () => { expect(server.requests).toBe(2); }); - test("a download that needed a retry writes the file and returns no body", async () => { - using dir = tempDir("utils-curl-retry", {}); - await using server = await fakeServer([busy, ["200 OK", "artifact bytes"]]); - const filename = join(String(dir), "artifact.bin"); - - const result = await curl(server.url, { filename, retryDelay: 0 }); - - expect(readFileSync(filename, "utf8")).toBe("artifact bytes"); - expect(result).toEqual({ status: 200, statusText: "OK", error: undefined, body: undefined }); - expect(server.requests).toBe(2); - }); - test("a failure on the last attempt is reported with nothing from the attempt before it", async () => { await using server = await fakeServer([busy, "drop"]); From 0d7fdfaa491005552659fbb7e3ff2eba600bf662 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 17 Sep 2026 01:02:16 +0000 Subject: [PATCH 3/3] ci: retrigger