diff --git a/src/cli.ts b/src/cli.ts index cc09329..644345c 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -117,6 +117,7 @@ export async function main(): Promise { baseUrl: config.baseUrl, temperature: config.temperature, max_tokens: config.maxTokens, + timeoutMs: config.timeoutMs, }) const agent = new Agent({ llm, maxContextTokens: config.maxContextTokens }) diff --git a/src/config.ts b/src/config.ts index a94ffe6..b2f958c 100644 --- a/src/config.ts +++ b/src/config.ts @@ -53,6 +53,8 @@ export interface Config { maxTokens: number temperature: number maxContextTokens: number + /** Per-request timeout for the LLM client, in ms. */ + timeoutMs: number } /** Parse an integer env var; garbage (or trailing junk) falls back to the default. */ @@ -85,5 +87,6 @@ export function configFromEnv(): Config { maxTokens: intFromEnv('CORECODER_MAX_TOKENS', 4096), temperature: floatFromEnv('CORECODER_TEMPERATURE', 0), maxContextTokens: intFromEnv('CORECODER_MAX_CONTEXT', 128000), + timeoutMs: intFromEnv('CORECODER_TIMEOUT_MS', 300000), } } diff --git a/src/llm.ts b/src/llm.ts index c36d095..8865a5d 100644 --- a/src/llm.ts +++ b/src/llm.ts @@ -159,19 +159,23 @@ export class LLM implements LLMClient { private apiKey: string private baseUrl: string + private timeoutMs: number private extra: Record constructor(opts: { model: string apiKey: string baseUrl?: string | null + /** How long to wait for each request before giving up and retrying. */ + timeoutMs?: number /** temperature, max_tokens, etc. — forwarded verbatim to the API */ [k: string]: unknown }) { - const { model, apiKey, baseUrl, ...extra } = opts + const { model, apiKey, baseUrl, timeoutMs, ...extra } = opts this.model = model this.apiKey = apiKey this.baseUrl = (baseUrl || 'https://api.openai.com/v1').replace(/\/+$/, '') + this.timeoutMs = timeoutMs ?? 300_000 // 5 minutes this.extra = extra } @@ -306,6 +310,26 @@ export class LLM implements LLMClient { } private async callOnce(params: Record, signal?: AbortSignal): Promise { + // Node's fetch has no timeout option (RequestInit.timeout is silently + // ignored), so a provider that accepts the connection and then never + // answers would hang the agent forever. Race the request against a timer: + // the timer aborts the fetch and we translate that into a retryable + // transient error. A user cancellation must pass through untouched — the + // controller is aborted by the user's signal first, before the timer can + // claim it. + const controller = new AbortController() + let timedOut = false + const onAbort = () => controller.abort() + if (signal?.aborted) { + controller.abort() + } else { + signal?.addEventListener('abort', onAbort, { once: true }) + } + const timer = setTimeout(() => { + timedOut = true + controller.abort() + }, this.timeoutMs) + let res: Response try { res = await fetch(`${this.baseUrl}/chat/completions`, { @@ -315,12 +339,20 @@ export class LLM implements LLMClient { authorization: `Bearer ${this.apiKey}`, }, body: JSON.stringify(params), - signal: signal ?? null, + signal: controller.signal, }) } catch (e) { // user cancellation must not be retried - if (e instanceof Error && e.name === 'AbortError') throw e + if (e instanceof Error && e.name === 'AbortError') { + if (timedOut) { + throw new TransientError(`request timed out after ${this.timeoutMs}ms`) + } + throw e + } throw new TransientError(`connection error: ${e}`) + } finally { + clearTimeout(timer) + signal?.removeEventListener('abort', onAbort) } if (res.ok) return res diff --git a/tests/llm-timeout.test.ts b/tests/llm-timeout.test.ts new file mode 100644 index 0000000..85b3173 --- /dev/null +++ b/tests/llm-timeout.test.ts @@ -0,0 +1,103 @@ +/** + * Request timeout tests: a provider that accepts the connection and never + * answers must not hang the agent forever, and a user cancellation must + * never be mistaken for a timeout. + */ + +import assert from 'node:assert/strict' +import { test } from 'node:test' + +import { LLM, drain } from '../src/llm.js' + +/** fetch stub that hangs until its signal aborts (the timeout or the user). */ +function hungFetch(): typeof fetch { + return ((_url, init) => + new Promise((_resolve, reject) => { + // real fetch rejects immediately when handed an already-aborted signal; + // addEventListener alone would never fire and the stub would hang forever + if (init?.signal?.aborted) { + reject(new DOMException('Aborted', 'AbortError')) + return + } + init?.signal?.addEventListener( + 'abort', + () => reject(new DOMException('Aborted', 'AbortError')), + { once: true }, + ) + })) as typeof fetch +} + +function sseResponse(content: string): Response { + const body = + 'data: {"id":"1","object":"chat.completion.chunk","choices":[{"index":0,"delta":{"content":"' + + content + + '"}}]}\n\n' + + 'data: [DONE]\n\n' + return new Response(body, { headers: { 'content-type': 'text/event-stream' } }) +} + +test('a hung provider times out and the retry loop reports it', async () => { + const originalFetch = globalThis.fetch + globalThis.fetch = hungFetch() + const llm = new LLM({ model: 'm', apiKey: 'k', timeoutMs: 50 }) + try { + await assert.rejects( + drain(llm.chat([{ role: 'user', content: 'hi' }])), + (e: Error) => /timed out/.test(e.message), + ) + } finally { + globalThis.fetch = originalFetch + } +}) + +test('a user cancellation is not masked by the timeout', async () => { + const originalFetch = globalThis.fetch + globalThis.fetch = hungFetch() + const llm = new LLM({ model: 'm', apiKey: 'k', timeoutMs: 50 }) + const ac = new AbortController() + ac.abort() // cancelled before the request even starts + try { + await assert.rejects( + drain(llm.chat([{ role: 'user', content: 'hi' }], undefined, ac.signal)), + (e: Error) => e.name === 'AbortError', + ) + } finally { + globalThis.fetch = originalFetch + } +}) + +test('a timeout is retried and the next attempt succeeds', async () => { + const originalFetch = globalThis.fetch + let calls = 0 + globalThis.fetch = ((_url, init) => { + calls++ + if (calls === 1) { + // first attempt hangs until the timeout aborts it + return new Promise((_resolve, reject) => { + if (init?.signal?.aborted) { + reject(new DOMException('Aborted', 'AbortError')) + return + } + init?.signal?.addEventListener( + 'abort', + () => reject(new DOMException('Aborted', 'AbortError')), + { once: true }, + ) + }) + } + return Promise.resolve(sseResponse('ok')) + }) as typeof fetch + const llm = new LLM({ model: 'm', apiKey: 'k', timeoutMs: 50 }) + try { + const parts: string[] = [] + const gen = llm.chat([{ role: 'user', content: 'hi' }]) + let step = await gen.next() + while (!step.done) { + parts.push(step.value) + step = await gen.next() + } + assert.deepEqual(parts, ['ok']) + } finally { + globalThis.fetch = originalFetch + } +})