Compare commits

...
Author SHA1 Message Date
Peter Steinberger f697310f62 refactor(cli): centralize HTTP status errors and timeout tests 2026-02-14 02:38:08 +01:00
Peter Steinberger 5476e8a786 fix(cli): unify timeout abort handling 2026-02-14 02:26:19 +01:00
Peter Steinberger d4bc86d5f4 Merge remote-tracking branch 'origin/main' into fix/cli-timeout 2026-02-14 02:24:30 +01:00
Sash Zats 8665d44738 fix(http): wrap fetch calls in try-finally to prevent timer leaks
Addresses Vercel review comment: clearTimeout was not called on error paths when fetch throws an exception.
2026-01-26 12:36:07 +00:00
Sash Zats f500c73372 fix(cli): throw Error on timeout aborts
Users have seen an elevated number of:\n  clawdhub search image\n  ✖ Non-error was thrown: "Timeout". You should only throw errors.\n\nInvestigation shows we were aborting with a string instead of an Error. Switching to controller.abort(new Error('Timeout')) makes retries/formatting treat it as a real error and clears the message.\n\nExample after change:\n  clawdhub search image\n  table-image v1.0.0  Table Image  (0.332)\n  nano-banana-pro v1.0.1  Nano Banana Pro  (0.319)\n  vap-media v1.0.1  AI media generation API - Flux2pro, Veo3.1, Suno Ai  (0.281)\n  clawdbot-meshyai-skill v0.1.0  Meshy AI  (0.276)\n  venice-ai-media v1.0.0  Venice AI Media  (0.274)\n  daily-recap v1.0.2  Daily Recap  (0.260)\n  openai-image-gen v1.0.1  Openai Image Gen  (0.260)\n  bible-votd v1.0.1  Bible Verse of the Day  (0.248)\n  orf v1.0.1  ORF  (0.224)\n  smalltalk v1.0.1  Smalltalk  (0.161)
2026-01-25 22:42:46 -05:00
3 changed files with 107 additions and 55 deletions
+1 -1
View File
@@ -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 {
+73 -1
View File
@@ -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<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', () => {
@@ -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()
})
})
+33 -53
View File
@@ -52,22 +52,13 @@ export async function apiRequest<T>(
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<T>(
const headers: Record<string, string> = { 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<string, string> = { 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<Response> {
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<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) {
@@ -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()