refactor(cli): centralize HTTP status errors and timeout tests

packages/clawdhub/src/http.test.ts

@@ -4,6 +4,38 @@ import { describe, expect, it, vi } from 'vitest'
 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<typeof setTimeout>
+  })
+  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<Response>((_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', () => {
@@ -157,32 +208,8 @@ describe('apiRequestForm', () => {
 
 describe('fetchText', () => {
   it('aborts with Error timeouts and retries', async () => {
-    const setTimeoutMock = vi.fn((callback: () => void) => {
-      callback()
-      return 1 as unknown as ReturnType<typeof setTimeout>
-    })
-    const clearTimeoutMock = vi.fn()
-    vi.stubGlobal('setTimeout', setTimeoutMock as typeof setTimeout)
-    vi.stubGlobal('clearTimeout', clearTimeoutMock as typeof clearTimeout)
-
-    const fetchMock = 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<Response>((_resolve, reject) => {
-        signal.addEventListener(
-          'abort',
-          () => {
-            reject(signal.reason)
-          },
-          { once: true },
-        )
-      })
-    })
+    const { clearTimeoutMock } = mockImmediateTimeouts()
+    const fetchMock = createAbortingFetchMock()
     vi.stubGlobal('fetch', fetchMock)
 
     let caught: unknown
@@ -195,6 +222,7 @@ describe('fetchText', () => {
     expect(caught).toBeInstanceOf(Error)
     expect((caught as Error).message).toBe('Timeout')
     expect(fetchMock).toHaveBeenCalledTimes(3)
+    expect(clearTimeoutMock.mock.calls.length).toBeGreaterThanOrEqual(3)
     vi.unstubAllGlobals()
   })
 })

packages/clawdhub/src/http.ts

@@ -58,12 +58,7 @@ export async function apiRequest<T>(
         body,
       })
       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
     },
@@ -103,12 +98,7 @@ export async function apiRequestForm<T>(
         body: args.form,
       })
       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
     },
@@ -133,11 +123,7 @@ export async function fetchText(registry: string, args: TextRequestArgs): Promis
       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
     },
@@ -157,11 +143,7 @@ export async function downloadZip(registry: string, args: { slug: string; versio
 
       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())
     },
@@ -179,6 +161,18 @@ async function fetchWithTimeout(url: string, init: RequestInit): Promise<Respons
   }
 }
 
+async function readResponseTextSafe(response: Response): Promise<string> {
+  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) {
@@ -213,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
 }
@@ -268,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 {
@@ -340,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()