diff --git a/e2e/clawdhub.e2e.test.ts b/e2e/clawdhub.e2e.test.ts index 10e563dc7..00c20b80d 100644 --- a/e2e/clawdhub.e2e.test.ts +++ b/e2e/clawdhub.e2e.test.ts @@ -60,7 +60,7 @@ async function makeTempConfig(registry: string, token: string | null) { async function fetchWithTimeout(input: RequestInfo | URL, init?: RequestInit) { const controller = new AbortController() - const timeout = setTimeout(() => controller.abort('Timeout'), REQUEST_TIMEOUT_MS) + const timeout = setTimeout(() => controller.abort(new Error('Timeout')), REQUEST_TIMEOUT_MS) try { return await fetch(input, { ...init, signal: controller.signal }) } finally { diff --git a/packages/clawdhub/src/http.test.ts b/packages/clawdhub/src/http.test.ts index b98f144bb..e6cf962ec 100644 --- a/packages/clawdhub/src/http.test.ts +++ b/packages/clawdhub/src/http.test.ts @@ -1,9 +1,41 @@ /* @vitest-environment node */ import { describe, expect, it, vi } from 'vitest' -import { apiRequest, apiRequestForm, downloadZip } from './http' +import { apiRequest, apiRequestForm, downloadZip, fetchText } from './http' import { ApiV1WhoamiResponseSchema } from './schema/index.js' +function mockImmediateTimeouts() { + const setTimeoutMock = vi.fn((callback: () => void) => { + callback() + return 1 as unknown as ReturnType + }) + const clearTimeoutMock = vi.fn() + vi.stubGlobal('setTimeout', setTimeoutMock as typeof setTimeout) + vi.stubGlobal('clearTimeout', clearTimeoutMock as typeof clearTimeout) + return { setTimeoutMock, clearTimeoutMock } +} + +function createAbortingFetchMock() { + return vi.fn(async (_url: string, init?: RequestInit) => { + const signal = init?.signal + if (!signal || !(signal instanceof AbortSignal)) { + throw new Error('Missing abort signal') + } + if (signal.aborted) { + throw signal.reason + } + return await new Promise((_resolve, reject) => { + signal.addEventListener( + 'abort', + () => { + reject(signal.reason) + }, + { once: true }, + ) + }) + }) +} + describe('apiRequest', () => { it('adds bearer token and parses json', async () => { const fetchMock = vi.fn().mockResolvedValue({ @@ -92,6 +124,25 @@ describe('apiRequest', () => { expect(fetchMock).toHaveBeenCalledTimes(1) vi.unstubAllGlobals() }) + + it('aborts with Error timeouts and retries', async () => { + const { clearTimeoutMock } = mockImmediateTimeouts() + const fetchMock = createAbortingFetchMock() + vi.stubGlobal('fetch', fetchMock) + + let caught: unknown + try { + await apiRequest('https://example.com', { method: 'GET', path: '/x' }) + } catch (error) { + caught = error + } + + expect(caught).toBeInstanceOf(Error) + expect((caught as Error).message).toBe('Timeout') + expect(fetchMock).toHaveBeenCalledTimes(3) + expect(clearTimeoutMock.mock.calls.length).toBeGreaterThanOrEqual(3) + vi.unstubAllGlobals() + }) }) describe('apiRequestForm', () => { @@ -154,3 +205,24 @@ describe('apiRequestForm', () => { vi.unstubAllGlobals() }) }) + +describe('fetchText', () => { + it('aborts with Error timeouts and retries', async () => { + const { clearTimeoutMock } = mockImmediateTimeouts() + const fetchMock = createAbortingFetchMock() + vi.stubGlobal('fetch', fetchMock) + + let caught: unknown + try { + await fetchText('https://example.com', { path: '/x' }) + } catch (error) { + caught = error + } + + expect(caught).toBeInstanceOf(Error) + expect((caught as Error).message).toBe('Timeout') + expect(fetchMock).toHaveBeenCalledTimes(3) + expect(clearTimeoutMock.mock.calls.length).toBeGreaterThanOrEqual(3) + vi.unstubAllGlobals() + }) +}) diff --git a/packages/clawdhub/src/http.ts b/packages/clawdhub/src/http.ts index 430098970..34cf87cc4 100644 --- a/packages/clawdhub/src/http.ts +++ b/packages/clawdhub/src/http.ts @@ -52,22 +52,13 @@ export async function apiRequest( headers['Content-Type'] = 'application/json' body = JSON.stringify(args.body ?? {}) } - const controller = new AbortController() - const timeout = setTimeout(() => controller.abort('Timeout'), REQUEST_TIMEOUT_MS) - const response = await fetch(url, { + const response = await fetchWithTimeout(url, { method: args.method, headers, body, - signal: controller.signal, }) - clearTimeout(timeout) if (!response.ok) { - const text = await response.text().catch(() => '') - const message = text || `HTTP ${response.status}` - if (response.status === 429 || response.status >= 500) { - throw new Error(message) - } - throw new AbortError(message) + throwHttpStatusError(response.status, await readResponseTextSafe(response)) } return (await response.json()) as unknown }, @@ -101,22 +92,13 @@ export async function apiRequestForm( const headers: Record = { Accept: 'application/json' } if (args.token) headers.Authorization = `Bearer ${args.token}` - const controller = new AbortController() - const timeout = setTimeout(() => controller.abort('Timeout'), REQUEST_TIMEOUT_MS) - const response = await fetch(url, { + const response = await fetchWithTimeout(url, { method: args.method, headers, body: args.form, - signal: controller.signal, }) - clearTimeout(timeout) if (!response.ok) { - const text = await response.text().catch(() => '') - const message = text || `HTTP ${response.status}` - if (response.status === 429 || response.status >= 500) { - throw new Error(message) - } - throw new AbortError(message) + throwHttpStatusError(response.status, await readResponseTextSafe(response)) } return (await response.json()) as unknown }, @@ -138,17 +120,10 @@ export async function fetchText(registry: string, args: TextRequestArgs): Promis const headers: Record = { Accept: 'text/plain' } if (args.token) headers.Authorization = `Bearer ${args.token}` - const controller = new AbortController() - const timeout = setTimeout(() => controller.abort('Timeout'), REQUEST_TIMEOUT_MS) - const response = await fetch(url, { method: 'GET', headers, signal: controller.signal }) - clearTimeout(timeout) + const response = await fetchWithTimeout(url, { method: 'GET', headers }) const text = await response.text() if (!response.ok) { - const message = text || `HTTP ${response.status}` - if (response.status === 429 || response.status >= 500) { - throw new Error(message) - } - throw new AbortError(message) + throwHttpStatusError(response.status, text) } return text }, @@ -166,16 +141,9 @@ export async function downloadZip(registry: string, args: { slug: string; versio return await fetchBinaryViaCurl(url.toString()) } - const controller = new AbortController() - const timeout = setTimeout(() => controller.abort('Timeout'), REQUEST_TIMEOUT_MS) - const response = await fetch(url.toString(), { method: 'GET', signal: controller.signal }) - clearTimeout(timeout) + const response = await fetchWithTimeout(url.toString(), { method: 'GET' }) if (!response.ok) { - const message = (await response.text().catch(() => '')) || `HTTP ${response.status}` - if (response.status === 429 || response.status >= 500) { - throw new Error(message) - } - throw new AbortError(message) + throwHttpStatusError(response.status, await readResponseTextSafe(response)) } return new Uint8Array(await response.arrayBuffer()) }, @@ -183,6 +151,28 @@ export async function downloadZip(registry: string, args: { slug: string; versio ) } +async function fetchWithTimeout(url: string, init: RequestInit): Promise { + const controller = new AbortController() + const timeout = setTimeout(() => controller.abort(new Error('Timeout')), REQUEST_TIMEOUT_MS) + try { + return await fetch(url, { ...init, signal: controller.signal }) + } finally { + clearTimeout(timeout) + } +} + +async function readResponseTextSafe(response: Response): Promise { + return await response.text().catch(() => '') +} + +function throwHttpStatusError(status: number, text: string): never { + const message = text || `HTTP ${status}` + if (status === 429 || status >= 500) { + throw new Error(message) + } + throw new AbortError(message) +} + async function fetchJsonViaCurl(url: string, args: RequestArgs) { const headers = ['-H', 'Accept: application/json'] if (args.token) { @@ -217,10 +207,7 @@ async function fetchJsonViaCurl(url: string, args: RequestArgs) { const status = Number(output.slice(splitAt + 1).trim()) if (!Number.isFinite(status)) throw new Error('curl response missing status') if (status < 200 || status >= 300) { - if (status === 429 || status >= 500) { - throw new Error(body || `HTTP ${status}`) - } - throw new AbortError(body || `HTTP ${status}`) + throwHttpStatusError(status, body) } return JSON.parse(body || 'null') as unknown } @@ -272,10 +259,7 @@ async function fetchJsonFormViaCurl(url: string, args: FormRequestArgs) { const status = Number(output.slice(splitAt + 1).trim()) if (!Number.isFinite(status)) throw new Error('curl response missing status') if (status < 200 || status >= 300) { - if (status === 429 || status >= 500) { - throw new Error(body || `HTTP ${status}`) - } - throw new AbortError(body || `HTTP ${status}`) + throwHttpStatusError(status, body) } return JSON.parse(body || 'null') as unknown } finally { @@ -344,11 +328,7 @@ async function fetchBinaryViaCurl(url: string) { if (!Number.isFinite(status)) throw new Error('curl response missing status') if (status < 200 || status >= 300) { const body = await readFileSafe(filePath) - const message = body ? new TextDecoder().decode(body) : `HTTP ${status}` - if (status === 429 || status >= 500) { - throw new Error(message) - } - throw new AbortError(message) + throwHttpStatusError(status, body ? new TextDecoder().decode(body) : '') } const bytes = await readFileSafe(filePath) return bytes ? new Uint8Array(bytes) : new Uint8Array()