Skip to content

Commit 899e717

Browse files
committed
fix(api): improve timeout and abort signal handling across providers
1 parent 69a98a6 commit 899e717

7 files changed

Lines changed: 46 additions & 36 deletions

File tree

src/api/providers/__tests__/gemini.spec.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,7 @@ describe("GeminiHandler", () => {
193193
model: GEMINI_MODEL_NAME,
194194
contents: [{ role: "user", parts: [{ text: "test prompt" }] }],
195195
config: {
196-
httpOptions: { signal: controller.signal },
196+
httpOptions: { signal: controller.signal, timeout: 10000 },
197197
temperature: 1,
198198
},
199199
})
@@ -206,7 +206,7 @@ describe("GeminiHandler", () => {
206206
model: GEMINI_MODEL_NAME,
207207
contents: [{ role: "user", parts: [{ text: "test prompt" }] }],
208208
config: {
209-
httpOptions: undefined,
209+
httpOptions: { timeout: 5000 },
210210
temperature: 1,
211211
},
212212
})

src/api/providers/__tests__/mistral.spec.ts

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -508,7 +508,7 @@ describe("MistralHandler", () => {
508508
})
509509
await handler.completePrompt("test prompt", { abortSignal: controller.signal })
510510
expect(mockComplete).toHaveBeenCalledWith(expect.objectContaining({ model: expect.any(String) }), {
511-
signal: controller.signal,
511+
fetchOptions: { signal: controller.signal },
512512
})
513513
})
514514

@@ -528,8 +528,8 @@ describe("MistralHandler", () => {
528528
})
529529
await handler.completePrompt("test prompt", { abortSignal: controller.signal, timeoutMs: 5000 })
530530
expect(mockComplete).toHaveBeenCalledWith(expect.objectContaining({ model: expect.any(String) }), {
531-
signal: controller.signal,
532-
timeout: 5000,
531+
fetchOptions: { signal: controller.signal },
532+
timeoutMs: 5000,
533533
})
534534
})
535535

@@ -539,7 +539,7 @@ describe("MistralHandler", () => {
539539
})
540540
await handler.completePrompt("test prompt", { timeoutMs: 3000 })
541541
expect(mockComplete).toHaveBeenCalledWith(expect.objectContaining({ model: expect.any(String) }), {
542-
timeout: 3000,
542+
timeoutMs: 3000,
543543
})
544544
})
545545

@@ -548,10 +548,9 @@ describe("MistralHandler", () => {
548548
choices: [{ message: { content: "response" } }],
549549
})
550550
await handler.completePrompt("test prompt", { timeoutMs: 0 })
551-
expect(mockComplete).toHaveBeenCalledWith(
552-
expect.objectContaining({ model: expect.any(String) }),
553-
undefined, // truthy check means 0 is falsy
554-
)
551+
expect(mockComplete).toHaveBeenCalledWith(expect.objectContaining({ model: expect.any(String) }), {
552+
timeoutMs: 0,
553+
})
555554
})
556555
})
557556
})

src/api/providers/gemini.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -589,6 +589,9 @@ export class GeminiHandler extends BaseProvider implements SingleCompletionHandl
589589
if (options?.abortSignal) {
590590
httpOpts.signal = options.abortSignal
591591
}
592+
if (options?.timeoutMs !== undefined) {
593+
httpOpts.timeout = options.timeoutMs
594+
}
592595
if (this.options.googleGeminiBaseUrl) {
593596
httpOpts.baseUrl = this.options.googleGeminiBaseUrl
594597
}

src/api/providers/mistral.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -197,13 +197,13 @@ export class MistralHandler extends BaseProvider implements SingleCompletionHand
197197
const { id: model, temperature } = this.getModel()
198198

199199
try {
200-
// Build request options with abortSignal and/or timeout handling
201-
const requestOptions: Record<string, unknown> = {}
200+
// Build Mistral SDK RequestOptions
201+
const requestOptions: Parameters<typeof this.client.chat.complete>[1] = {}
202202
if (options?.abortSignal) {
203-
requestOptions.signal = options.abortSignal
203+
requestOptions.fetchOptions = { signal: options.abortSignal }
204204
}
205-
if (options?.timeoutMs) {
206-
requestOptions.timeout = options.timeoutMs
205+
if (options?.timeoutMs !== undefined) {
206+
requestOptions.timeoutMs = options.timeoutMs
207207
}
208208

209209
const response = await this.client.chat.complete(
@@ -212,7 +212,7 @@ export class MistralHandler extends BaseProvider implements SingleCompletionHand
212212
messages: [{ role: "user", content: prompt }],
213213
temperature,
214214
},
215-
Object.keys(requestOptions).length > 0 ? (requestOptions as any) : undefined,
215+
Object.keys(requestOptions).length > 0 ? requestOptions : undefined,
216216
)
217217

218218
const content = response.choices?.[0]?.message.content

src/api/providers/openai-compatible.ts

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -208,19 +208,23 @@ export abstract class OpenAICompatibleHandler extends BaseProvider implements Si
208208
}
209209

210210
// Merge abortSignal and timeoutMs into a single abortSignal
211+
let timeoutId: ReturnType<typeof setTimeout> | undefined
211212
if (options?.abortSignal && options?.timeoutMs && options.timeoutMs > 0) {
212213
// When both are provided, create a merged signal that aborts when either fires
213214
const controller = new AbortController()
214-
const timeoutId = setTimeout(() => controller.abort(), options.timeoutMs)
215-
216-
options.abortSignal.addEventListener(
217-
"abort",
218-
() => {
219-
clearTimeout(timeoutId)
220-
controller.abort()
221-
},
222-
{ once: true },
223-
)
215+
if (options.abortSignal.aborted) {
216+
controller.abort()
217+
} else {
218+
timeoutId = setTimeout(() => controller.abort(), options.timeoutMs)
219+
options.abortSignal.addEventListener(
220+
"abort",
221+
() => {
222+
clearTimeout(timeoutId)
223+
controller.abort()
224+
},
225+
{ once: true },
226+
)
227+
}
224228

225229
generateOptions.abortSignal = controller.signal
226230
} else if (options?.abortSignal) {
@@ -229,8 +233,13 @@ export abstract class OpenAICompatibleHandler extends BaseProvider implements Si
229233
generateOptions.abortSignal = AbortSignal.timeout(options.timeoutMs)
230234
}
231235

232-
const { text } = await generateText(generateOptions)
233-
234-
return text
236+
try {
237+
const { text } = await generateText(generateOptions)
238+
return text
239+
} finally {
240+
if (timeoutId !== undefined) {
241+
clearTimeout(timeoutId)
242+
}
243+
}
235244
}
236245
}

src/api/providers/openai.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -315,7 +315,7 @@ export class OpenAiHandler extends BaseProvider implements SingleCompletionHandl
315315
if (options?.abortSignal) {
316316
createOptions.signal = options.abortSignal
317317
}
318-
if (options?.timeoutMs) {
318+
if (options?.timeoutMs !== undefined) {
319319
createOptions.timeout = options.timeoutMs
320320
}
321321

src/api/providers/vscode-lm.ts

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -595,19 +595,18 @@ export class VsCodeLmHandler extends BaseProvider implements SingleCompletionHan
595595
}
596596
}
597597
return result
598+
} catch (error: any) {
599+
if (error instanceof Error) {
600+
throw new Error(`VSCode LM completion error: ${error.message}`)
601+
}
602+
throw error
598603
} finally {
599604
if (timeoutTimeout) {
600605
clearTimeout(timeoutTimeout)
601606
}
602607
tokenSource.dispose()
603608
}
604609
}
605-
catch(error: any) {
606-
if (error instanceof Error) {
607-
throw new Error(`VSCode LM completion error: ${error.message}`)
608-
}
609-
throw error
610-
}
611610
}
612611

613612
// Static blacklist of VS Code Language Model IDs that should be excluded from the model list e.g. because they will never work

0 commit comments

Comments
 (0)