Skip to content

Commit ef36cfd

Browse files
steipetesteipete
authored
refactor(cli): centralize HTTP status errors and timeout tests (#286)
1 parent e0637ad commit ef36cfd

File tree

2 files changed

+73
-61
lines changed

2 files changed

+73
-61
lines changed

‎packages/clawdhub/src/http.test.ts‎

Lines changed: 54 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,38 @@ import { describe, expect, it, vi } from 'vitest'
44
import { apiRequest, apiRequestForm, downloadZip, fetchText } from './http'
55
import { ApiV1WhoamiResponseSchema } from './schema/index.js'
66

7+
function mockImmediateTimeouts() {
8+
const setTimeoutMock = vi.fn((callback: () => void) => {
9+
callback()
10+
return 1 as unknown as ReturnType<typeof setTimeout>
11+
})
12+
const clearTimeoutMock = vi.fn()
13+
vi.stubGlobal('setTimeout', setTimeoutMock as typeof setTimeout)
14+
vi.stubGlobal('clearTimeout', clearTimeoutMock as typeof clearTimeout)
15+
return { setTimeoutMock, clearTimeoutMock }
16+
}
17+
18+
function createAbortingFetchMock() {
19+
return vi.fn(async (_url: string, init?: RequestInit) => {
20+
const signal = init?.signal
21+
if (!signal || !(signal instanceof AbortSignal)) {
22+
throw new Error('Missing abort signal')
23+
}
24+
if (signal.aborted) {
25+
throw signal.reason
26+
}
27+
return await new Promise<Response>((_resolve, reject) => {
28+
signal.addEventListener(
29+
'abort',
30+
() => {
31+
reject(signal.reason)
32+
},
33+
{ once: true },
34+
)
35+
})
36+
})
37+
}
38+
739
describe('apiRequest', () => {
840
it('adds bearer token and parses json', async () => {
941
const fetchMock = vi.fn().mockResolvedValue({
@@ -92,6 +124,25 @@ describe('apiRequest', () => {
92124
expect(fetchMock).toHaveBeenCalledTimes(1)
93125
vi.unstubAllGlobals()
94126
})
127+
128+
it('aborts with Error timeouts and retries', async () => {
129+
const { clearTimeoutMock } = mockImmediateTimeouts()
130+
const fetchMock = createAbortingFetchMock()
131+
vi.stubGlobal('fetch', fetchMock)
132+
133+
let caught: unknown
134+
try {
135+
await apiRequest('https://example.com', { method: 'GET', path: '/x' })
136+
} catch (error) {
137+
caught = error
138+
}
139+
140+
expect(caught).toBeInstanceOf(Error)
141+
expect((caught as Error).message).toBe('Timeout')
142+
expect(fetchMock).toHaveBeenCalledTimes(3)
143+
expect(clearTimeoutMock.mock.calls.length).toBeGreaterThanOrEqual(3)
144+
vi.unstubAllGlobals()
145+
})
95146
})
96147

97148
describe('apiRequestForm', () => {
@@ -157,32 +208,8 @@ describe('apiRequestForm', () => {
157208

158209
describe('fetchText', () => {
159210
it('aborts with Error timeouts and retries', async () => {
160-
const setTimeoutMock = vi.fn((callback: () => void) => {
161-
callback()
162-
return 1 as unknown as ReturnType<typeof setTimeout>
163-
})
164-
const clearTimeoutMock = vi.fn()
165-
vi.stubGlobal('setTimeout', setTimeoutMock as typeof setTimeout)
166-
vi.stubGlobal('clearTimeout', clearTimeoutMock as typeof clearTimeout)
167-
168-
const fetchMock = vi.fn(async (_url: string, init?: RequestInit) => {
169-
const signal = init?.signal
170-
if (!signal || !(signal instanceof AbortSignal)) {
171-
throw new Error('Missing abort signal')
172-
}
173-
if (signal.aborted) {
174-
throw signal.reason
175-
}
176-
return await new Promise<Response>((_resolve, reject) => {
177-
signal.addEventListener(
178-
'abort',
179-
() => {
180-
reject(signal.reason)
181-
},
182-
{ once: true },
183-
)
184-
})
185-
})
211+
const { clearTimeoutMock } = mockImmediateTimeouts()
212+
const fetchMock = createAbortingFetchMock()
186213
vi.stubGlobal('fetch', fetchMock)
187214

188215
let caught: unknown
@@ -195,6 +222,7 @@ describe('fetchText', () => {
195222
expect(caught).toBeInstanceOf(Error)
196223
expect((caught as Error).message).toBe('Timeout')
197224
expect(fetchMock).toHaveBeenCalledTimes(3)
225+
expect(clearTimeoutMock.mock.calls.length).toBeGreaterThanOrEqual(3)
198226
vi.unstubAllGlobals()
199227
})
200228
})

‎packages/clawdhub/src/http.ts‎

Lines changed: 19 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -58,12 +58,7 @@ export async function apiRequest<T>(
5858
body,
5959
})
6060
if (!response.ok) {
61-
const text = await response.text().catch(() => '')
62-
const message = text || `HTTP ${response.status}`
63-
if (response.status === 429 || response.status >= 500) {
64-
throw new Error(message)
65-
}
66-
throw new AbortError(message)
61+
throwHttpStatusError(response.status, await readResponseTextSafe(response))
6762
}
6863
return (await response.json()) as unknown
6964
},
@@ -103,12 +98,7 @@ export async function apiRequestForm<T>(
10398
body: args.form,
10499
})
105100
if (!response.ok) {
106-
const text = await response.text().catch(() => '')
107-
const message = text || `HTTP ${response.status}`
108-
if (response.status === 429 || response.status >= 500) {
109-
throw new Error(message)
110-
}
111-
throw new AbortError(message)
101+
throwHttpStatusError(response.status, await readResponseTextSafe(response))
112102
}
113103
return (await response.json()) as unknown
114104
},
@@ -133,11 +123,7 @@ export async function fetchText(registry: string, args: TextRequestArgs): Promis
133123
const response = await fetchWithTimeout(url, { method: 'GET', headers })
134124
const text = await response.text()
135125
if (!response.ok) {
136-
const message = text || `HTTP ${response.status}`
137-
if (response.status === 429 || response.status >= 500) {
138-
throw new Error(message)
139-
}
140-
throw new AbortError(message)
126+
throwHttpStatusError(response.status, text)
141127
}
142128
return text
143129
},
@@ -157,11 +143,7 @@ export async function downloadZip(registry: string, args: { slug: string; versio
157143

158144
const response = await fetchWithTimeout(url.toString(), { method: 'GET' })
159145
if (!response.ok) {
160-
const message = (await response.text().catch(() => '')) || `HTTP ${response.status}`
161-
if (response.status === 429 || response.status >= 500) {
162-
throw new Error(message)
163-
}
164-
throw new AbortError(message)
146+
throwHttpStatusError(response.status, await readResponseTextSafe(response))
165147
}
166148
return new Uint8Array(await response.arrayBuffer())
167149
},
@@ -179,6 +161,18 @@ async function fetchWithTimeout(url: string, init: RequestInit): Promise<Respons
179161
}
180162
}
181163

164+
async function readResponseTextSafe(response: Response): Promise<string> {
165+
return await response.text().catch(() => '')
166+
}
167+
168+
function throwHttpStatusError(status: number, text: string): never {
169+
const message = text || `HTTP ${status}`
170+
if (status === 429 || status >= 500) {
171+
throw new Error(message)
172+
}
173+
throw new AbortError(message)
174+
}
175+
182176
async function fetchJsonViaCurl(url: string, args: RequestArgs) {
183177
const headers = ['-H', 'Accept: application/json']
184178
if (args.token) {
@@ -213,10 +207,7 @@ async function fetchJsonViaCurl(url: string, args: RequestArgs) {
213207
const status = Number(output.slice(splitAt + 1).trim())
214208
if (!Number.isFinite(status)) throw new Error('curl response missing status')
215209
if (status < 200 || status >= 300) {
216-
if (status === 429 || status >= 500) {
217-
throw new Error(body || `HTTP ${status}`)
218-
}
219-
throw new AbortError(body || `HTTP ${status}`)
210+
throwHttpStatusError(status, body)
220211
}
221212
return JSON.parse(body || 'null') as unknown
222213
}
@@ -268,10 +259,7 @@ async function fetchJsonFormViaCurl(url: string, args: FormRequestArgs) {
268259
const status = Number(output.slice(splitAt + 1).trim())
269260
if (!Number.isFinite(status)) throw new Error('curl response missing status')
270261
if (status < 200 || status >= 300) {
271-
if (status === 429 || status >= 500) {
272-
throw new Error(body || `HTTP ${status}`)
273-
}
274-
throw new AbortError(body || `HTTP ${status}`)
262+
throwHttpStatusError(status, body)
275263
}
276264
return JSON.parse(body || 'null') as unknown
277265
} finally {
@@ -340,11 +328,7 @@ async function fetchBinaryViaCurl(url: string) {
340328
if (!Number.isFinite(status)) throw new Error('curl response missing status')
341329
if (status < 200 || status >= 300) {
342330
const body = await readFileSafe(filePath)
343-
const message = body ? new TextDecoder().decode(body) : `HTTP ${status}`
344-
if (status === 429 || status >= 500) {
345-
throw new Error(message)
346-
}
347-
throw new AbortError(message)
331+
throwHttpStatusError(status, body ? new TextDecoder().decode(body) : '')
348332
}
349333
const bytes = await readFileSafe(filePath)
350334
return bytes ? new Uint8Array(bytes) : new Uint8Array()

0 commit comments

Comments
 (0)