diff --git a/packages/api-client/package.json b/packages/api-client/package.json index 453e48b9..f29a7f66 100644 --- a/packages/api-client/package.json +++ b/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" } } diff --git a/packages/api-client/src/fetch.test.ts b/packages/api-client/src/fetch.test.ts new file mode 100644 index 00000000..90fe6e85 --- /dev/null +++ b/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((_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((_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(); + }); +}); diff --git a/packages/api-client/src/fetch.ts b/packages/api-client/src/fetch.ts new file mode 100644 index 00000000..de5a3572 --- /dev/null +++ b/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)`); + } + }, + }, + ); +} diff --git a/packages/api-client/src/index.ts b/packages/api-client/src/index.ts index 698e2681..fd22f300 100644 --- a/packages/api-client/src/index.ts +++ b/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; @@ -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. */ diff --git a/packages/cli/e2e/upload-tokenless.test.js b/packages/cli/e2e/upload-tokenless.test.js index 22728dd3..f069bab5 100644 --- a/packages/cli/e2e/upload-tokenless.test.js +++ b/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 }, () => { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index d86b37f0..644e0657 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -76,6 +76,9 @@ importers: openapi-fetch: specifier: ^0.17.0 version: 0.17.0 + p-retry: + specifier: ^8.0.0 + version: 8.0.0 devDependencies: '@types/debug': specifier: ^4.1.13 @@ -86,9 +89,9 @@ importers: openapi-typescript: specifier: ^7.13.0 version: 7.13.0(typescript@6.0.3) - p-retry: - specifier: ^8.0.0 - version: 8.0.0 + vitest: + specifier: 'catalog:' + version: 4.1.5(@types/node@20.19.40)(@vitest/browser-playwright@4.1.5)(@vitest/coverage-v8@4.1.5)(@vitest/ui@4.1.5)(msw@2.14.5(@types/node@20.19.40)(typescript@6.0.3))(vite@7.3.3(@types/node@20.19.40)(jiti@2.7.0)(tsx@4.21.0)(yaml@2.8.4)) packages/browser: devDependencies: