Skip to content

Commit efccacd

Browse files
easonLiangWorldedtecheasonLiangWorldedtech
committed
refactor(providers): share the timeout-vs-abort helpers and dedupe the LM Studio catch selection
Review follow-ups: createTimeoutError/isTimeoutAbort move from qwen-code.ts and lm-studio.ts into utils/abort-signal.ts (providerName stays a createTimeoutError parameter; behavior unchanged) and both providers import them. LM Studio completePrompt selects TimeoutError vs the contract AbortError through one private helper used by the inner create catch and the outer completion catch. The qwen elapsed-timeout test now passes a live caller signal alongside timeoutMs and asserts TimeoutError, and uses mockImplementationOnce so the signal-waiting mock cannot leak into later tests. Contract tests for the shared helpers live in abort-signal.spec.ts. qwen-code + lm-studio + abort-signal suites 111 passed; abort-signal 58 passed; tsc clean.
1 parent 2f014e4 commit efccacd

5 files changed

Lines changed: 88 additions & 47 deletions

File tree

‎src/api/providers/__tests__/qwen-code.spec.ts‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -385,8 +385,8 @@ describe("QwenCodeHandler abort wiring", () => {
385385
expect(removeSpy).toHaveBeenCalledWith("abort", listener)
386386
})
387387

388-
it("reports an elapsed timeout as TimeoutError, not as caller cancellation", async () => {
389-
mockCreate.mockImplementation((_params?: unknown, opts?: { signal?: AbortSignal }) => {
388+
it("reports an elapsed timeout as TimeoutError while the caller signal stays live", async () => {
389+
mockCreate.mockImplementationOnce((_params?: unknown, opts?: { signal?: AbortSignal }) => {
390390
return new Promise((_resolve, reject) => {
391391
const signal = opts?.signal
392392
if (!signal) {
@@ -401,15 +401,17 @@ describe("QwenCodeHandler abort wiring", () => {
401401
})
402402
})
403403

404+
const caller = new AbortController()
404405
let caught: unknown
405406
try {
406-
await handler.completePrompt("hi", { timeoutMs: 20 })
407+
await handler.completePrompt("hi", { abortSignal: caller.signal, timeoutMs: 20 })
407408
} catch (error) {
408409
caught = error
409410
}
410411

411412
// The merged signal fired its own timeout while the caller's signal
412413
// stayed live: that is a deadline, not a user stop.
414+
expect(caller.signal.aborted).toBe(false)
413415
expect(caught).toBeInstanceOf(Error)
414416
expect((caught as Error).name).toBe("TimeoutError")
415417
expect((caught as Error).message).toBe("The Qwen Code completion request timed out")

‎src/api/providers/lm-studio.ts‎

Lines changed: 18 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -30,32 +30,15 @@ import {
3030
mergeAbortSignalAndTimeout,
3131
throwIfAborted,
3232
createAbortError,
33+
createTimeoutError,
3334
isRequestAborted,
35+
isTimeoutAbort,
3436
settleOnAbort,
3537
type OpenAiRequestOptions,
3638
} from "./utils/abort-signal"
3739
import { handleOpenAIError } from "./utils/error-handler"
3840
import { extractReasoningFromDelta } from "./utils/extract-reasoning"
3941

40-
/**
41-
* A per-request timeout is not a caller cancellation: report it as the distinct
42-
* TimeoutError so callers can tell a stalled request from a user stop (the
43-
* Gemini and LiteLLM units in this series use the same contract).
44-
*/
45-
function createTimeoutError(providerName: string): Error {
46-
const timeoutError = new Error(`The ${providerName} completion request timed out`)
47-
timeoutError.name = "TimeoutError"
48-
return timeoutError
49-
}
50-
51-
/**
52-
* Whether an abort-shaped failure came from the merged signal's own timeout
53-
* rather than from the caller's stop signal.
54-
*/
55-
function isTimeoutAbort(requestSignal: AbortSignal | undefined, callerSignal: AbortSignal | undefined): boolean {
56-
return Boolean(requestSignal?.aborted && !callerSignal?.aborted && requestSignal.reason?.name === "TimeoutError")
57-
}
58-
5942
export class LmStudioHandler extends BaseProvider implements SingleCompletionHandler {
6043
protected options: ApiHandlerOptions
6144
private client: OpenAI
@@ -330,6 +313,20 @@ export class LmStudioHandler extends BaseProvider implements SingleCompletionHan
330313
}
331314
}
332315

316+
/**
317+
* A timeout elapsing on the merged request signal is reported as the distinct TimeoutError; a
318+
* caller stop as the contract AbortError. The same selection runs from the inner create catch
319+
* and the outer completion catch, so it lives in one helper.
320+
*/
321+
private toCompletionAbortError(
322+
requestSignal: AbortSignal | undefined,
323+
callerSignal: AbortSignal | undefined,
324+
): Error {
325+
return isTimeoutAbort(requestSignal, callerSignal)
326+
? createTimeoutError("LM Studio")
327+
: createAbortError("LM Studio")
328+
}
329+
333330
async completePrompt(prompt: string, options?: CompletePromptOptions): Promise<string> {
334331
// Fast-fail if the caller’s stop signal already fired before we started.
335332
throwIfAborted(options?.abortSignal)
@@ -365,18 +362,14 @@ export class LmStudioHandler extends BaseProvider implements SingleCompletionHan
365362
response = await this.client.chat.completions.create(params, createOptions)
366363
} catch (error) {
367364
if (isRequestAborted(error, requestSignal)) {
368-
throw isTimeoutAbort(requestSignal, options?.abortSignal)
369-
? createTimeoutError("LM Studio")
370-
: createAbortError("LM Studio")
365+
throw this.toCompletionAbortError(requestSignal, options?.abortSignal)
371366
}
372367
throw handleOpenAIError(error, this.providerName)
373368
}
374369
return response.choices[0]?.message.content || ""
375370
} catch (error) {
376371
if (isRequestAborted(error, requestSignal)) {
377-
throw isTimeoutAbort(requestSignal, options?.abortSignal)
378-
? createTimeoutError("LM Studio")
379-
: createAbortError("LM Studio")
372+
throw this.toCompletionAbortError(requestSignal, options?.abortSignal)
380373
}
381374
throw new Error(
382375
"Please check the LM Studio developer logs to debug what went wrong. You may need to load the model with a larger context length to work with Zoo Code's prompts.",

‎src/api/providers/qwen-code.ts‎

Lines changed: 2 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ import {
2121
isRequestAborted,
2222
settleOnAbort,
2323
type OpenAiRequestOptions,
24+
createTimeoutError,
25+
isTimeoutAbort,
2426
} from "./utils/abort-signal"
2527
import type { SingleCompletionHandler, ApiHandlerCreateMessageMetadata, CompletePromptOptions } from "../index"
2628

@@ -59,25 +61,6 @@ function objectToUrlEncoded(data: Record<string, string>): string {
5961
.join("&")
6062
}
6163

62-
/**
63-
* A per-request timeout is not a caller cancellation: report it as the distinct
64-
* TimeoutError so callers can tell a stalled request from a user stop (the
65-
* Gemini and LiteLLM units in this series use the same contract).
66-
*/
67-
function createTimeoutError(providerName: string): Error {
68-
const timeoutError = new Error(`The ${providerName} completion request timed out`)
69-
timeoutError.name = "TimeoutError"
70-
return timeoutError
71-
}
72-
73-
/**
74-
* Whether the merged request signal aborted because its own timeout elapsed
75-
* (rather than because the caller stopped the request).
76-
*/
77-
function isTimeoutAbort(requestSignal: AbortSignal | undefined, callerSignal: AbortSignal | undefined): boolean {
78-
return Boolean(requestSignal?.aborted && !callerSignal?.aborted && requestSignal.reason?.name === "TimeoutError")
79-
}
80-
8164
export class QwenCodeHandler extends BaseProvider implements SingleCompletionHandler {
8265
protected options: QwenCodeHandlerOptions
8366
private credentials: QwenOAuthCredentials | null = null

‎src/api/providers/utils/__tests__/abort-signal.spec.ts‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ import {
1111
throwIfAborted,
1212
MAX_TIMEOUT_MS,
1313
toSdkTimeoutMs,
14+
createTimeoutError,
15+
isTimeoutAbort,
1416
} from "../abort-signal"
1517
import { withSettleGuard } from "../../../../test-utils/settle-guard"
1618

@@ -724,3 +726,45 @@ describe("toSdkTimeoutMs", () => {
724726
expect(toSdkTimeoutMs(MAX_TIMEOUT_MS + 1)).toBe(MAX_TIMEOUT_MS)
725727
})
726728
})
729+
730+
describe("createTimeoutError", () => {
731+
it("builds a TimeoutError naming the provider", () => {
732+
const error = createTimeoutError("LM Studio")
733+
734+
expect(error).toBeInstanceOf(Error)
735+
expect(error.name).toBe("TimeoutError")
736+
expect(error.message).toBe("The LM Studio completion request timed out")
737+
})
738+
})
739+
740+
describe("isTimeoutAbort", () => {
741+
it("is true only when the merged signal timed out while the caller stayed live", () => {
742+
const request = new AbortController()
743+
const caller = new AbortController()
744+
request.abort(createTimeoutError("Qwen Code"))
745+
746+
expect(isTimeoutAbort(request.signal, caller.signal)).toBe(true)
747+
})
748+
749+
it("is false for a caller cancellation", () => {
750+
const request = new AbortController()
751+
const caller = new AbortController()
752+
request.abort(createTimeoutError("Qwen Code"))
753+
caller.abort()
754+
755+
expect(isTimeoutAbort(request.signal, caller.signal)).toBe(false)
756+
})
757+
758+
it("is false when the request was aborted for a non-timeout reason", () => {
759+
const request = new AbortController()
760+
const caller = new AbortController()
761+
request.abort(new Error("shutdown"))
762+
763+
expect(isTimeoutAbort(request.signal, caller.signal)).toBe(false)
764+
})
765+
766+
it("is false without an aborted request signal", () => {
767+
expect(isTimeoutAbort(undefined, undefined)).toBe(false)
768+
expect(isTimeoutAbort(new AbortController().signal, undefined)).toBe(false)
769+
})
770+
})

‎src/api/providers/utils/abort-signal.ts‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -291,3 +291,22 @@ export function settleOnAbort<T>(
291291
)
292292
})
293293
}
294+
295+
/**
296+
* A per-request timeout is not a caller cancellation: report it as the distinct TimeoutError so
297+
* callers can tell a stalled request from a user stop (the Gemini and LiteLLM units in this
298+
* series use the same contract).
299+
*/
300+
export function createTimeoutError(providerName: string): Error {
301+
const timeoutError = new Error(`The ${providerName} completion request timed out`)
302+
timeoutError.name = "TimeoutError"
303+
return timeoutError
304+
}
305+
306+
/**
307+
* Whether an abort-shaped failure came from the merged signal's own timeout rather than from the
308+
* caller's stop signal.
309+
*/
310+
export function isTimeoutAbort(requestSignal: AbortSignal | undefined, callerSignal: AbortSignal | undefined): boolean {
311+
return Boolean(requestSignal?.aborted && !callerSignal?.aborted && requestSignal.reason?.name === "TimeoutError")
312+
}

0 commit comments

Comments
 (0)