fix(api-client): fix retry failing and add a timeout of 30s

packages/api-client/package.json

@@ -40,16 +40,18 @@
     "build": "tsdown",
     "check-types": "tsc",
     "check-format": "prettier --check --ignore-unknown --ignore-path=../../.gitignore --ignore-path=../../.prettierignore .",
-    "lint": "eslint ."
+    "lint": "eslint .",
+    "test": "vitest"
   },
   "devDependencies": {
     "@types/debug": "^4.1.13",
     "@types/node": "catalog:",
     "openapi-typescript": "^7.13.0",
-    "p-retry": "^8.0.0"
+    "vitest": "catalog:"
   },
   "dependencies": {
     "debug": "^4.4.3",
-    "openapi-fetch": "^0.17.0"
+    "openapi-fetch": "^0.17.0",
+    "p-retry": "^8.0.0"
   }
 }

packages/api-client/src/fetch.test.ts

@@ -0,0 +1,160 @@
+import { afterEach, describe, expect, it, vi } from "vitest";
+import { APIError, apiFetch } from "./fetch";
+
+describe("apiFetch", () => {
+  afterEach(() => {
+    vi.useRealTimers();
+    vi.restoreAllMocks();
+  });
+
+  it("retries server errors with a fresh request and replays the body", async () => {
+    const cloneSpy = vi
+      .spyOn(Request.prototype, "clone")
+      .mockImplementation(() => {
+        throw new TypeError("unusable");
+      });
+    const bodies: string[] = [];
+    const fetchMock = vi.fn(async (input: RequestInfo | URL) => {
+      const request = input instanceof Request ? input : new Request(input);
+      bodies.push(await request.text());
+
+      return new Response("{}", {
+        status: bodies.length === 1 ? 500 : 200,
+      });
+    });
+    const body = JSON.stringify({ commit: "abc123" });
+    const request = new Request("https://api.argos-ci.test/builds", {
+      body,
+      headers: {
+        "content-type": "application/json",
+      },
+      method: "POST",
+    });
+
+    const response = await apiFetch(request, {
+      fetch: fetchMock as unknown as typeof fetch,
+      minTimeout: 0,
+    });
+
+    expect(response.status).toBe(200);
+    expect(fetchMock).toHaveBeenCalledTimes(2);
+    expect(bodies).toEqual([body, body]);
+    expect(cloneSpy).not.toHaveBeenCalled();
+  });
+
+  it("does not retry client errors", async () => {
+    const fetchMock = vi.fn(async () => new Response("{}", { status: 400 }));
+
+    const response = await apiFetch(
+      new Request("https://api.argos-ci.test/builds"),
+      {
+        fetch: fetchMock as unknown as typeof fetch,
+        minTimeout: 0,
+      },
+    );
+
+    expect(response.status).toBe(400);
+    expect(fetchMock).toHaveBeenCalledTimes(1);
+  });
+
+  it("throws APIError after server error retries are exhausted", async () => {
+    const fetchMock = vi.fn(async () => new Response("{}", { status: 503 }));
+
+    const promise = apiFetch(new Request("https://api.argos-ci.test/builds"), {
+      fetch: fetchMock as unknown as typeof fetch,
+      minTimeout: 0,
+      retries: 1,
+    });
+
+    await expect(promise).rejects.toThrow(APIError);
+    await expect(promise).rejects.toThrow("Internal Server Error (503)");
+
+    expect(fetchMock).toHaveBeenCalledTimes(2);
+  });
+
+  it("aborts the request after the configured timeout", async () => {
+    const fetchMock = vi.fn(
+      async (input: RequestInfo | URL) =>
+        new Promise<Response>((_resolve, reject) => {
+          const request = input instanceof Request ? input : new Request(input);
+          if (request.signal.aborted) {
+            reject(request.signal.reason);
+            return;
+          }
+          request.signal.addEventListener(
+            "abort",
+            () => reject(request.signal.reason),
+            { once: true },
+          );
+        }),
+    );
+
+    await expect(
+      apiFetch(new Request("https://api.argos-ci.test/builds"), {
+        fetch: fetchMock as unknown as typeof fetch,
+        minTimeout: 0,
+        retries: 0,
+        timeout: 1,
+      }),
+    ).rejects.toMatchObject({
+      name: "TimeoutError",
+    });
+    expect(fetchMock).toHaveBeenCalledTimes(1);
+  });
+
+  it("preserves caller cancellation signal", async () => {
+    const controller = new AbortController();
+    const reason = new Error("cancelled by caller");
+    const fetchMock = vi.fn(
+      async (input: RequestInfo | URL) =>
+        new Promise<Response>((_resolve, reject) => {
+          const request = input instanceof Request ? input : new Request(input);
+          if (request.signal.aborted) {
+            reject(request.signal.reason);
+            return;
+          }
+          request.signal.addEventListener(
+            "abort",
+            () => reject(request.signal.reason),
+            { once: true },
+          );
+        }),
+    );
+
+    const promise = apiFetch(
+      new Request("https://api.argos-ci.test/builds", {
+        signal: controller.signal,
+      }),
+      {
+        fetch: fetchMock as unknown as typeof fetch,
+        minTimeout: 0,
+      },
+    );
+    const rejection = promise.catch((error: unknown) => error);
+
+    controller.abort(reason);
+
+    await expect(rejection).resolves.toBe(reason);
+    expect(fetchMock).toHaveBeenCalledTimes(1);
+  });
+
+  it("does not call fetch when the caller signal is already aborted", async () => {
+    const controller = new AbortController();
+    const reason = new Error("already cancelled");
+    const fetchMock = vi.fn(async () => new Response("{}"));
+
+    controller.abort(reason);
+
+    await expect(
+      apiFetch(
+        new Request("https://api.argos-ci.test/builds", {
+          signal: controller.signal,
+        }),
+        {
+          fetch: fetchMock as unknown as typeof fetch,
+        },
+      ),
+    ).rejects.toBe(reason);
+    expect(fetchMock).not.toHaveBeenCalled();
+  });
+});

packages/api-client/src/fetch.ts

@@ -0,0 +1,70 @@
+import pRetry from "p-retry";
+import { debug } from "./debug";
+
+const DEFAULT_TIMEOUT = 30_000;
+
+export class APIError extends Error {
+  constructor(message: string) {
+    super(message);
+  }
+}
+
+interface APIFetchOptions {
+  fetch?: typeof fetch;
+  minTimeout?: number;
+  retries?: number;
+  timeout?: number;
+}
+
+async function createRequestFactory(request: Request, timeout: number) {
+  // Snapshot the body once so retries do not clone/tee the original Request.
+  const body = request.body ? await request.arrayBuffer() : undefined;
+  const headers = new Headers(request.headers);
+
+  return () =>
+    new Request(request.url, {
+      body,
+      cache: request.cache,
+      credentials: request.credentials,
+      headers,
+      integrity: request.integrity,
+      keepalive: request.keepalive,
+      method: request.method,
+      mode: request.mode,
+      redirect: request.redirect,
+      referrer: request.referrer,
+      referrerPolicy: request.referrerPolicy,
+      signal: AbortSignal.any([request.signal, AbortSignal.timeout(timeout)]),
+    });
+}
+
+export async function apiFetch(input: Request, options: APIFetchOptions = {}) {
+  input.signal.throwIfAborted();
+
+  const fetchImpl = options.fetch ?? fetch;
+  const createRequest = await createRequestFactory(
+    input,
+    options.timeout ?? DEFAULT_TIMEOUT,
+  );
+
+  return pRetry(
+    async () => {
+      const response = await fetchImpl(createRequest());
+      if (response.status >= 500) {
+        throw new APIError(`Internal Server Error (${response.status})`);
+      }
+      return response;
+    },
+    {
+      minTimeout: options.minTimeout,
+      retries: options.retries ?? 3,
+      shouldRetry: () => !input.signal.aborted,
+      onFailedAttempt: (context) => {
+        debug("API request failed", context.error.message);
+        if (context.retriesLeft > 0) {
+          debug(`Retrying API request... (${context.retriesLeft} left)`);
+        }
+      },
+    },
+  );
+}

packages/api-client/src/index.ts

@@ -1,9 +1,10 @@
 import createFetchClient from "openapi-fetch";
-import pRetry from "p-retry";
 import { debug } from "./debug";
+import { apiFetch, APIError } from "./fetch";
 import type { paths, components } from "./schema";
 
 export * as ArgosAPISchema from "./schema";
+export { APIError } from "./fetch";
 
 export type ArgosAPIClient = ReturnType<typeof createClient>;
 
@@ -20,35 +21,10 @@ export function createClient(options?: {
     headers: {
       Authorization: authToken ? `Bearer ${authToken}` : undefined,
     },
-    fetch: (input) => {
-      return pRetry(
-        async () => {
-          const response = await fetch(input.clone());
-          if (response.status >= 500) {
-            throw new APIError("Internal Server Error");
-          }
-          return response;
-        },
-        {
-          retries: 3,
-          onFailedAttempt: (context) => {
-            debug("API request failed", context.error.message);
-            if (context.retriesLeft > 0) {
-              debug(`Retrying API request... (${context.retriesLeft} left)`);
-            }
-          },
-        },
-      );
-    },
+    fetch: apiFetch,
   });
 }
 
-export class APIError extends Error {
-  constructor(message: string) {
-    super(message);
-  }
-}
-
 /**
  * Handle API errors.
  */

packages/cli/e2e/upload-tokenless.test.js

@@ -4,7 +4,9 @@ import { run } from "./utils.js";
 
 // No ARGOS_TOKEN — authentication is handled via the GitHub Actions
 // tokenless exchange flow.
-test(
+// It is skipped because it only works on PR, enable it if you have to test tokenless.
+// eslint-disable-next-line vitest/no-disabled-tests
+test.skip(
   "upload returns a full build URL using tokenless authentication",
   { tags: ["tokenless"], timeout: 20_000 },
   () => {