From e61feb13e160e711f914c5dd8d283b70bd764221 Mon Sep 17 00:00:00 2001 From: Eason Liang Date: Wed, 19 Aug 2026 20:55:39 +0800 Subject: [PATCH 01/11] feat(api): add throwIfAborted helper and completePrompt options regression tests Add a fast-fail throwIfAborted guard to the shared abort-signal utilities and regression tests for the CompletePromptOptions interface (added by #901). --- .../__tests__/complete-prompt-options.spec.ts | 29 +++++++++++++++++++ .../utils/__tests__/abort-signal.spec.ts | 29 ++++++++++++++++++- src/api/providers/utils/abort-signal.ts | 17 +++++++++++ 3 files changed, 74 insertions(+), 1 deletion(-) create mode 100644 src/api/providers/__tests__/complete-prompt-options.spec.ts diff --git a/src/api/providers/__tests__/complete-prompt-options.spec.ts b/src/api/providers/__tests__/complete-prompt-options.spec.ts new file mode 100644 index 0000000000..f9925cd119 --- /dev/null +++ b/src/api/providers/__tests__/complete-prompt-options.spec.ts @@ -0,0 +1,29 @@ +import { describe, it, expect } from "vitest" + +import type { CompletePromptOptions } from "../../index" + +describe("CompletePromptOptions", () => { + it("should allow abortSignal property", () => { + const controller = new AbortController() + const options: CompletePromptOptions = { abortSignal: controller.signal } + expect(options.abortSignal).toBe(controller.signal) + }) + + it("should allow timeoutMs property", () => { + const options: CompletePromptOptions = { timeoutMs: 5000 } + expect(options.timeoutMs).toBe(5000) + }) + + it("should allow both abortSignal and timeoutMs together", () => { + const controller = new AbortController() + const options: CompletePromptOptions = { abortSignal: controller.signal, timeoutMs: 10000 } + expect(options.abortSignal).toBe(controller.signal) + expect(options.timeoutMs).toBe(10000) + }) + + it("should allow empty options object", () => { + const options: CompletePromptOptions = {} + expect(options.abortSignal).toBeUndefined() + expect(options.timeoutMs).toBeUndefined() + }) +}) diff --git a/src/api/providers/utils/__tests__/abort-signal.spec.ts b/src/api/providers/utils/__tests__/abort-signal.spec.ts index ebc7edf3d3..1e2181655f 100644 --- a/src/api/providers/utils/__tests__/abort-signal.spec.ts +++ b/src/api/providers/utils/__tests__/abort-signal.spec.ts @@ -1,4 +1,4 @@ -import { mergeAbortSignalAndTimeout, mergeAbortSignals } from "../abort-signal" +import { mergeAbortSignalAndTimeout, mergeAbortSignals, throwIfAborted } from "../abort-signal" describe("abort-signal utilities", () => { describe("mergeAbortSignalAndTimeout", () => { @@ -99,4 +99,31 @@ describe("abort-signal utilities", () => { expect(result.aborted).toBe(true) }) }) + + describe("throwIfAborted", () => { + it("does not throw when signal is undefined", () => { + expect(() => throwIfAborted()).not.toThrow() + }) + + it("does not throw when signal is not aborted", () => { + const controller = new AbortController() + + expect(() => throwIfAborted(controller.signal)).not.toThrow() + }) + + it("throws an AbortError when signal is already aborted", () => { + const controller = new AbortController() + controller.abort() + + let caught: unknown + try { + throwIfAborted(controller.signal) + } catch (error) { + caught = error + } + + expect(caught).toBeInstanceOf(Error) + expect((caught as Error).name).toBe("AbortError") + }) + }) }) diff --git a/src/api/providers/utils/abort-signal.ts b/src/api/providers/utils/abort-signal.ts index 73e0356f7b..033e861b2b 100644 --- a/src/api/providers/utils/abort-signal.ts +++ b/src/api/providers/utils/abort-signal.ts @@ -35,3 +35,20 @@ export function mergeAbortSignals(primarySignal: AbortSignal, secondarySignal?: return AbortSignal.any([primarySignal, secondarySignal]) } + +/** + * Throw an AbortError if the given signal is already aborted. + * + * Use as a fast-fail guard at the top of request-building code paths so + * callers receive a consistent `name === "AbortError"` when the operation + * was cancelled before it started, without building or issuing the request. + */ +export function throwIfAborted(signal?: AbortSignal): void { + if (!signal?.aborted) { + return + } + + const abortError = new Error("This operation was aborted") + abortError.name = "AbortError" + throw abortError +} From e65cc08e8a6968256a8e1306317fada4691e1d7d Mon Sep 17 00:00:00 2001 From: easonLiangWorldedtech Date: Fri, 21 Aug 2026 05:13:05 +0800 Subject: [PATCH 02/11] feat(api): abort signal support for openai, openai-compatible base, zai, kimi-code (round 2) Round 2 of the abort-signal series: wires request-cancellation signals through the OpenAI family of providers (addresses #404). - openai.ts: all five client.chat.completions.create sites (createMessage streaming + non-streaming, O3-family streaming + non-streaming, completePrompt) build their request config through RequestConfigBuilder; the Azure AI Inference path option and the abort signal compose in one builder (setOption("path", ...) + setAbortSignal). Every catch normalizes abort failures to the Task.ts contract shape (name === "AbortError", message ending in "aborted") via an abort-aware handleOpenAIRequestError; non-abort errors keep the existing provider-prefix wrap. - base-openai-compatible-provider.ts: the shared createMessage / createStream / completePrompt path adopts RequestConfigBuilder for signal forwarding and gains the exported abort-aware error helper handleOpenAIRequestError (reused by zai.ts); subclasses that do not override these methods inherit the wiring. - zai.ts: audit finding fixed - the GLM thinking path in createStream no longer drops requestOptions; the thinking path and the glm-5.3 completePrompt path forward a merged signal (external signal + timeoutMs via mergeAbortSignalAndTimeout). - kimi-code.ts: completePrompt no longer drops CompletePromptOptions - options are forwarded on both the initial call and the 401 OAuth retry. - Design notes: CompletePromptOptions is not ApiHandlerCreateMessageMetadata (required taskId, gap G7), so completePrompt paths use setOption("signal", mergeAbortSignalAndTimeout(...)) instead of setAbortSignal(metadata); gap G5 - mergeAbortSignalAndTimeout treats timeoutMs <= 0 as no explicit timeout. Each call builds a fresh request-local config (no class-field abort controller) with a per-entry-point throwIfAborted guard that rejects before any network I/O. - eslint-suppressions.json: one stale suppression entry pruned (kimi-code.spec.ts @typescript-eslint/no-explicit-any 1 -> 0 - the spec rewrite removed the only as-any cast); no suppression count increased. This branch is STACKED on open PR #1288: the foundation commit e61feb13e (generic RequestConfigBuilder, mergeAbortSignalAndTimeout, mergeAbortSignals, throwIfAborted) rides inside by design. --- .../base-openai-compatible-provider.spec.ts | 140 +++++++++- src/api/providers/__tests__/kimi-code.spec.ts | 114 +++++++- src/api/providers/__tests__/openai.spec.ts | 245 +++++++++++++++++- src/api/providers/__tests__/zai.spec.ts | 200 +++++++++++++- .../base-openai-compatible-provider.ts | 56 +++- src/api/providers/kimi-code.ts | 10 +- src/api/providers/openai.ts | 93 ++++++- src/api/providers/zai.ts | 36 ++- src/eslint-suppressions.json | 5 - 9 files changed, 852 insertions(+), 47 deletions(-) diff --git a/src/api/providers/__tests__/base-openai-compatible-provider.spec.ts b/src/api/providers/__tests__/base-openai-compatible-provider.spec.ts index fa7c19c5ed..d2a2a40b94 100644 --- a/src/api/providers/__tests__/base-openai-compatible-provider.spec.ts +++ b/src/api/providers/__tests__/base-openai-compatible-provider.spec.ts @@ -1,7 +1,7 @@ // npx vitest run api/providers/__tests__/base-openai-compatible-provider.spec.ts import { Anthropic } from "@anthropic-ai/sdk" -import OpenAI from "openai" +import OpenAI, { APIUserAbortError } from "openai" import type { ModelInfo } from "@roo-code/types" @@ -14,6 +14,8 @@ const mockCreate = vi.fn() // Mock OpenAI module vi.mock("openai", () => ({ + // Named export consumed by the provider for abort-error normalization + APIUserAbortError: class extends Error {}, default: vi.fn(function () { return { chat: { @@ -49,6 +51,20 @@ class TestOpenAiCompatibleProvider extends BaseOpenAiCompatibleProvider<"test-mo } } +/** + * Captures the rejection of an operation as an Error. The abort contract always + * throws an Error; the guard keeps strict typing without a cast. Fails the test + * if the operation resolves. + */ +async function captureError(operation: Promise): Promise { + try { + await operation + } catch (error) { + return error instanceof Error ? error : new Error(String(error)) + } + throw new Error("Expected the operation to reject") +} + describe("BaseOpenAiCompatibleProvider", () => { let handler: TestOpenAiCompatibleProvider @@ -278,6 +294,128 @@ describe("BaseOpenAiCompatibleProvider", () => { }) }) + describe("abort signal wiring", () => { + it("should pass the metadata abort signal to the client request", async () => { + const controller = new AbortController() + mockCreate.mockImplementationOnce(() => asyncStreamFrom([])) + + const stream = handler.createMessage("system prompt", [], { + taskId: "test-task", + abortSignal: controller.signal, + }) + await stream.next() + + expect(mockCreate).toHaveBeenCalledWith(expect.objectContaining({ model: "test-model" }), { + signal: controller.signal, + }) + }) + + it("should reject before issuing any request when the abort signal is already aborted", async () => { + const controller = new AbortController() + controller.abort() + + await expect(async () => { + for await (const _ of handler.createMessage("system prompt", [], { + taskId: "test-task", + abortSignal: controller.signal, + })) { + // consume + } + }).rejects.toMatchObject({ name: "AbortError", message: "This operation was aborted" }) + expect(mockCreate).not.toHaveBeenCalled() + }) + + it("should normalize the SDK APIUserAbortError from the stream path into the abort contract", async () => { + mockCreate.mockImplementationOnce(() => { + throw new APIUserAbortError() + }) + + const result = await captureError( + (async () => { + for await (const _ of handler.createMessage("system prompt", [])) { + // consume + } + })(), + ) + + // The SDK error has name "Error" and a message ending in a period; the + // provider must rethrow the Task.ts contract shape instead. + expect(result.name).toBe("AbortError") + expect(result.message).toBe("TestProvider request aborted") + expect(result.message.endsWith("aborted")).toBe(true) + }) + + it("should still wrap non-abort request errors with the provider prefix", async () => { + mockCreate.mockImplementationOnce(() => { + throw new Error("boom") + }) + + const result = await captureError( + (async () => { + for await (const _ of handler.createMessage("system prompt", [])) { + // consume + } + })(), + ) + + expect(result.message).toBe("TestProvider completion error: boom") + }) + + it("should pass the completePrompt abort signal to the client request", async () => { + const controller = new AbortController() + mockCreate.mockResolvedValueOnce({ choices: [{ message: { content: "response" } }] }) + + const result = await handler.completePrompt("test prompt", { abortSignal: controller.signal }) + + expect(result).toBe("response") + expect(mockCreate).toHaveBeenCalledWith(expect.objectContaining({ model: "test-model" }), { + signal: controller.signal, + }) + }) + + it("should merge completePrompt timeoutMs into the request signal", async () => { + mockCreate.mockResolvedValueOnce({ choices: [{ message: { content: "response" } }] }) + + await handler.completePrompt("test prompt", { timeoutMs: 5000 }) + + const requestOptions = mockCreate.mock.calls.at(-1)?.[1] + expect(requestOptions?.signal).toBeInstanceOf(AbortSignal) + expect(requestOptions?.signal.aborted).toBe(false) + }) + + it("should not set a request signal for zero completePrompt timeoutMs", async () => { + mockCreate.mockResolvedValueOnce({ choices: [{ message: { content: "response" } }] }) + + await handler.completePrompt("test prompt", { timeoutMs: 0 }) + + expect(mockCreate).toHaveBeenCalledWith(expect.objectContaining({ model: "test-model" }), undefined) + }) + + it("should reject before any request when the completePrompt signal is already aborted", async () => { + const controller = new AbortController() + controller.abort() + + await expect( + handler.completePrompt("test prompt", { abortSignal: controller.signal }), + ).rejects.toMatchObject({ + name: "AbortError", + message: "This operation was aborted", + }) + expect(mockCreate).not.toHaveBeenCalled() + }) + + it("should normalize the SDK APIUserAbortError from completePrompt", async () => { + mockCreate.mockImplementationOnce(() => { + throw new APIUserAbortError() + }) + + const result = await captureError(handler.completePrompt("test prompt")) + + expect(result.name).toBe("AbortError") + expect(result.message).toBe("TestProvider request aborted") + }) + }) + describe("Tool call handling", () => { it("should yield tool_call_end events when finish_reason is tool_calls", async () => { mockCreate.mockImplementationOnce(() => diff --git a/src/api/providers/__tests__/kimi-code.spec.ts b/src/api/providers/__tests__/kimi-code.spec.ts index df909d57d4..63bbb783bd 100644 --- a/src/api/providers/__tests__/kimi-code.spec.ts +++ b/src/api/providers/__tests__/kimi-code.spec.ts @@ -18,6 +18,32 @@ vi.mock("../../../integrations/kimi-code/oauth", () => ({ vi.mock("../fetchers/modelCache", () => ({ getModels: mockGetModels })) +/** + * Spies on the inherited OpenAI client's chat.completions.create. `client` is + * protected on the OpenAiHandler base (not on the public interface), so it is + * reached through a documented `as unknown as` double assertion (AGENTS.md + * last resort; no `as any`). + */ +function completionsCreate(handler: KimiCodeHandler): ReturnType { + const client = ( + handler as unknown as { + client: { chat: { completions: Record never> } } + } + ).client + return vi.spyOn(client.chat.completions, "create") as unknown as ReturnType +} + +/** Captures the rejection of an operation; fails the test if it resolves. */ +async function captureError(operation: Promise): Promise { + try { + await operation + } catch (error) { + // Abort normalization always throws an Error; the guard keeps strict typing without casts. + return error instanceof Error ? error : new Error(String(error)) + } + throw new Error("Expected the operation to reject") +} + describe("KimiCodeHandler", () => { beforeEach(() => { clearAllMocks() @@ -117,8 +143,7 @@ describe("KimiCodeHandler", () => { it("force-refreshes and retries exactly once after a non-streaming OAuth 401", async () => { const handler = new KimiCodeHandler({ kimiCodeAuthMethod: "oauth" }) const unauthorized = Object.assign(new Error("Unauthorized"), { status: 401 }) - const createCompletion = vi - .spyOn((handler as any).client.chat.completions, "create") + const createCompletion = completionsCreate(handler) .mockRejectedValueOnce(unauthorized) .mockResolvedValueOnce({ choices: [{ message: { content: "retried" } }] }) @@ -235,4 +260,89 @@ describe("KimiCodeHandler", () => { }) expect(handler.getModel().reasoning).toEqual({ reasoning_effort: "max" }) }) + + it("forwards the metadata abort signal to the inherited OpenAI SDK request", async () => { + const handler = new KimiCodeHandler({ kimiCodeAuthMethod: "api-key", kimiCodeApiKey: "key" }) + const controller = new AbortController() + const streamChunks = (async function* () { + yield { choices: [{ delta: { content: "hi" } }] } + })() + const createCompletion = completionsCreate(handler).mockResolvedValueOnce(streamChunks) + + const gen = handler.createMessage("system", [{ role: "user", content: "test" }], { + taskId: "test-task", + abortSignal: controller.signal, + }) + const first = await gen.next() + + expect(first.value).toEqual({ type: "text", text: "hi" }) + expect(createCompletion).toHaveBeenCalledWith(expect.anything(), { signal: controller.signal }) + }) + + it("rejects before any request when the createMessage abort signal is already aborted", async () => { + const handler = new KimiCodeHandler({ kimiCodeAuthMethod: "api-key", kimiCodeApiKey: "key" }) + const controller = new AbortController() + controller.abort() + const createCompletion = completionsCreate(handler) + + const gen = handler.createMessage("system", [{ role: "user", content: "test" }], { + taskId: "test-task", + abortSignal: controller.signal, + }) + + await expect(async () => { + for await (const _ of gen) { + // consume + } + }).rejects.toMatchObject({ name: "AbortError", message: "This operation was aborted" }) + expect(createCompletion).not.toHaveBeenCalled() + }) + + it("forwards completePrompt abort options through the override on both 401 retry attempts", async () => { + const handler = new KimiCodeHandler({ kimiCodeAuthMethod: "oauth" }) + const unauthorized = Object.assign(new Error("Unauthorized"), { status: 401 }) + const createCompletion = completionsCreate(handler) + .mockRejectedValueOnce(unauthorized) + .mockResolvedValueOnce({ choices: [{ message: { content: "retried" } }] }) + const controller = new AbortController() + + await expect(handler.completePrompt("test", { abortSignal: controller.signal })).resolves.toBe("retried") + expect(mockForceRefreshAccessToken).toHaveBeenCalledOnce() + expect(createCompletion).toHaveBeenCalledTimes(2) + for (const call of createCompletion.mock.calls) { + expect(call[1]).toEqual({ signal: controller.signal }) + } + }) + + it("rejects before any request when the completePrompt signal is already aborted", async () => { + const handler = new KimiCodeHandler({ kimiCodeAuthMethod: "api-key", kimiCodeApiKey: "key" }) + const controller = new AbortController() + controller.abort() + const createCompletion = completionsCreate(handler) + + await expect(handler.completePrompt("test", { abortSignal: controller.signal })).rejects.toMatchObject({ + name: "AbortError", + message: "This operation was aborted", + }) + expect(createCompletion).not.toHaveBeenCalled() + }) + + it("surfaces a normalized AbortError when the SDK aborts a streaming request", async () => { + const handler = new KimiCodeHandler({ kimiCodeAuthMethod: "api-key", kimiCodeApiKey: "key" }) + // The real SDK class: this spec does not mock the openai module. + const { APIUserAbortError } = await import("openai") + completionsCreate(handler).mockRejectedValueOnce(new APIUserAbortError()) + + const gen = handler.createMessage("system", [{ role: "user", content: "test" }], { taskId: "test-task" }) + const result = await captureError( + (async () => { + for await (const _ of gen) { + // consume + } + })(), + ) + + expect(result.name).toBe("AbortError") + expect(result.message).toBe("OpenAI request aborted") + }) }) diff --git a/src/api/providers/__tests__/openai.spec.ts b/src/api/providers/__tests__/openai.spec.ts index 38550533a5..67fe920818 100644 --- a/src/api/providers/__tests__/openai.spec.ts +++ b/src/api/providers/__tests__/openai.spec.ts @@ -3,7 +3,7 @@ import { OpenAiHandler, getOpenAiModels } from "../openai" import { ApiHandlerOptions } from "../../../shared/api" import { Anthropic } from "@anthropic-ai/sdk" -import OpenAI, { AzureOpenAI } from "openai" +import OpenAI, { AzureOpenAI, APIUserAbortError } from "openai" import { openAiModelInfoSaneDefaults, DEEP_SEEK_DEFAULT_TEMPERATURE, @@ -25,8 +25,10 @@ const mockCreate = vitest.fn() vitest.mock("openai", () => { const mockConstructor = vitest.fn() const mockAzureConstructor = vitest.fn() + const APIUserAbortError = class extends Error {} return { __esModule: true, + APIUserAbortError, default: mockConstructor.mockImplementation(function () { return { chat: { @@ -90,6 +92,20 @@ vitest.mock("axios", () => ({ }, })) +/** + * Captures the rejection of an operation as an Error. The abort contract always + * throws an Error; the guard keeps strict typing without a cast. Fails the test + * if the operation resolves. + */ +async function captureError(operation: Promise): Promise { + try { + await operation + } catch (error) { + return error instanceof Error ? error : new Error(String(error)) + } + throw new Error("Expected the operation to reject") +} + describe("OpenAiHandler", () => { let handler: OpenAiHandler let mockOptions: ApiHandlerOptions @@ -863,6 +879,233 @@ describe("OpenAiHandler", () => { }) }) + describe("abort signal wiring", () => { + it("should pass the metadata abort signal to the streaming createMessage request", async () => { + const controller = new AbortController() + const metadata = { taskId: "test-task", abortSignal: controller.signal } + const gen = handler.createMessage("system prompt", [], metadata) + const first = await gen.next() + expect(first.value).toEqual({ type: "text", text: "Test response" }) + expect(mockCreate).toHaveBeenCalledWith( + expect.objectContaining({ model: mockOptions.openAiModelId, stream: true }), + { signal: controller.signal }, + ) + }) + + it("should pass the metadata abort signal to the non-streaming createMessage request", async () => { + const nonStreamingHandler = new OpenAiHandler({ ...mockOptions, openAiStreamingEnabled: false }) + const controller = new AbortController() + const metadata = { taskId: "test-task", abortSignal: controller.signal } + const gen = nonStreamingHandler.createMessage("system prompt", [], metadata) + const first = await gen.next() + expect(first.value).toEqual({ type: "text", text: "Test response" }) + expect(mockCreate).toHaveBeenCalledWith(expect.objectContaining({ model: mockOptions.openAiModelId }), { + signal: controller.signal, + }) + }) + + it("should pass the metadata abort signal to the o3-family streaming request", async () => { + const o3Handler = new OpenAiHandler({ ...mockOptions, openAiModelId: "o3-mini" }) + const controller = new AbortController() + const metadata = { taskId: "test-task", abortSignal: controller.signal } + const gen = o3Handler.createMessage("system prompt", [], metadata) + const first = await gen.next() + expect(first.value).toEqual({ type: "text", text: "Test response" }) + expect(mockCreate).toHaveBeenCalledWith(expect.objectContaining({ model: "o3-mini", stream: true }), { + signal: controller.signal, + }) + }) + + it("should pass the metadata abort signal to the o3-family non-streaming request", async () => { + const o3Handler = new OpenAiHandler({ + ...mockOptions, + openAiModelId: "o3-mini", + openAiStreamingEnabled: false, + }) + const controller = new AbortController() + const metadata = { taskId: "test-task", abortSignal: controller.signal } + const gen = o3Handler.createMessage("system prompt", [], metadata) + const first = await gen.next() + expect(first.value).toEqual({ type: "text", text: "Test response" }) + expect(mockCreate).toHaveBeenCalledWith(expect.objectContaining({ model: "o3-mini" }), { + signal: controller.signal, + }) + }) + + it("should keep the Azure AI Inference path while passing the abort signal", async () => { + const azureHandler = new OpenAiHandler({ + ...mockOptions, + openAiBaseUrl: "https://test.services.ai.azure.com", + openAiModelId: "deepseek-v3", + }) + const controller = new AbortController() + const metadata = { taskId: "test-task", abortSignal: controller.signal } + const gen = azureHandler.createMessage("system prompt", [], metadata) + await gen.next() + expect(mockCreate).toHaveBeenCalledWith(expect.anything(), { + path: "/models/chat/completions", + signal: controller.signal, + }) + }) + + it("should pass the completePrompt abort signal to the request", async () => { + const controller = new AbortController() + mockCreate.mockResolvedValueOnce({ choices: [{ message: { content: "response" } }] }) + const result = await handler.completePrompt("test prompt", { abortSignal: controller.signal }) + expect(result).toBe("response") + expect(mockCreate).toHaveBeenCalledWith(expect.anything(), { signal: controller.signal }) + }) + + it("should merge the completePrompt abort signal and timeout into a single request signal", async () => { + const controller = new AbortController() + mockCreate.mockResolvedValueOnce({ choices: [{ message: { content: "response" } }] }) + await handler.completePrompt("test prompt", { abortSignal: controller.signal, timeoutMs: 5000 }) + const requestOptions = mockCreate.mock.calls.at(-1)?.[1] + expect(requestOptions?.signal).toBeInstanceOf(AbortSignal) + expect(requestOptions?.signal.aborted).toBe(false) + }) + + it("should pass a timeout-only request signal when completePrompt timeoutMs is positive", async () => { + mockCreate.mockResolvedValueOnce({ choices: [{ message: { content: "response" } }] }) + await handler.completePrompt("test prompt", { timeoutMs: 5000 }) + const requestOptions = mockCreate.mock.calls.at(-1)?.[1] + expect(requestOptions?.signal).toBeInstanceOf(AbortSignal) + }) + + it("should not pass a request signal for zero timeoutMs in completePrompt", async () => { + mockCreate.mockResolvedValueOnce({ choices: [{ message: { content: "response" } }] }) + await handler.completePrompt("test prompt", { timeoutMs: 0 }) + expect(mockCreate).toHaveBeenCalledWith(expect.anything(), {}) + }) + + it("should reject before issuing any request when the abort signal is already aborted", async () => { + const controller = new AbortController() + controller.abort() + const metadata = { taskId: "test-task", abortSignal: controller.signal } + await expect( + (async () => { + for await (const _ of handler.createMessage("system prompt", [], metadata)) { + // consume + } + })(), + ).rejects.toMatchObject({ name: "AbortError", message: "This operation was aborted" }) + expect(mockCreate).not.toHaveBeenCalled() + + mockCreate.mockClear() + await expect( + handler.completePrompt("test prompt", { abortSignal: controller.signal }), + ).rejects.toMatchObject({ + name: "AbortError", + message: "This operation was aborted", + }) + expect(mockCreate).not.toHaveBeenCalled() + }) + + it("should normalize the SDK APIUserAbortError from the streaming path into the abort contract", async () => { + const controller = new AbortController() + mockCreate.mockImplementationOnce(() => { + throw new APIUserAbortError() + }) + const metadata = { taskId: "test-task", abortSignal: controller.signal } + const result = await captureError( + (async () => { + for await (const _ of handler.createMessage("system prompt", [], metadata)) { + // consume + } + })(), + ) + // The SDK error has name "Error" and a message ending in a period; the + // provider must rethrow the Task.ts contract shape instead. + expect(result.name).toBe("AbortError") + expect(result.message).toBe("OpenAI request aborted") + expect(result.message.endsWith("aborted")).toBe(true) + }) + + it("should normalize the SDK APIUserAbortError from completePrompt without an external signal", async () => { + mockCreate.mockImplementationOnce(() => { + throw new APIUserAbortError() + }) + const result = await captureError(handler.completePrompt("test prompt")) + expect(result.name).toBe("AbortError") + expect(result.message).toBe("OpenAI request aborted") + }) + + it("should normalize a fetch-level AbortError from completePrompt into the abort contract", async () => { + const fetchAbort = new Error("The operation was aborted.") + fetchAbort.name = "AbortError" + mockCreate.mockImplementationOnce(() => { + throw fetchAbort + }) + const result = await captureError(handler.completePrompt("test prompt")) + expect(result.name).toBe("AbortError") + expect(result.message).toBe("OpenAI request aborted") + expect(result.cause).toBe(fetchAbort) + }) + + it("should normalize the SDK APIUserAbortError from the non-streaming createMessage path", async () => { + mockCreate.mockImplementationOnce(() => { + throw new APIUserAbortError() + }) + const noStreamHandler = new OpenAiHandler({ ...mockOptions, openAiStreamingEnabled: false }) + const result = await captureError( + (async () => { + for await (const _ of noStreamHandler.createMessage("system prompt", [])) { + // consume + } + })(), + ) + expect(result.name).toBe("AbortError") + expect(result.message).toBe("OpenAI request aborted") + }) + + it("should normalize the SDK APIUserAbortError from the o3-family streaming path", async () => { + mockCreate.mockImplementationOnce(() => { + throw new APIUserAbortError() + }) + const o3Handler = new OpenAiHandler({ ...mockOptions, openAiModelId: "o3-mini" }) + const result = await captureError( + (async () => { + for await (const _ of o3Handler.createMessage("system prompt", [])) { + // consume + } + })(), + ) + expect(result.name).toBe("AbortError") + expect(result.message).toBe("OpenAI request aborted") + }) + + it("should normalize the SDK APIUserAbortError from the o3-family non-streaming path", async () => { + mockCreate.mockImplementationOnce(() => { + throw new APIUserAbortError() + }) + const o3Handler = new OpenAiHandler({ + ...mockOptions, + openAiModelId: "o3-mini", + openAiStreamingEnabled: false, + }) + const result = await captureError( + (async () => { + for await (const _ of o3Handler.createMessage("system prompt", [])) { + // consume + } + })(), + ) + expect(result.name).toBe("AbortError") + expect(result.message).toBe("OpenAI request aborted") + }) + + it("should still wrap non-abort completion errors with the provider prefix", async () => { + mockCreate.mockImplementationOnce(() => { + throw new Error("boom") + }) + const result = await captureError(handler.completePrompt("test prompt")) + expect(result.name).toBe("Error") + // The inner catch already wraps with the provider prefix and the outer + // catch wraps again (pre-existing double-wrap of openai.ts completePrompt). + expect(result.message).toBe("OpenAI completion error: OpenAI completion error: boom") + }) + }) + describe("getModel", () => { it("should return model info with sane defaults", () => { const model = handler.getModel() diff --git a/src/api/providers/__tests__/zai.spec.ts b/src/api/providers/__tests__/zai.spec.ts index 0230b679a9..9ddbe40944 100644 --- a/src/api/providers/__tests__/zai.spec.ts +++ b/src/api/providers/__tests__/zai.spec.ts @@ -1,6 +1,6 @@ // npx vitest run src/api/providers/__tests__/zai.spec.ts -import OpenAI from "openai" +import OpenAI, { APIUserAbortError } from "openai" import { Anthropic } from "@anthropic-ai/sdk" import { @@ -21,12 +21,28 @@ import { clearAllMocks } from "../../../test-utils/reset" vitest.mock("openai", () => { const createMock = vitest.fn() return { + // Named export consumed by the provider for abort-error normalization + APIUserAbortError: class extends Error {}, default: vitest.fn(function () { return { chat: { completions: { create: createMock } } } }), } }) +/** + * Captures the rejection of an operation as an Error. The abort contract always + * throws an Error; the guard keeps strict typing without a cast. Fails the test + * if the operation resolves. + */ +async function captureError(operation: Promise): Promise { + try { + await operation + } catch (error) { + return error instanceof Error ? error : new Error(String(error)) + } + throw new Error("Expected the operation to reject") +} + describe("ZAiHandler", () => { let handler: ZAiHandler let mockCreate: any @@ -550,6 +566,159 @@ describe("ZAiHandler", () => { }) }) + describe("abort signal wiring", () => { + beforeEach(() => { + handler = new ZAiHandler({ zaiApiKey: "test-zai-api-key", zaiApiLine: "international_coding" }) + }) + + it("createMessage should pass the abort signal on the GLM thinking path", async () => { + const thinkingHandler = new ZAiHandler({ + apiModelId: "glm-4.7", + zaiApiKey: "test-zai-api-key", + zaiApiLine: "international_coding", + }) + const controller = new AbortController() + mockCreate.mockImplementationOnce(() => asyncStreamFrom([])) + + const gen = thinkingHandler.createMessage("system prompt", [], { + taskId: "test-task", + abortSignal: controller.signal, + }) + await gen.next() + + expect(mockCreate).toHaveBeenCalledWith( + expect.objectContaining({ model: "glm-4.7", thinking: { type: "enabled" } }), + { signal: controller.signal }, + ) + }) + + it("createMessage should pass the abort signal for non-thinking models via the base path", async () => { + const plainHandler = new ZAiHandler({ + apiModelId: "glm-4.6", + zaiApiKey: "test-zai-api-key", + zaiApiLine: "international_coding", + }) + const controller = new AbortController() + mockCreate.mockImplementationOnce(() => asyncStreamFrom([])) + + const gen = plainHandler.createMessage("system prompt", [], { + taskId: "test-task", + abortSignal: controller.signal, + }) + await gen.next() + + expect(mockCreate).toHaveBeenCalledWith(expect.objectContaining({ model: "glm-4.6" }), { + signal: controller.signal, + }) + }) + + it("createMessage should reject before any request when the abort signal is already aborted", async () => { + const controller = new AbortController() + controller.abort() + + await expect(async () => { + for await (const _ of handler.createMessage("system prompt", [], { + taskId: "test-task", + abortSignal: controller.signal, + })) { + // consume + } + }).rejects.toMatchObject({ name: "AbortError", message: "This operation was aborted" }) + expect(mockCreate).not.toHaveBeenCalled() + }) + + it("createMessage should normalize the SDK APIUserAbortError on the thinking path", async () => { + const thinkingHandler = new ZAiHandler({ + apiModelId: "glm-4.7", + zaiApiKey: "test-zai-api-key", + zaiApiLine: "international_coding", + }) + mockCreate.mockImplementationOnce(() => { + throw new APIUserAbortError() + }) + + const result = await captureError( + (async () => { + for await (const _ of thinkingHandler.createMessage("system prompt", [])) { + // consume + } + })(), + ) + + expect(result.name).toBe("AbortError") + expect(result.message).toBe("Z.ai request aborted") + expect(result.message.endsWith("aborted")).toBe(true) + }) + + it("completePrompt should pass the abort signal through to the client", async () => { + const controller = new AbortController() + mockCreate.mockResolvedValueOnce({ choices: [{ message: { content: "response" } }] }) + + const result = await handler.completePrompt("test prompt", { abortSignal: controller.signal }) + + expect(result).toBe("response") + expect(mockCreate).toHaveBeenCalledWith(expect.anything(), { signal: controller.signal }) + }) + + it("completePrompt should reject before any request when the signal is already aborted", async () => { + const controller = new AbortController() + controller.abort() + + await expect( + handler.completePrompt("test prompt", { abortSignal: controller.signal }), + ).rejects.toMatchObject({ + name: "AbortError", + message: "This operation was aborted", + }) + expect(mockCreate).not.toHaveBeenCalled() + }) + + it("completePrompt should normalize the SDK APIUserAbortError", async () => { + mockCreate.mockImplementationOnce(() => { + throw new APIUserAbortError() + }) + + const result = await captureError(handler.completePrompt("test prompt")) + + expect(result.name).toBe("AbortError") + expect(result.message).toBe("Z.ai request aborted") + }) + + it("glm-5.3 completePrompt should pass the abort signal on the thinking path", async () => { + const h53 = new ZAiHandler({ + apiModelId: "glm-5.3", + zaiApiKey: "test-zai-api-key", + zaiApiLine: "international_coding", + }) + const controller = new AbortController() + mockCreate.mockResolvedValueOnce({ choices: [{ message: { content: "response" } }] }) + + const result = await h53.completePrompt("prompt", { abortSignal: controller.signal }) + + expect(result).toBe("response") + expect(mockCreate).toHaveBeenCalledWith( + expect.objectContaining({ model: "glm-5.3", thinking: { type: "enabled", clear_thinking: false } }), + { signal: controller.signal }, + ) + }) + + it("glm-5.3 completePrompt should normalize the SDK APIUserAbortError", async () => { + const h53 = new ZAiHandler({ + apiModelId: "glm-5.3", + zaiApiKey: "test-zai-api-key", + zaiApiLine: "international_coding", + }) + mockCreate.mockImplementationOnce(() => { + throw new APIUserAbortError() + }) + + const result = await captureError(h53.completePrompt("prompt")) + + expect(result.name).toBe("AbortError") + expect(result.message).toBe("Z.ai request aborted") + }) + }) + describe("GLM-4.7 Thinking Mode", () => { it("should cap GLM-5.1 max_tokens to 20% of context window by default", async () => { const handlerWithModel = new ZAiHandler({ @@ -568,6 +737,7 @@ describe("ZAiHandler", () => { model: "glm-5.1", max_tokens: 40_000, }), + undefined, ) }) @@ -600,6 +770,7 @@ describe("ZAiHandler", () => { model: "glm-5.1", max_tokens: 100_000, }), + undefined, ) }) @@ -622,6 +793,7 @@ describe("ZAiHandler", () => { model: "glm-4.7", thinking: { type: "enabled" }, }), + undefined, ) }) @@ -644,6 +816,7 @@ describe("ZAiHandler", () => { thinking: { type: "enabled" }, reasoning_effort: "high", }), + undefined, ) }) @@ -666,6 +839,7 @@ describe("ZAiHandler", () => { thinking: { type: "enabled" }, reasoning_effort: "max", }), + undefined, ) }) @@ -689,6 +863,7 @@ describe("ZAiHandler", () => { reasoning_effort: "max", temperature: 1, }), + undefined, ) }) @@ -711,6 +886,7 @@ describe("ZAiHandler", () => { thinking: { type: "enabled", clear_thinking: false }, reasoning_effort: "max", }), + undefined, ) }) @@ -725,13 +901,16 @@ describe("ZAiHandler", () => { mockCreate.mockResolvedValueOnce({ choices: [{ message: { content: "response" } }] }) await expect(handlerWithModel.completePrompt("prompt")).resolves.toBe("response") - expect(mockCreate).toHaveBeenCalledWith({ - model: "glm-5.3", - messages: [{ role: "user", content: "prompt" }], - temperature: 1, - thinking: { type: "enabled", clear_thinking: false }, - reasoning_effort: "low", - }) + expect(mockCreate).toHaveBeenCalledWith( + { + model: "glm-5.3", + messages: [{ role: "user", content: "prompt" }], + temperature: 1, + thinking: { type: "enabled", clear_thinking: false }, + reasoning_effort: "low", + }, + undefined, + ) }) it("should omit reasoning_effort for GLM-5.2 when reasoningEffort is set to disable", async () => { @@ -771,6 +950,7 @@ describe("ZAiHandler", () => { thinking: { type: "enabled" }, reasoning_effort: "high", }), + undefined, ) }) @@ -794,6 +974,7 @@ describe("ZAiHandler", () => { model: "glm-4.7", thinking: { type: "disabled" }, }), + undefined, ) }) @@ -817,6 +998,7 @@ describe("ZAiHandler", () => { model: "glm-4.7", thinking: { type: "enabled" }, }), + undefined, ) }) @@ -854,6 +1036,7 @@ describe("ZAiHandler", () => { model: "glm-5-turbo", thinking: { type: "enabled" }, }), + undefined, ) }) @@ -876,6 +1059,7 @@ describe("ZAiHandler", () => { model: "glm-5-turbo", thinking: { type: "disabled" }, }), + undefined, ) }) }) diff --git a/src/api/providers/base-openai-compatible-provider.ts b/src/api/providers/base-openai-compatible-provider.ts index f4928b0b0a..344ccfa38e 100644 --- a/src/api/providers/base-openai-compatible-provider.ts +++ b/src/api/providers/base-openai-compatible-provider.ts @@ -1,5 +1,5 @@ import { Anthropic } from "@anthropic-ai/sdk" -import OpenAI from "openai" +import OpenAI, { APIUserAbortError } from "openai" import type { ModelInfo } from "@roo-code/types" @@ -14,6 +14,8 @@ import { BaseProvider } from "./base-provider" import { handleOpenAIError } from "./utils/error-handler" import { calculateApiCostOpenAI } from "../../shared/cost" import { extractReasoningFromDelta } from "./utils/extract-reasoning" +import { RequestConfigBuilder } from "./config-builder/request-config-builder" +import { mergeAbortSignalAndTimeout, throwIfAborted } from "./utils/abort-signal" type BaseOpenAiCompatibleProviderOptions = ApiHandlerOptions & { providerName: string @@ -23,6 +25,32 @@ type BaseOpenAiCompatibleProviderOptions = ApiHandlerO defaultTemperature?: number } +/** Subset of OpenAI.RequestOptions built per request for the abort-signal wiring. */ +type OpenAiRequestConfig = { + signal?: AbortSignal +} + +/** + * Handles errors from OpenAI Node SDK request sites with abort awareness. + * + * An abort failure (the external signal already aborted, the SDK's + * APIUserAbortError, or a fetch-level AbortError) is normalized to a fresh + * Error with name "AbortError" and a message ending in "aborted" (the Task.ts + * contract) instead of being wrapped as a regular completion error, which a + * plain rethrow of the SDK abort error would produce. + */ +export function handleOpenAIRequestError(error: unknown, providerName: string, abortSignal?: AbortSignal): Error { + if ( + abortSignal?.aborted || + (error instanceof Error && (error.name === "AbortError" || error instanceof APIUserAbortError)) + ) { + const aborted = new Error(`${providerName} request aborted`, { cause: error }) + aborted.name = "AbortError" + return aborted + } + return handleOpenAIError(error, providerName) +} + export abstract class BaseOpenAiCompatibleProvider extends BaseProvider implements SingleCompletionHandler @@ -67,7 +95,7 @@ export abstract class BaseOpenAiCompatibleProvider }) } - protected createStream( + protected async createStream( systemPrompt: string, messages: Anthropic.Messages.MessageParam[], metadata?: ApiHandlerCreateMessageMetadata, @@ -104,9 +132,9 @@ export abstract class BaseOpenAiCompatibleProvider } try { - return this.client.chat.completions.create(params, requestOptions) + return await this.client.chat.completions.create(params, requestOptions) } catch (error) { - throw handleOpenAIError(error, this.providerName) + throw handleOpenAIRequestError(error, this.providerName, metadata?.abortSignal) } } @@ -115,7 +143,12 @@ export abstract class BaseOpenAiCompatibleProvider messages: Anthropic.Messages.MessageParam[], metadata?: ApiHandlerCreateMessageMetadata, ): ApiStream { - const stream = await this.createStream(systemPrompt, messages, metadata) + throwIfAborted(metadata?.abortSignal) + + // Per-request abort wiring (RequestConfigBuilder adoption): subclasses inherit + // it by receiving the built config as createStream's requestOptions. + const requestConfig = new RequestConfigBuilder().setAbortSignal(metadata).build() + const stream = await this.createStream(systemPrompt, messages, metadata, requestConfig) const matcher = new TagMatcher( ["think", "thought"], @@ -213,6 +246,8 @@ export abstract class BaseOpenAiCompatibleProvider } async completePrompt(prompt: string, options?: CompletePromptOptions): Promise { + throwIfAborted(options?.abortSignal) + const { id: modelId, info: modelInfo } = this.getModel() const params: OpenAI.Chat.Completions.ChatCompletionCreateParams = { @@ -225,8 +260,15 @@ export abstract class BaseOpenAiCompatibleProvider ;(params as any).thinking = { type: "enabled" } } + // CompletePromptOptions is not ApiHandlerCreateMessageMetadata (required taskId), + // so the abort/timeout merge goes through setOption; the helper treats + // timeoutMs <= 0 as "no explicit timeout". + const requestConfig = new RequestConfigBuilder() + .setOption("signal", mergeAbortSignalAndTimeout(options?.abortSignal, options?.timeoutMs)) + .build() + try { - const response = await this.client.chat.completions.create(params) + const response = await this.client.chat.completions.create(params, requestConfig) // Check for provider-specific error responses (e.g., MiniMax base_resp) const responseAny = response as any @@ -238,7 +280,7 @@ export abstract class BaseOpenAiCompatibleProvider return response.choices?.[0]?.message.content || "" } catch (error) { - throw handleOpenAIError(error, this.providerName) + throw handleOpenAIRequestError(error, this.providerName, options?.abortSignal) } } diff --git a/src/api/providers/kimi-code.ts b/src/api/providers/kimi-code.ts index 3f7136806b..a81efb488a 100644 --- a/src/api/providers/kimi-code.ts +++ b/src/api/providers/kimi-code.ts @@ -13,7 +13,7 @@ import { import type { ApiHandlerOptions } from "../../shared/api" import { kimiCodeOAuthManager } from "../../integrations/kimi-code/oauth" -import type { ApiHandlerCreateMessageMetadata } from "../index" +import type { ApiHandlerCreateMessageMetadata, CompletePromptOptions } from "../index" import type { ApiStream } from "../transform/stream" import { getModelParams } from "../transform/model-params" @@ -98,14 +98,16 @@ export class KimiCodeHandler extends OpenAiHandler { } } - override async completePrompt(prompt: string): Promise { + override async completePrompt(prompt: string, options?: CompletePromptOptions): Promise { await this.prepareRequest() try { - return await super.completePrompt(prompt) + // Forward abort/timeout options so the inherited OpenAiHandler wiring + // applies (the createMessage override inherits the same via metadata). + return await super.completePrompt(prompt, options) } catch (error) { if (getHttpStatus(error) !== 401 || !this.canRefreshOAuth()) throw error await this.prepareRequest(true) - return super.completePrompt(prompt) + return super.completePrompt(prompt, options) } } diff --git a/src/api/providers/openai.ts b/src/api/providers/openai.ts index 5588dd37d6..0f28a888e5 100644 --- a/src/api/providers/openai.ts +++ b/src/api/providers/openai.ts @@ -1,5 +1,5 @@ import { Anthropic } from "@anthropic-ai/sdk" -import OpenAI, { AzureOpenAI } from "openai" +import OpenAI, { AzureOpenAI, APIUserAbortError } from "openai" import axios from "axios" import { @@ -26,6 +26,35 @@ import { BaseProvider } from "./base-provider" import type { SingleCompletionHandler, ApiHandlerCreateMessageMetadata, CompletePromptOptions } from "../index" import { handleOpenAIError } from "./utils/error-handler" import { extractReasoningFromDelta } from "./utils/extract-reasoning" +import { RequestConfigBuilder } from "./config-builder/request-config-builder" +import { mergeAbortSignalAndTimeout, throwIfAborted } from "./utils/abort-signal" + +/** Subset of OpenAI.RequestOptions built per request for the abort-signal wiring. */ +type OpenAiRequestConfig = { + path?: string + signal?: AbortSignal +} + +/** + * Handles errors from OpenAI Node SDK request sites with abort awareness. + * + * An abort failure (the external signal already aborted, the SDK's + * APIUserAbortError, or a fetch-level AbortError) is normalized to a fresh + * Error with name "AbortError" and a message ending in "aborted" (the Task.ts + * contract) instead of being wrapped as a regular completion error, which a + * plain rethrow of the SDK abort error would produce. + */ +function handleOpenAIRequestError(error: unknown, providerName: string, abortSignal?: AbortSignal): Error { + if ( + abortSignal?.aborted || + (error instanceof Error && (error.name === "AbortError" || error instanceof APIUserAbortError)) + ) { + const aborted = new Error(`${providerName} request aborted`, { cause: error }) + aborted.name = "AbortError" + return aborted + } + return handleOpenAIError(error, providerName) +} // TODO: Rename this to OpenAICompatibleHandler. Also, I think the // `OpenAINativeHandler` can subclass from this, since it's obviously @@ -84,6 +113,8 @@ export class OpenAiHandler extends BaseProvider implements SingleCompletionHandl messages: Anthropic.Messages.MessageParam[], metadata?: ApiHandlerCreateMessageMetadata, ): ApiStream { + throwIfAborted(metadata?.abortSignal) + const { info: modelInfo, reasoning } = this.getModel() const modelUrl = this.options.openAiBaseUrl ?? "" const modelId = this.options.openAiModelId ?? "" @@ -179,10 +210,10 @@ export class OpenAiHandler extends BaseProvider implements SingleCompletionHandl try { stream = await this.client.chat.completions.create( requestOptions, - isAzureAiInference ? { path: OPENAI_AZURE_AI_INFERENCE_PATH } : {}, + this.buildChatRequestConfig(isAzureAiInference, metadata), ) } catch (error) { - throw handleOpenAIError(error, this.providerName) + throw handleOpenAIRequestError(error, this.providerName, metadata?.abortSignal) } const matcher = new TagMatcher( @@ -245,10 +276,10 @@ export class OpenAiHandler extends BaseProvider implements SingleCompletionHandl try { response = await this.client.chat.completions.create( requestOptions, - this._isAzureAiInference(modelUrl) ? { path: OPENAI_AZURE_AI_INFERENCE_PATH } : {}, + this.buildChatRequestConfig(this._isAzureAiInference(modelUrl), metadata), ) } catch (error) { - throw handleOpenAIError(error, this.providerName) + throw handleOpenAIRequestError(error, this.providerName, metadata?.abortSignal) } const message = response.choices?.[0]?.message @@ -299,6 +330,8 @@ export class OpenAiHandler extends BaseProvider implements SingleCompletionHandl } async completePrompt(prompt: string, options?: CompletePromptOptions): Promise { + throwIfAborted(options?.abortSignal) + try { const isAzureAiInference = this._isAzureAiInference(this.options.openAiBaseUrl) const model = this.getModel() @@ -316,14 +349,18 @@ export class OpenAiHandler extends BaseProvider implements SingleCompletionHandl try { response = await this.client.chat.completions.create( requestOptions, - isAzureAiInference ? { path: OPENAI_AZURE_AI_INFERENCE_PATH } : {}, + this.buildCompletePromptRequestConfig(isAzureAiInference, options), ) } catch (error) { - throw handleOpenAIError(error, this.providerName) + throw handleOpenAIRequestError(error, this.providerName, options?.abortSignal) } return response.choices?.[0]?.message.content || "" } catch (error) { + if (error instanceof Error && error.name === "AbortError") { + // Preserve the normalized abort error (name + message contract) as-is. + throw error + } if (error instanceof Error) { const wrapped = new Error(`${this.providerName} completion error: ${error.message}`, { cause: error }) const source = error as Error & { status?: number; errorDetails?: unknown; code?: unknown } @@ -338,6 +375,40 @@ export class OpenAiHandler extends BaseProvider implements SingleCompletionHandl } } + /** + * Builds the per-request OpenAI SDK options for a chat completions create + * (createMessage paths): the Azure AI Inference path when applicable, plus + * the caller's abort signal, via RequestConfigBuilder adoption. + */ + private buildChatRequestConfig( + isAzureAiInference: boolean, + metadata?: ApiHandlerCreateMessageMetadata, + ): OpenAI.RequestOptions { + return ( + new RequestConfigBuilder() + .setOption("path", isAzureAiInference ? OPENAI_AZURE_AI_INFERENCE_PATH : undefined) + .setAbortSignal(metadata) + .build() ?? {} + ) + } + + /** + * Builds the per-request options for completePrompt. CompletePromptOptions + * is not ApiHandlerCreateMessageMetadata (required taskId), so the merged + * abort/timeout signal goes through setOption instead of setAbortSignal. + */ + private buildCompletePromptRequestConfig( + isAzureAiInference: boolean, + options?: CompletePromptOptions, + ): OpenAI.RequestOptions { + return ( + new RequestConfigBuilder() + .setOption("path", isAzureAiInference ? OPENAI_AZURE_AI_INFERENCE_PATH : undefined) + .setOption("signal", mergeAbortSignalAndTimeout(options?.abortSignal, options?.timeoutMs)) + .build() ?? {} + ) + } + private async *handleO3FamilyMessage( modelId: string, systemPrompt: string, @@ -378,10 +449,10 @@ export class OpenAiHandler extends BaseProvider implements SingleCompletionHandl try { stream = await this.client.chat.completions.create( requestOptions, - methodIsAzureAiInference ? { path: OPENAI_AZURE_AI_INFERENCE_PATH } : {}, + this.buildChatRequestConfig(methodIsAzureAiInference, metadata), ) } catch (error) { - throw handleOpenAIError(error, this.providerName) + throw handleOpenAIRequestError(error, this.providerName, metadata?.abortSignal) } yield* this.handleStreamResponse(stream) @@ -412,10 +483,10 @@ export class OpenAiHandler extends BaseProvider implements SingleCompletionHandl try { response = await this.client.chat.completions.create( requestOptions, - methodIsAzureAiInference ? { path: OPENAI_AZURE_AI_INFERENCE_PATH } : {}, + this.buildChatRequestConfig(methodIsAzureAiInference, metadata), ) } catch (error) { - throw handleOpenAIError(error, this.providerName) + throw handleOpenAIRequestError(error, this.providerName, metadata?.abortSignal) } const message = response.choices?.[0]?.message diff --git a/src/api/providers/zai.ts b/src/api/providers/zai.ts index c53a434e38..f286d26f9b 100644 --- a/src/api/providers/zai.ts +++ b/src/api/providers/zai.ts @@ -14,9 +14,10 @@ import { type ApiHandlerOptions, getModelMaxOutputTokens } from "../../shared/ap import { convertToZAiFormat } from "../transform/zai-format" import type { ApiHandlerCreateMessageMetadata, CompletePromptOptions } from "../index" -import { BaseOpenAiCompatibleProvider } from "./base-openai-compatible-provider" +import { BaseOpenAiCompatibleProvider, handleOpenAIRequestError } from "./base-openai-compatible-provider" import { NOT_PROVIDED } from "./constants" -import { handleOpenAIError } from "./utils/error-handler" +import { RequestConfigBuilder } from "./config-builder/request-config-builder" +import { mergeAbortSignalAndTimeout, throwIfAborted } from "./utils/abort-signal" // Custom interface for Z.ai params to support thinking mode and reasoning effort tiers. // Z.ai accepts the standard `reasoning_effort` ladder (none/minimal/low/medium/high/xhigh/max) @@ -27,6 +28,11 @@ type ZAiChatCompletionParams = Omit { constructor(options: ApiHandlerOptions) { const apiLine = options.zaiApiLine ?? "international_coding" @@ -62,8 +68,10 @@ export class ZAiHandler extends BaseOpenAiCompatibleProvider { const isThinkingModel = Array.isArray(info.supportsReasoningEffort) if (isThinkingModel) { - // Create the stream with our custom thinking parameter - return this.createStreamWithThinking(systemPrompt, messages, metadata) + // Create the stream with our custom thinking parameter. + // Forward the per-request options (abort signal) so the thinking path + // keeps them instead of dropping requestOptions. + return this.createStreamWithThinking(systemPrompt, messages, metadata, requestOptions) } // For non-thinking models, use the default behavior @@ -73,10 +81,11 @@ export class ZAiHandler extends BaseOpenAiCompatibleProvider { /** * Creates a stream with explicit thinking control for GLM thinking-capable models. */ - private createStreamWithThinking( + private async createStreamWithThinking( systemPrompt: string, messages: Anthropic.Messages.MessageParam[], metadata?: ApiHandlerCreateMessageMetadata, + requestOptions?: OpenAI.RequestOptions, ) { const { id: model, info } = this.getModel() const { reasoningEffort, useReasoning } = this.getReasoningSettings(info) @@ -114,11 +123,12 @@ export class ZAiHandler extends BaseOpenAiCompatibleProvider { } try { - return this.client.chat.completions.create( + return await this.client.chat.completions.create( params as OpenAI.Chat.Completions.ChatCompletionCreateParamsStreaming, + requestOptions, ) } catch (error) { - throw handleOpenAIError(error, this.providerName) + throw handleOpenAIRequestError(error, this.providerName, metadata?.abortSignal) } } @@ -147,6 +157,8 @@ export class ZAiHandler extends BaseOpenAiCompatibleProvider { return super.completePrompt(prompt, options) } + throwIfAborted(options?.abortSignal) + const { reasoningEffort } = this.getReasoningSettings(info) const params: ZAiChatCompletionParams = { model, @@ -156,13 +168,21 @@ export class ZAiHandler extends BaseOpenAiCompatibleProvider { reasoning_effort: reasoningEffort, } + // CompletePromptOptions is not ApiHandlerCreateMessageMetadata (required + // taskId), so the abort/timeout merge goes through setOption; the helper + // treats timeoutMs <= 0 as "no explicit timeout". + const requestConfig = new RequestConfigBuilder() + .setOption("signal", mergeAbortSignalAndTimeout(options?.abortSignal, options?.timeoutMs)) + .build() + try { const response = await this.client.chat.completions.create( params as OpenAI.Chat.Completions.ChatCompletionCreateParamsNonStreaming, + requestConfig, ) return response.choices?.[0]?.message.content || "" } catch (error) { - throw handleOpenAIError(error, this.providerName) + throw handleOpenAIRequestError(error, this.providerName, options?.abortSignal) } } } diff --git a/src/eslint-suppressions.json b/src/eslint-suppressions.json index f790fba436..430be7ca31 100644 --- a/src/eslint-suppressions.json +++ b/src/eslint-suppressions.json @@ -164,11 +164,6 @@ "count": 1 } }, - "api/providers/__tests__/kimi-code.spec.ts": { - "@typescript-eslint/no-explicit-any": { - "count": 1 - } - }, "api/providers/__tests__/lite-llm.spec.ts": { "@typescript-eslint/no-explicit-any": { "count": 36 From 4a3169281cc0cf49a2cbb36fc1fc40b18f65b02a Mon Sep 17 00:00:00 2001 From: easonLiangWorldedtech Date: Fri, 21 Aug 2026 06:07:17 +0800 Subject: [PATCH 03/11] test(providers): export APIUserAbortError from openai mock in fireworks and sambanova specs Root cause: the abort-aware completePrompt error path inherited by fireworks and sambanova (base-openai-compatible-provider.ts) references the APIUserAbortError export of the openai SDK, which their specs' partial vi.mock("openai", ...) factories did not define, so the completePrompt error-path tests failed in the CI full suite with 'No "APIUserAbortError" export is defined on the "openai" mock'. The mocks now export APIUserAbortError using the same shape as the other series specs (base-openai-compatible-provider, zai, openai, kimi-code). --- src/api/providers/__tests__/fireworks.spec.ts | 2 ++ src/api/providers/__tests__/sambanova.spec.ts | 2 ++ 2 files changed, 4 insertions(+) diff --git a/src/api/providers/__tests__/fireworks.spec.ts b/src/api/providers/__tests__/fireworks.spec.ts index bde144591d..ab543252d9 100644 --- a/src/api/providers/__tests__/fireworks.spec.ts +++ b/src/api/providers/__tests__/fireworks.spec.ts @@ -23,6 +23,8 @@ vi.mock("openai", () => ({ }, } }), + // Named export consumed by the provider for abort-error normalization + APIUserAbortError: class extends Error {}, })) describe("FireworksHandler", () => { diff --git a/src/api/providers/__tests__/sambanova.spec.ts b/src/api/providers/__tests__/sambanova.spec.ts index 2a19d5659c..18636f9c4f 100644 --- a/src/api/providers/__tests__/sambanova.spec.ts +++ b/src/api/providers/__tests__/sambanova.spec.ts @@ -15,6 +15,8 @@ vitest.mock("openai", () => { default: vitest.fn(function () { return { chat: { completions: { create: createMock } } } }), + // Named export consumed by the provider for abort-error normalization + APIUserAbortError: class extends Error {}, } }) From 35c95ea19e902f6b1992beda46631909ea2a3398 Mon Sep 17 00:00:00 2001 From: easonLiangWorldedtech Date: Fri, 21 Aug 2026 06:37:22 +0800 Subject: [PATCH 04/11] fix(api): normalize abort errors raised during stream iteration Root cause: the creation-site catches only cover chat.completions.create; an abort that surfaces while the async iterator is being consumed (APIUserAbortError / fetch-level AbortError thrown mid-stream) leaked as the raw SDK error, which violates the Task.ts abort contract (an Error whose name is "AbortError" and whose message ends in "aborted"). The stream iteration is now wrapped and normalized through the same abort-aware handleOpenAIRequestError used at the creation sites: - base-openai-compatible-provider.ts: the createMessage for-await loop - openai.ts: the streaming createMessage for-await loop - openai.ts: the o3-family yield* this.handleStreamResponse(stream) The Z.ai thinking path inherits the base createMessage iteration, so it is covered by the base-provider fix. Non-abort iteration errors keep the existing provider-prefix wrap. Adds four regression tests (base, openai streaming, o3-family streaming, zai thinking path) with iterators that reject with APIUserAbortError after yielding the first chunk. Addresses the CodeRabbit pre-merge review comment on PR #1311. --- .../base-openai-compatible-provider.spec.ts | 23 +++++ src/api/providers/__tests__/openai.spec.ts | 46 ++++++++++ src/api/providers/__tests__/zai.spec.ts | 28 ++++++ .../base-openai-compatible-provider.ts | 86 ++++++++++--------- src/api/providers/openai.ts | 42 +++++---- 5 files changed, 170 insertions(+), 55 deletions(-) diff --git a/src/api/providers/__tests__/base-openai-compatible-provider.spec.ts b/src/api/providers/__tests__/base-openai-compatible-provider.spec.ts index d2a2a40b94..a6adeb53e5 100644 --- a/src/api/providers/__tests__/base-openai-compatible-provider.spec.ts +++ b/src/api/providers/__tests__/base-openai-compatible-provider.spec.ts @@ -345,6 +345,29 @@ describe("BaseOpenAiCompatibleProvider", () => { expect(result.message.endsWith("aborted")).toBe(true) }) + it("should normalize an abort error raised during stream iteration into the abort contract", async () => { + mockCreate.mockImplementationOnce(() => + (async function* () { + yield { choices: [{ delta: { content: "partial" } }] } + throw new APIUserAbortError() + })(), + ) + + const result = await captureError( + (async () => { + for await (const _ of handler.createMessage("system prompt", [])) { + // consume + } + })(), + ) + + // The iterator rejects after the first chunk; the provider must normalize + // it to the Task.ts contract shape (name + message ending in "aborted"). + expect(result.name).toBe("AbortError") + expect(result.message).toBe("TestProvider request aborted") + expect(result.message.endsWith("aborted")).toBe(true) + }) + it("should still wrap non-abort request errors with the provider prefix", async () => { mockCreate.mockImplementationOnce(() => { throw new Error("boom") diff --git a/src/api/providers/__tests__/openai.spec.ts b/src/api/providers/__tests__/openai.spec.ts index 67fe920818..e62d3ab6a6 100644 --- a/src/api/providers/__tests__/openai.spec.ts +++ b/src/api/providers/__tests__/openai.spec.ts @@ -1021,6 +1021,30 @@ describe("OpenAiHandler", () => { expect(result.message.endsWith("aborted")).toBe(true) }) + it("should normalize an abort error raised during stream iteration into the abort contract", async () => { + const controller = new AbortController() + mockCreate.mockImplementationOnce(() => + (async function* () { + yield { choices: [{ delta: { content: "partial" } }] } + throw new APIUserAbortError() + })(), + ) + const metadata = { taskId: "test-task", abortSignal: controller.signal } + const result = await captureError( + (async () => { + for await (const _ of handler.createMessage("system prompt", [], metadata)) { + // consume + } + })(), + ) + + // The iterator rejects after the first chunk; the provider must normalize + // it to the Task.ts contract shape instead of leaking the raw SDK error. + expect(result.name).toBe("AbortError") + expect(result.message).toBe("OpenAI request aborted") + expect(result.message.endsWith("aborted")).toBe(true) + }) + it("should normalize the SDK APIUserAbortError from completePrompt without an external signal", async () => { mockCreate.mockImplementationOnce(() => { throw new APIUserAbortError() @@ -1074,6 +1098,28 @@ describe("OpenAiHandler", () => { expect(result.message).toBe("OpenAI request aborted") }) + it("should normalize an abort error raised during o3-family stream iteration into the abort contract", async () => { + mockCreate.mockImplementationOnce(() => + (async function* () { + yield { choices: [{ delta: { content: "partial" } }] } + throw new APIUserAbortError() + })(), + ) + const o3Handler = new OpenAiHandler({ ...mockOptions, openAiModelId: "o3-mini" }) + const result = await captureError( + (async () => { + for await (const _ of o3Handler.createMessage("system prompt", [])) { + // consume + } + })(), + ) + + // handleStreamResponse consumes the iterator; a mid-stream abort must be + // normalized to the Task.ts contract shape, not leak as the raw SDK error. + expect(result.name).toBe("AbortError") + expect(result.message).toBe("OpenAI request aborted") + }) + it("should normalize the SDK APIUserAbortError from the o3-family non-streaming path", async () => { mockCreate.mockImplementationOnce(() => { throw new APIUserAbortError() diff --git a/src/api/providers/__tests__/zai.spec.ts b/src/api/providers/__tests__/zai.spec.ts index 9ddbe40944..d4a390d4c6 100644 --- a/src/api/providers/__tests__/zai.spec.ts +++ b/src/api/providers/__tests__/zai.spec.ts @@ -650,6 +650,34 @@ describe("ZAiHandler", () => { expect(result.message.endsWith("aborted")).toBe(true) }) + it("createMessage should normalize an abort error raised during stream iteration on the thinking path", async () => { + const thinkingHandler = new ZAiHandler({ + apiModelId: "glm-4.7", + zaiApiKey: "test-zai-api-key", + zaiApiLine: "international_coding", + }) + mockCreate.mockImplementationOnce(() => + (async function* () { + yield { choices: [{ delta: { content: "partial" } }] } + throw new APIUserAbortError() + })(), + ) + + const result = await captureError( + (async () => { + for await (const _ of thinkingHandler.createMessage("system prompt", [])) { + // consume + } + })(), + ) + + // The thinking path inherits stream iteration from the base provider; a + // mid-stream abort must normalize to the Task.ts contract shape. + expect(result.name).toBe("AbortError") + expect(result.message).toBe("Z.ai request aborted") + expect(result.message.endsWith("aborted")).toBe(true) + }) + it("completePrompt should pass the abort signal through to the client", async () => { const controller = new AbortController() mockCreate.mockResolvedValueOnce({ choices: [{ message: { content: "response" } }] }) diff --git a/src/api/providers/base-openai-compatible-provider.ts b/src/api/providers/base-openai-compatible-provider.ts index 344ccfa38e..2f37b2101b 100644 --- a/src/api/providers/base-openai-compatible-provider.ts +++ b/src/api/providers/base-openai-compatible-provider.ts @@ -162,57 +162,63 @@ export abstract class BaseOpenAiCompatibleProvider let lastUsage: OpenAI.CompletionUsage | undefined const activeToolCallIds = new Set() - for await (const chunk of stream) { - // Check for provider-specific error responses (e.g., MiniMax base_resp) - const chunkAny = chunk as any - if (chunkAny.base_resp?.status_code && chunkAny.base_resp.status_code !== 0) { - throw new Error( - `${this.providerName} API Error (${chunkAny.base_resp.status_code}): ${chunkAny.base_resp.status_msg || "Unknown error"}`, - ) - } + try { + for await (const chunk of stream) { + // Check for provider-specific error responses (e.g., MiniMax base_resp) + const chunkAny = chunk as any + if (chunkAny.base_resp?.status_code && chunkAny.base_resp.status_code !== 0) { + throw new Error( + `${this.providerName} API Error (${chunkAny.base_resp.status_code}): ${chunkAny.base_resp.status_msg || "Unknown error"}`, + ) + } - const delta = chunk.choices?.[0]?.delta - const finishReason = chunk.choices?.[0]?.finish_reason + const delta = chunk.choices?.[0]?.delta + const finishReason = chunk.choices?.[0]?.finish_reason - if (delta?.content) { - for (const processedChunk of matcher.update(delta.content)) { - yield processedChunk + if (delta?.content) { + for (const processedChunk of matcher.update(delta.content)) { + yield processedChunk + } } - } - const reasoningText = extractReasoningFromDelta(delta) - if (reasoningText) { - yield { type: "reasoning", text: reasoningText } - } + const reasoningText = extractReasoningFromDelta(delta) + if (reasoningText) { + yield { type: "reasoning", text: reasoningText } + } - // Emit raw tool call chunks - NativeToolCallParser handles state management - if (delta?.tool_calls) { - for (const toolCall of delta.tool_calls) { - if (toolCall.id) { - activeToolCallIds.add(toolCall.id) - } - yield { - type: "tool_call_partial", - index: toolCall.index, - id: toolCall.id, - name: toolCall.function?.name, - arguments: toolCall.function?.arguments, + // Emit raw tool call chunks - NativeToolCallParser handles state management + if (delta?.tool_calls) { + for (const toolCall of delta.tool_calls) { + if (toolCall.id) { + activeToolCallIds.add(toolCall.id) + } + yield { + type: "tool_call_partial", + index: toolCall.index, + id: toolCall.id, + name: toolCall.function?.name, + arguments: toolCall.function?.arguments, + } } } - } - // Emit tool_call_end events when finish_reason is "tool_calls" - // This ensures tool calls are finalized even if the stream doesn't properly close - if (finishReason === "tool_calls" && activeToolCallIds.size > 0) { - for (const id of activeToolCallIds) { - yield { type: "tool_call_end", id } + // Emit tool_call_end events when finish_reason is "tool_calls" + // This ensures tool calls are finalized even if the stream doesn't properly close + if (finishReason === "tool_calls" && activeToolCallIds.size > 0) { + for (const id of activeToolCallIds) { + yield { type: "tool_call_end", id } + } + activeToolCallIds.clear() } - activeToolCallIds.clear() - } - if (chunk.usage) { - lastUsage = chunk.usage + if (chunk.usage) { + lastUsage = chunk.usage + } } + } catch (error) { + // The creation-site catch does not cover errors raised by the async + // iterator itself (e.g. a mid-stream abort); normalize them the same way. + throw handleOpenAIRequestError(error, this.providerName, metadata?.abortSignal) } if (lastUsage) { diff --git a/src/api/providers/openai.ts b/src/api/providers/openai.ts index 0f28a888e5..d6b13cd039 100644 --- a/src/api/providers/openai.ts +++ b/src/api/providers/openai.ts @@ -228,26 +228,32 @@ export class OpenAiHandler extends BaseProvider implements SingleCompletionHandl let lastUsage const activeToolCallIds = new Set() - for await (const chunk of stream) { - const delta = chunk.choices?.[0]?.delta ?? {} - const finishReason = chunk.choices?.[0]?.finish_reason + try { + for await (const chunk of stream) { + const delta = chunk.choices?.[0]?.delta ?? {} + const finishReason = chunk.choices?.[0]?.finish_reason - if (delta.content) { - for (const chunk of matcher.update(delta.content)) { - yield chunk + if (delta.content) { + for (const chunk of matcher.update(delta.content)) { + yield chunk + } } - } - const reasoningText = extractReasoningFromDelta(delta) - if (reasoningText) { - yield { type: "reasoning", text: reasoningText } - } + const reasoningText = extractReasoningFromDelta(delta) + if (reasoningText) { + yield { type: "reasoning", text: reasoningText } + } - yield* this.processToolCalls(delta, finishReason, activeToolCallIds) + yield* this.processToolCalls(delta, finishReason, activeToolCallIds) - if (chunk.usage) { - lastUsage = chunk.usage + if (chunk.usage) { + lastUsage = chunk.usage + } } + } catch (error) { + // The creation-site catch does not cover errors raised by the async + // iterator itself (e.g. a mid-stream abort); normalize them the same way. + throw handleOpenAIRequestError(error, this.providerName, metadata?.abortSignal) } for (const chunk of matcher.final()) { @@ -455,7 +461,13 @@ export class OpenAiHandler extends BaseProvider implements SingleCompletionHandl throw handleOpenAIRequestError(error, this.providerName, metadata?.abortSignal) } - yield* this.handleStreamResponse(stream) + try { + yield* this.handleStreamResponse(stream) + } catch (error) { + // The creation-site catch does not cover errors raised by the async + // iterator itself (e.g. a mid-stream abort); normalize them the same way. + throw handleOpenAIRequestError(error, this.providerName, metadata?.abortSignal) + } } else { const requestOptions: OpenAI.Chat.Completions.ChatCompletionCreateParamsNonStreaming = { model: modelId, From 217f120770a23fd3316df5c499bbb0e7c2e75ce1 Mon Sep 17 00:00:00 2001 From: easonLiangWorldedtech Date: Fri, 21 Aug 2026 07:12:36 +0800 Subject: [PATCH 05/11] test(providers): cover the base_resp stream error path through the iteration wrapper The stream-iteration wrapper added in 35c95ea19 routes non-abort iteration errors through handleOpenAIRequestError, so a provider base_resp stream error (MiniMax-style inline error chunk) is now rethrown with the provider-prefix wrap ("TestProvider completion error: ...") instead of the raw message. Adds a focused regression test that yields a chunk carrying base_resp and pins the wrapped message. --- .../base-openai-compatible-provider.spec.ts | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/api/providers/__tests__/base-openai-compatible-provider.spec.ts b/src/api/providers/__tests__/base-openai-compatible-provider.spec.ts index a6adeb53e5..615a3c0327 100644 --- a/src/api/providers/__tests__/base-openai-compatible-provider.spec.ts +++ b/src/api/providers/__tests__/base-openai-compatible-provider.spec.ts @@ -368,6 +368,26 @@ describe("BaseOpenAiCompatibleProvider", () => { expect(result.message.endsWith("aborted")).toBe(true) }) + it("should wrap a non-abort base_resp stream error with the provider prefix through the iteration wrapper", async () => { + mockCreate.mockImplementationOnce(() => + asyncStreamFrom([ + { + choices: [{ delta: { content: "partial" } }], + base_resp: { status_code: 1041, status_msg: "Invalid token" }, + }, + ]), + ) + + const result = await captureError( + (async () => { + for await (const _ of handler.createMessage("system prompt", [])) { + // consume + } + })(), + ) + + expect(result.message).toBe("TestProvider completion error: TestProvider API Error (1041): Invalid token") + }) it("should still wrap non-abort request errors with the provider prefix", async () => { mockCreate.mockImplementationOnce(() => { throw new Error("boom") From af320da16a98037d158ad1367729e6d9c58ea1e6 Mon Sep 17 00:00:00 2001 From: easonLiangWorldedtech Date: Fri, 21 Aug 2026 09:00:38 +0800 Subject: [PATCH 06/11] test(providers): cover remaining branch partials in base provider and openai abort paths The codecov patch report (97.83% at 217f12077) flagged 2 partial branch lines (BRDA taken=0 on the ?? / || fallback sides of added lines): - api/providers/base-openai-compatible-provider.ts:171 branch 1 of `${...} ${chunkAny.base_resp.status_msg || "Unknown error"}` - the || "Unknown error" fallback was never exercised; added a focused test yielding a base_resp chunk with status_code set but no status_msg, asserting the wrapped "Unknown error" message. - api/providers/openai.ts:233 branch 1 of `const delta = chunk.choices?.[0]?.delta ?? {}` - the ?? {} fallback (chunk with no delta field) was never exercised; added a focused streaming test yielding a delta-less final chunk and asserting the stream completes without throwing. Full api/providers suite: 1698 passed. No provider code changed. --- .../base-openai-compatible-provider.spec.ts | 22 ++++++++++++++++++- src/api/providers/__tests__/openai.spec.ts | 16 ++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/src/api/providers/__tests__/base-openai-compatible-provider.spec.ts b/src/api/providers/__tests__/base-openai-compatible-provider.spec.ts index 615a3c0327..7ecb98d400 100644 --- a/src/api/providers/__tests__/base-openai-compatible-provider.spec.ts +++ b/src/api/providers/__tests__/base-openai-compatible-provider.spec.ts @@ -385,9 +385,29 @@ describe("BaseOpenAiCompatibleProvider", () => { } })(), ) + }) + + it("should fall back to an Unknown error when a base_resp stream chunk has no status_msg", async () => { + mockCreate.mockImplementationOnce(() => + asyncStreamFrom([ + { + choices: [{ delta: { content: "partial" } }], + base_resp: { status_code: 1041 }, + }, + ]), + ) - expect(result.message).toBe("TestProvider completion error: TestProvider API Error (1041): Invalid token") + const result = await captureError( + (async () => { + for await (const _ of handler.createMessage("system prompt", [])) { + // consume + } + })(), + ) + + expect(result.message).toBe("TestProvider completion error: TestProvider API Error (1041): Unknown error") }) + it("should still wrap non-abort request errors with the provider prefix", async () => { mockCreate.mockImplementationOnce(() => { throw new Error("boom") diff --git a/src/api/providers/__tests__/openai.spec.ts b/src/api/providers/__tests__/openai.spec.ts index e62d3ab6a6..d07816b80e 100644 --- a/src/api/providers/__tests__/openai.spec.ts +++ b/src/api/providers/__tests__/openai.spec.ts @@ -277,6 +277,22 @@ describe("OpenAiHandler", () => { expect(textChunks[0].text).toBe("Test response") }) + it("should treat a streaming chunk without a delta as an empty delta", async () => { + mockCreate.mockImplementationOnce(async () => + asyncStreamFrom([ + { choices: [{ delta: { content: "hello " }, index: 0 }] }, + // Some providers emit a final chunk with no delta field at all; the + // iteration must fall back to an empty object and not throw. + { choices: [{ finish_reason: "stop", index: 0 }] }, + ]), + ) + + const chunks = await collectStream(handler.createMessage(systemPrompt, messages)) + + const textChunks = chunks.filter((chunk) => chunk.type === "text") + expect(textChunks).toEqual([{ type: "text", text: "hello " }]) + }) + it("streams reasoning chunks from delta.reasoning_content", async () => { mockCreate.mockImplementationOnce(async () => asyncStreamFrom([ From a0117fb7cd080cf49e7a54d442691bdea687995c Mon Sep 17 00:00:00 2001 From: easonLiangWorldedtech Date: Fri, 21 Aug 2026 09:19:26 +0800 Subject: [PATCH 07/11] feat(api): add shared isRequestAborted and createAbortError helpers to abort-signal utils The OpenAI-family provider PRs (#1309, #1311) carry per-provider copies of the same abort-detection helper (isRequestAborted) and the same abort-error constructor (createAbortError); only the provider name in the message differs. Per the CodeRabbit maintainability finding on #1309 (extract the shared abort helpers into utils/abort-signal.ts), these are now shared in the foundation utility: - isRequestAborted(error, signal?) - true when the caller signal fired, a native AbortError / OpenAI SDK APIUserAbortError was raised, or the message is exactly "Request was aborted." (exact match; a substring match would misclassify unrelated errors that merely mention aborting) - createAbortError(providerName) - fresh error with name === "AbortError" and message "The request was aborted", satisfying the Task.ts abort contract - exported OpenAiRequestOptions type 7 new tests (isRequestAborted 4, createAbortError 3). --- .../utils/__tests__/abort-signal.spec.ts | 61 ++++++++++++++++++- src/api/providers/utils/abort-signal.ts | 41 +++++++++++++ 2 files changed, 101 insertions(+), 1 deletion(-) diff --git a/src/api/providers/utils/__tests__/abort-signal.spec.ts b/src/api/providers/utils/__tests__/abort-signal.spec.ts index 1e2181655f..aba72c181f 100644 --- a/src/api/providers/utils/__tests__/abort-signal.spec.ts +++ b/src/api/providers/utils/__tests__/abort-signal.spec.ts @@ -1,4 +1,10 @@ -import { mergeAbortSignalAndTimeout, mergeAbortSignals, throwIfAborted } from "../abort-signal" +import { + createAbortError, + isRequestAborted, + mergeAbortSignalAndTimeout, + mergeAbortSignals, + throwIfAborted, +} from "../abort-signal" describe("abort-signal utilities", () => { describe("mergeAbortSignalAndTimeout", () => { @@ -126,4 +132,57 @@ describe("abort-signal utilities", () => { expect((caught as Error).name).toBe("AbortError") }) }) + + describe("isRequestAborted", () => { + it("returns true when the caller signal is aborted", () => { + const controller = new AbortController() + controller.abort() + + expect(isRequestAborted(new Error("boom"), controller.signal)).toBe(true) + expect(isRequestAborted(undefined, controller.signal)).toBe(true) + }) + + it("returns true for a native AbortError or the OpenAI SDK APIUserAbortError", () => { + const native = new Error("This operation was aborted") + native.name = "AbortError" + expect(isRequestAborted(native)).toBe(true) + + const sdk = new Error("whatever") + sdk.name = "APIUserAbortError" + expect(isRequestAborted(sdk)).toBe(true) + }) + + it("matches the OpenAI SDK abort message exactly, not as a substring", () => { + expect(isRequestAborted(new Error("Request was aborted."))).toBe(true) + expect(isRequestAborted(new Error("Request was aborted"))).toBe(false) + expect(isRequestAborted(new Error("Request was aborted. Please retry"))).toBe(false) + }) + + it("returns false for unrelated errors, nullish errors, and live signals", () => { + expect(isRequestAborted(new Error("the abort failed"))).toBe(false) + expect(isRequestAborted(undefined)).toBe(false) + expect(isRequestAborted(null)).toBe(false) + + const controller = new AbortController() + expect(isRequestAborted(new Error("boom"), controller.signal)).toBe(false) + }) + }) + + describe("createAbortError", () => { + it("builds an error satisfying the Task.ts abort contract", () => { + const error = createAbortError("LM Studio") + + expect(error).toBeInstanceOf(Error) + expect(error.name).toBe("AbortError") + expect(error.message).toBe("The LM Studio request was aborted") + }) + + it("interpolates the provider name", () => { + expect(createAbortError("Qwen Code").message).toBe("The Qwen Code request was aborted") + }) + + it("returns a fresh error on each call", () => { + expect(createAbortError("X")).not.toBe(createAbortError("X")) + }) + }) }) diff --git a/src/api/providers/utils/abort-signal.ts b/src/api/providers/utils/abort-signal.ts index 033e861b2b..26f57c3e9a 100644 --- a/src/api/providers/utils/abort-signal.ts +++ b/src/api/providers/utils/abort-signal.ts @@ -52,3 +52,44 @@ export function throwIfAborted(signal?: AbortSignal): void { abortError.name = "AbortError" throw abortError } + +/** + * Request options this series passes to the OpenAI SDK call. The SDK's + * `RequestOptions` declares `signal` as `AbortSignal | null | undefined`, + * which does not satisfy the builder's base constraint, so the builder is + * typed with only the options this series sets. The built config is still + * assignable to the SDK's `RequestOptions`. + */ +export type OpenAiRequestOptions = { + signal?: AbortSignal +} + +/** + * Whether a failure indicates an aborted request: the caller's signal fired, + * the SDK raised a native abort error, or the error carries the OpenAI SDK + * abort error message (exactly "Request was aborted."). The message check + * is an exact match on purpose: a substring match would misclassify + * unrelated errors that merely mention aborting. + */ +export function isRequestAborted(error: unknown, signal?: AbortSignal): boolean { + const candidate = error as { name?: string; message?: string } + return ( + Boolean(signal?.aborted) || + candidate?.name === "AbortError" || + candidate?.name === "APIUserAbortError" || + candidate?.message === "Request was aborted." + ) +} + +/** + * Fresh error satisfying the Task.ts abort contract: `name === + * "AbortError"` and a message ending in "aborted" (no trailing period). The + * OpenAI SDK's own abort error does not satisfy this contract (name "Error", + * message "Request was aborted."), so raw SDK abort errors must be + * normalized instead of rethrown. + */ +export function createAbortError(providerName: string): Error { + const abortError = new Error(`The ${providerName} request was aborted`) + abortError.name = "AbortError" + return abortError +} From 20b1228f2cd5e8debcbc58b54f27fc32a79735ec Mon Sep 17 00:00:00 2001 From: Eason Liang Date: Thu, 3 Sep 2026 01:17:03 +0800 Subject: [PATCH 08/11] test(zai): align GLM-5.3-Flash expectations with abort-signal request config --- src/api/providers/__tests__/zai.spec.ts | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/api/providers/__tests__/zai.spec.ts b/src/api/providers/__tests__/zai.spec.ts index a251ad7d20..e3d13708e8 100644 --- a/src/api/providers/__tests__/zai.spec.ts +++ b/src/api/providers/__tests__/zai.spec.ts @@ -958,6 +958,7 @@ describe("ZAiHandler", () => { reasoning_effort: "max", temperature: 1, }), + undefined, ) }) @@ -1018,13 +1019,16 @@ describe("ZAiHandler", () => { mockCreate.mockResolvedValueOnce({ choices: [{ message: { content: "response" } }] }) await expect(handlerWithModel.completePrompt("prompt")).resolves.toBe("response") - expect(mockCreate).toHaveBeenCalledWith({ - model: "glm-5.3-flash", - messages: [{ role: "user", content: "prompt" }], - temperature: 1, - thinking: { type: "enabled", clear_thinking: false }, - reasoning_effort: "low", - }) + expect(mockCreate).toHaveBeenCalledWith( + { + model: "glm-5.3-flash", + messages: [{ role: "user", content: "prompt" }], + temperature: 1, + thinking: { type: "enabled", clear_thinking: false }, + reasoning_effort: "low", + }, + undefined, + ) }) it("should omit reasoning_effort for GLM-5.2 when reasoningEffort is set to disable", async () => { From 8ce2489d2867b540fe170f3bd1934d69cf915ac3 Mon Sep 17 00:00:00 2001 From: Eason Liang Date: Thu, 3 Sep 2026 02:45:30 +0800 Subject: [PATCH 09/11] refactor(api): dedupe handleOpenAIRequestError and strengthen abort-signal specs --- .../base-openai-compatible-provider.spec.ts | 17 +++---------- src/api/providers/__tests__/kimi-code.spec.ts | 12 +-------- src/api/providers/__tests__/openai.spec.ts | 20 +++++---------- src/api/providers/__tests__/zai.spec.ts | 15 +---------- .../base-openai-compatible-provider.ts | 25 ++----------------- src/api/providers/openai.ts | 25 ++----------------- src/api/providers/utils/error-handler.ts | 23 +++++++++++++++++ src/api/providers/zai.ts | 3 ++- src/test-utils/errors.ts | 17 +++++++++++++ 9 files changed, 57 insertions(+), 100 deletions(-) create mode 100644 src/test-utils/errors.ts diff --git a/src/api/providers/__tests__/base-openai-compatible-provider.spec.ts b/src/api/providers/__tests__/base-openai-compatible-provider.spec.ts index 7ecb98d400..fde5ce242c 100644 --- a/src/api/providers/__tests__/base-openai-compatible-provider.spec.ts +++ b/src/api/providers/__tests__/base-openai-compatible-provider.spec.ts @@ -8,6 +8,7 @@ import type { ModelInfo } from "@roo-code/types" import { BaseOpenAiCompatibleProvider } from "../base-openai-compatible-provider" import { asyncStreamFrom, collectStream } from "../../../test-utils/stream" import { clearAllMocks } from "../../../test-utils/reset" +import { captureError } from "../../../test-utils/errors" // Create mock functions const mockCreate = vi.fn() @@ -51,20 +52,6 @@ class TestOpenAiCompatibleProvider extends BaseOpenAiCompatibleProvider<"test-mo } } -/** - * Captures the rejection of an operation as an Error. The abort contract always - * throws an Error; the guard keeps strict typing without a cast. Fails the test - * if the operation resolves. - */ -async function captureError(operation: Promise): Promise { - try { - await operation - } catch (error) { - return error instanceof Error ? error : new Error(String(error)) - } - throw new Error("Expected the operation to reject") -} - describe("BaseOpenAiCompatibleProvider", () => { let handler: TestOpenAiCompatibleProvider @@ -385,6 +372,8 @@ describe("BaseOpenAiCompatibleProvider", () => { } })(), ) + + expect(result.message).toBe("TestProvider completion error: TestProvider API Error (1041): Invalid token") }) it("should fall back to an Unknown error when a base_resp stream chunk has no status_msg", async () => { diff --git a/src/api/providers/__tests__/kimi-code.spec.ts b/src/api/providers/__tests__/kimi-code.spec.ts index 9179754695..e5f04148ca 100644 --- a/src/api/providers/__tests__/kimi-code.spec.ts +++ b/src/api/providers/__tests__/kimi-code.spec.ts @@ -2,6 +2,7 @@ import { buildApiHandler } from "../../index" import { KimiCodeHandler } from "../kimi-code" import { clearAllMocks } from "../../../test-utils/reset" +import { captureError } from "../../../test-utils/errors" import { providerIdentifiers } from "@roo-code/types/provider-identifiers" const { mockGetAccessToken, mockForceRefreshAccessToken, mockGetModels } = vi.hoisted(() => ({ @@ -37,17 +38,6 @@ function completionsCreate(handler: KimiCodeHandler): ReturnType { return vi.spyOn(client.chat.completions, "create") as unknown as ReturnType } -/** Captures the rejection of an operation; fails the test if it resolves. */ -async function captureError(operation: Promise): Promise { - try { - await operation - } catch (error) { - // Abort normalization always throws an Error; the guard keeps strict typing without casts. - return error instanceof Error ? error : new Error(String(error)) - } - throw new Error("Expected the operation to reject") -} - describe("KimiCodeHandler", () => { beforeEach(() => { clearAllMocks() diff --git a/src/api/providers/__tests__/openai.spec.ts b/src/api/providers/__tests__/openai.spec.ts index d07816b80e..7a38ae1f1e 100644 --- a/src/api/providers/__tests__/openai.spec.ts +++ b/src/api/providers/__tests__/openai.spec.ts @@ -12,6 +12,7 @@ import { import { Package } from "../../../shared/package" import { makeApiHandlerOptions } from "../../../test-utils/api" import { asyncStreamFrom, collectStream } from "../../../test-utils/stream" +import { captureError } from "../../../test-utils/errors" import axios from "axios" vitest.mock("../utils/timeout-config", () => ({ @@ -92,20 +93,6 @@ vitest.mock("axios", () => ({ }, })) -/** - * Captures the rejection of an operation as an Error. The abort contract always - * throws an Error; the guard keeps strict typing without a cast. Fails the test - * if the operation resolves. - */ -async function captureError(operation: Promise): Promise { - try { - await operation - } catch (error) { - return error instanceof Error ? error : new Error(String(error)) - } - throw new Error("Expected the operation to reject") -} - describe("OpenAiHandler", () => { let handler: OpenAiHandler let mockOptions: ApiHandlerOptions @@ -979,6 +966,11 @@ describe("OpenAiHandler", () => { const requestOptions = mockCreate.mock.calls.at(-1)?.[1] expect(requestOptions?.signal).toBeInstanceOf(AbortSignal) expect(requestOptions?.signal.aborted).toBe(false) + // The merged signal must follow the caller's controller: a regression + // that drops options.abortSignal (keeping only the timeout signal) + // would stop aborting here and fail the test. + controller.abort() + expect(requestOptions?.signal.aborted).toBe(true) }) it("should pass a timeout-only request signal when completePrompt timeoutMs is positive", async () => { diff --git a/src/api/providers/__tests__/zai.spec.ts b/src/api/providers/__tests__/zai.spec.ts index e3d13708e8..53a0801a1c 100644 --- a/src/api/providers/__tests__/zai.spec.ts +++ b/src/api/providers/__tests__/zai.spec.ts @@ -17,6 +17,7 @@ import { import { ZAiHandler } from "../zai" import { asyncStreamFrom } from "../../../test-utils/stream" import { clearAllMocks } from "../../../test-utils/reset" +import { captureError } from "../../../test-utils/errors" vitest.mock("openai", () => { const createMock = vitest.fn() @@ -29,20 +30,6 @@ vitest.mock("openai", () => { } }) -/** - * Captures the rejection of an operation as an Error. The abort contract always - * throws an Error; the guard keeps strict typing without a cast. Fails the test - * if the operation resolves. - */ -async function captureError(operation: Promise): Promise { - try { - await operation - } catch (error) { - return error instanceof Error ? error : new Error(String(error)) - } - throw new Error("Expected the operation to reject") -} - describe("ZAiHandler", () => { let handler: ZAiHandler let mockCreate: any diff --git a/src/api/providers/base-openai-compatible-provider.ts b/src/api/providers/base-openai-compatible-provider.ts index 2f37b2101b..698a348a54 100644 --- a/src/api/providers/base-openai-compatible-provider.ts +++ b/src/api/providers/base-openai-compatible-provider.ts @@ -1,5 +1,5 @@ import { Anthropic } from "@anthropic-ai/sdk" -import OpenAI, { APIUserAbortError } from "openai" +import OpenAI from "openai" import type { ModelInfo } from "@roo-code/types" @@ -11,7 +11,7 @@ import { convertToOpenAiMessages } from "../transform/openai-format" import type { SingleCompletionHandler, ApiHandlerCreateMessageMetadata, CompletePromptOptions } from "../index" import { DEFAULT_HEADERS } from "./constants" import { BaseProvider } from "./base-provider" -import { handleOpenAIError } from "./utils/error-handler" +import { handleOpenAIError, handleOpenAIRequestError } from "./utils/error-handler" import { calculateApiCostOpenAI } from "../../shared/cost" import { extractReasoningFromDelta } from "./utils/extract-reasoning" import { RequestConfigBuilder } from "./config-builder/request-config-builder" @@ -30,27 +30,6 @@ type OpenAiRequestConfig = { signal?: AbortSignal } -/** - * Handles errors from OpenAI Node SDK request sites with abort awareness. - * - * An abort failure (the external signal already aborted, the SDK's - * APIUserAbortError, or a fetch-level AbortError) is normalized to a fresh - * Error with name "AbortError" and a message ending in "aborted" (the Task.ts - * contract) instead of being wrapped as a regular completion error, which a - * plain rethrow of the SDK abort error would produce. - */ -export function handleOpenAIRequestError(error: unknown, providerName: string, abortSignal?: AbortSignal): Error { - if ( - abortSignal?.aborted || - (error instanceof Error && (error.name === "AbortError" || error instanceof APIUserAbortError)) - ) { - const aborted = new Error(`${providerName} request aborted`, { cause: error }) - aborted.name = "AbortError" - return aborted - } - return handleOpenAIError(error, providerName) -} - export abstract class BaseOpenAiCompatibleProvider extends BaseProvider implements SingleCompletionHandler diff --git a/src/api/providers/openai.ts b/src/api/providers/openai.ts index d6b13cd039..9f6c0d7197 100644 --- a/src/api/providers/openai.ts +++ b/src/api/providers/openai.ts @@ -1,5 +1,5 @@ import { Anthropic } from "@anthropic-ai/sdk" -import OpenAI, { AzureOpenAI, APIUserAbortError } from "openai" +import OpenAI, { AzureOpenAI } from "openai" import axios from "axios" import { @@ -24,7 +24,7 @@ import { getModelParams } from "../transform/model-params" import { DEFAULT_HEADERS, NOT_PROVIDED } from "./constants" import { BaseProvider } from "./base-provider" import type { SingleCompletionHandler, ApiHandlerCreateMessageMetadata, CompletePromptOptions } from "../index" -import { handleOpenAIError } from "./utils/error-handler" +import { handleOpenAIRequestError } from "./utils/error-handler" import { extractReasoningFromDelta } from "./utils/extract-reasoning" import { RequestConfigBuilder } from "./config-builder/request-config-builder" import { mergeAbortSignalAndTimeout, throwIfAborted } from "./utils/abort-signal" @@ -35,27 +35,6 @@ type OpenAiRequestConfig = { signal?: AbortSignal } -/** - * Handles errors from OpenAI Node SDK request sites with abort awareness. - * - * An abort failure (the external signal already aborted, the SDK's - * APIUserAbortError, or a fetch-level AbortError) is normalized to a fresh - * Error with name "AbortError" and a message ending in "aborted" (the Task.ts - * contract) instead of being wrapped as a regular completion error, which a - * plain rethrow of the SDK abort error would produce. - */ -function handleOpenAIRequestError(error: unknown, providerName: string, abortSignal?: AbortSignal): Error { - if ( - abortSignal?.aborted || - (error instanceof Error && (error.name === "AbortError" || error instanceof APIUserAbortError)) - ) { - const aborted = new Error(`${providerName} request aborted`, { cause: error }) - aborted.name = "AbortError" - return aborted - } - return handleOpenAIError(error, providerName) -} - // TODO: Rename this to OpenAICompatibleHandler. Also, I think the // `OpenAINativeHandler` can subclass from this, since it's obviously // compatible with the OpenAI API. We can also rename it to `OpenAIHandler`. diff --git a/src/api/providers/utils/error-handler.ts b/src/api/providers/utils/error-handler.ts index 2c55b96f9c..ffa73270b4 100644 --- a/src/api/providers/utils/error-handler.ts +++ b/src/api/providers/utils/error-handler.ts @@ -9,6 +9,8 @@ * - Enables telemetry and debugging with complete error context */ +import { APIUserAbortError } from "openai" + import i18n from "../../../i18n/setup" /** @@ -112,3 +114,24 @@ export function handleProviderError( export function handleOpenAIError(error: unknown, providerName: string): Error { return handleProviderError(error, providerName, { messagePrefix: "completion" }) } + +/** + * Handles errors from OpenAI Node SDK request sites with abort awareness. + * + * An abort failure (the external signal already aborted, the SDK's + * APIUserAbortError, or a fetch-level AbortError) is normalized to a fresh + * Error with name "AbortError" and a message ending in "aborted" (the Task.ts + * contract) instead of being wrapped as a regular completion error, which a + * plain rethrow of the SDK abort error would produce. + */ +export function handleOpenAIRequestError(error: unknown, providerName: string, abortSignal?: AbortSignal): Error { + if ( + abortSignal?.aborted || + (error instanceof Error && (error.name === "AbortError" || error instanceof APIUserAbortError)) + ) { + const aborted = new Error(`${providerName} request aborted`, { cause: error }) + aborted.name = "AbortError" + return aborted + } + return handleOpenAIError(error, providerName) +} diff --git a/src/api/providers/zai.ts b/src/api/providers/zai.ts index 5be0ec6059..98da44d898 100644 --- a/src/api/providers/zai.ts +++ b/src/api/providers/zai.ts @@ -14,8 +14,9 @@ import { type ApiHandlerOptions, getModelMaxOutputTokens } from "../../shared/ap import { convertToZAiFormat } from "../transform/zai-format" import type { ApiHandlerCreateMessageMetadata, CompletePromptOptions } from "../index" -import { BaseOpenAiCompatibleProvider, handleOpenAIRequestError } from "./base-openai-compatible-provider" +import { BaseOpenAiCompatibleProvider } from "./base-openai-compatible-provider" import { NOT_PROVIDED } from "./constants" +import { handleOpenAIRequestError } from "./utils/error-handler" import { RequestConfigBuilder } from "./config-builder/request-config-builder" import { mergeAbortSignalAndTimeout, throwIfAborted } from "./utils/abort-signal" diff --git a/src/test-utils/errors.ts b/src/test-utils/errors.ts new file mode 100644 index 0000000000..67d897b242 --- /dev/null +++ b/src/test-utils/errors.ts @@ -0,0 +1,17 @@ +/** + * Captures the rejection of an operation as an `Error`. + * + * Provider abort normalization always throws an `Error`, but an awaited + * operation is typed `unknown`, so the guard keeps strict typing without + * casts. A non-Error rejection is re-wrapped so the original failure message + * stays visible, and a resolving operation — the contract violation the + * surrounding test exists to catch — becomes a failing assertion. + */ +export async function captureError(operation: Promise): Promise { + try { + await operation + } catch (error) { + return error instanceof Error ? error : new Error(String(error)) + } + throw new Error("Expected the operation to reject") +} From cb77e0c24c80977d5e7e7fc0f88671c118e844e5 Mon Sep 17 00:00:00 2001 From: Eason Liang Date: Thu, 3 Sep 2026 03:21:30 +0800 Subject: [PATCH 10/11] fix(api): reject pre-aborted Kimi requests and forward per-request timeoutMs - KimiCodeHandler.createMessage/completePrompt now call throwIfAborted before prepareRequest, so pre-aborted requests skip model discovery and OAuth token work; cancellation specs assert the model-discovery and OAuth mocks received no calls. - openai and zai completePrompt request configs pass a positive options.timeoutMs as the per-request SDK timeout (a larger timeoutMs no longer expires at the client default); specs assert timeout: 5000 in the captured request options. - base provider stream iteration reads base_resp through an unknown guard instead of an as any cast (no-explicit-any 6 -> 5). - zai GLM-5.3 and base provider specs now cover the combined abortSignal + positive timeoutMs cancellation path. --- .../base-openai-compatible-provider.spec.ts | 9 +++++-- src/api/providers/__tests__/kimi-code.spec.ts | 8 ++++++ src/api/providers/__tests__/openai.spec.ts | 3 +++ src/api/providers/__tests__/zai.spec.ts | 17 +++++++++--- .../base-openai-compatible-provider.ts | 26 ++++++++++++++++--- src/api/providers/kimi-code.ts | 3 +++ src/api/providers/openai.ts | 9 ++++++- src/api/providers/zai.ts | 11 ++++++-- src/eslint-suppressions.json | 2 +- 9 files changed, 74 insertions(+), 14 deletions(-) diff --git a/src/api/providers/__tests__/base-openai-compatible-provider.spec.ts b/src/api/providers/__tests__/base-openai-compatible-provider.spec.ts index fde5ce242c..31494eb4ef 100644 --- a/src/api/providers/__tests__/base-openai-compatible-provider.spec.ts +++ b/src/api/providers/__tests__/base-openai-compatible-provider.spec.ts @@ -425,14 +425,19 @@ describe("BaseOpenAiCompatibleProvider", () => { }) }) - it("should merge completePrompt timeoutMs into the request signal", async () => { + it("should merge the completePrompt abort signal and timeoutMs into one request signal", async () => { + const controller = new AbortController() mockCreate.mockResolvedValueOnce({ choices: [{ message: { content: "response" } }] }) - await handler.completePrompt("test prompt", { timeoutMs: 5000 }) + await handler.completePrompt("test prompt", { abortSignal: controller.signal, timeoutMs: 5000 }) const requestOptions = mockCreate.mock.calls.at(-1)?.[1] expect(requestOptions?.signal).toBeInstanceOf(AbortSignal) expect(requestOptions?.signal.aborted).toBe(false) + // A timeout-only merged signal would still pass the assertions above; + // aborting the caller's controller proves caller cancellation survives. + controller.abort() + expect(requestOptions?.signal.aborted).toBe(true) }) it("should not set a request signal for zero completePrompt timeoutMs", async () => { diff --git a/src/api/providers/__tests__/kimi-code.spec.ts b/src/api/providers/__tests__/kimi-code.spec.ts index e5f04148ca..bbd1e1b4a9 100644 --- a/src/api/providers/__tests__/kimi-code.spec.ts +++ b/src/api/providers/__tests__/kimi-code.spec.ts @@ -290,6 +290,10 @@ describe("KimiCodeHandler", () => { } }).rejects.toMatchObject({ name: "AbortError", message: "This operation was aborted" }) expect(createCompletion).not.toHaveBeenCalled() + // Cancellation must also skip model discovery and OAuth token work. + expect(mockGetModels).not.toHaveBeenCalled() + expect(mockGetAccessToken).not.toHaveBeenCalled() + expect(mockForceRefreshAccessToken).not.toHaveBeenCalled() }) it("forwards completePrompt abort options through the override on both 401 retry attempts", async () => { @@ -319,6 +323,10 @@ describe("KimiCodeHandler", () => { message: "This operation was aborted", }) expect(createCompletion).not.toHaveBeenCalled() + // Cancellation must also skip model discovery and OAuth token work. + expect(mockGetModels).not.toHaveBeenCalled() + expect(mockGetAccessToken).not.toHaveBeenCalled() + expect(mockForceRefreshAccessToken).not.toHaveBeenCalled() }) it("surfaces a normalized AbortError when the SDK aborts a streaming request", async () => { diff --git a/src/api/providers/__tests__/openai.spec.ts b/src/api/providers/__tests__/openai.spec.ts index 7a38ae1f1e..7eb276277d 100644 --- a/src/api/providers/__tests__/openai.spec.ts +++ b/src/api/providers/__tests__/openai.spec.ts @@ -971,6 +971,8 @@ describe("OpenAiHandler", () => { // would stop aborting here and fail the test. controller.abort() expect(requestOptions?.signal.aborted).toBe(true) + // A positive timeoutMs must be forwarded as the per-request SDK timeout. + expect(requestOptions?.timeout).toBe(5000) }) it("should pass a timeout-only request signal when completePrompt timeoutMs is positive", async () => { @@ -978,6 +980,7 @@ describe("OpenAiHandler", () => { await handler.completePrompt("test prompt", { timeoutMs: 5000 }) const requestOptions = mockCreate.mock.calls.at(-1)?.[1] expect(requestOptions?.signal).toBeInstanceOf(AbortSignal) + expect(requestOptions?.timeout).toBe(5000) }) it("should not pass a request signal for zero timeoutMs in completePrompt", async () => { diff --git a/src/api/providers/__tests__/zai.spec.ts b/src/api/providers/__tests__/zai.spec.ts index 53a0801a1c..02751434ce 100644 --- a/src/api/providers/__tests__/zai.spec.ts +++ b/src/api/providers/__tests__/zai.spec.ts @@ -742,7 +742,7 @@ describe("ZAiHandler", () => { expect(result.message).toBe("Z.ai request aborted") }) - it("glm-5.3 completePrompt should pass the abort signal on the thinking path", async () => { + it("glm-5.3 completePrompt should merge the abort signal and timeoutMs on the thinking path", async () => { const h53 = new ZAiHandler({ apiModelId: "glm-5.3", zaiApiKey: "test-zai-api-key", @@ -751,13 +751,22 @@ describe("ZAiHandler", () => { const controller = new AbortController() mockCreate.mockResolvedValueOnce({ choices: [{ message: { content: "response" } }] }) - const result = await h53.completePrompt("prompt", { abortSignal: controller.signal }) + const result = await h53.completePrompt("prompt", { abortSignal: controller.signal, timeoutMs: 5000 }) expect(result).toBe("response") - expect(mockCreate).toHaveBeenCalledWith( + const requestCall = mockCreate.mock.calls.at(-1) + expect(requestCall?.[0]).toEqual( expect.objectContaining({ model: "glm-5.3", thinking: { type: "enabled", clear_thinking: false } }), - { signal: controller.signal }, ) + const requestOptions = requestCall?.[1] + expect(requestOptions?.signal).toBeInstanceOf(AbortSignal) + expect(requestOptions?.signal.aborted).toBe(false) + // A positive timeoutMs must be forwarded as the per-request SDK timeout. + expect(requestOptions?.timeout).toBe(5000) + // A timeout-only merged signal would still pass the assertions above; + // aborting the caller's controller proves caller cancellation survives. + controller.abort() + expect(requestOptions?.signal.aborted).toBe(true) }) it("glm-5.3 completePrompt should normalize the SDK APIUserAbortError", async () => { diff --git a/src/api/providers/base-openai-compatible-provider.ts b/src/api/providers/base-openai-compatible-provider.ts index 698a348a54..bba5b33da8 100644 --- a/src/api/providers/base-openai-compatible-provider.ts +++ b/src/api/providers/base-openai-compatible-provider.ts @@ -143,11 +143,29 @@ export abstract class BaseOpenAiCompatibleProvider try { for await (const chunk of stream) { - // Check for provider-specific error responses (e.g., MiniMax base_resp) - const chunkAny = chunk as any - if (chunkAny.base_resp?.status_code && chunkAny.base_resp.status_code !== 0) { + // Check for provider-specific error responses (e.g., MiniMax base_resp). + // ChatCompletionChunk has no base_resp member, so read it through an + // unknown guard instead of casting the whole chunk. + const chunkUnknown: unknown = chunk + const baseResp: unknown = + typeof chunkUnknown === "object" && chunkUnknown !== null && "base_resp" in chunkUnknown + ? (chunkUnknown as { base_resp?: unknown }).base_resp + : undefined + const baseRespFields = + baseResp !== null && typeof baseResp === "object" + ? (baseResp as Record) + : undefined + const baseRespStatusCode = baseRespFields?.["status_code"] + const baseRespStatusMsg = baseRespFields?.["status_msg"] + if ( + baseRespStatusCode && + baseRespStatusCode !== 0 && + (typeof baseRespStatusCode === "number" || typeof baseRespStatusCode === "string") + ) { throw new Error( - `${this.providerName} API Error (${chunkAny.base_resp.status_code}): ${chunkAny.base_resp.status_msg || "Unknown error"}`, + `${this.providerName} API Error (${baseRespStatusCode}): ${ + typeof baseRespStatusMsg === "string" ? baseRespStatusMsg : "Unknown error" + }`, ) } diff --git a/src/api/providers/kimi-code.ts b/src/api/providers/kimi-code.ts index a81efb488a..84d6fd885a 100644 --- a/src/api/providers/kimi-code.ts +++ b/src/api/providers/kimi-code.ts @@ -20,6 +20,7 @@ import { getModelParams } from "../transform/model-params" import { OpenAiHandler } from "./openai" import { NOT_PROVIDED } from "./constants" import { getModels } from "./fetchers/modelCache" +import { throwIfAborted } from "./utils/abort-signal" const OAUTH_AUTH_METHOD: KimiCodeAuthMethod = "oauth" const API_KEY_AUTH_METHOD: KimiCodeAuthMethod = "api-key" @@ -88,6 +89,7 @@ export class KimiCodeHandler extends OpenAiHandler { messages: Anthropic.Messages.MessageParam[], metadata?: ApiHandlerCreateMessageMetadata, ): ApiStream { + throwIfAborted(metadata?.abortSignal) await this.prepareRequest() try { yield* super.createMessage(systemPrompt, messages, metadata) @@ -99,6 +101,7 @@ export class KimiCodeHandler extends OpenAiHandler { } override async completePrompt(prompt: string, options?: CompletePromptOptions): Promise { + throwIfAborted(options?.abortSignal) await this.prepareRequest() try { // Forward abort/timeout options so the inherited OpenAiHandler wiring diff --git a/src/api/providers/openai.ts b/src/api/providers/openai.ts index 9f6c0d7197..51237767f3 100644 --- a/src/api/providers/openai.ts +++ b/src/api/providers/openai.ts @@ -33,6 +33,8 @@ import { mergeAbortSignalAndTimeout, throwIfAborted } from "./utils/abort-signal type OpenAiRequestConfig = { path?: string signal?: AbortSignal + /** Per-request SDK timeout; a positive timeoutMs overrides the client default. */ + timeout?: number } // TODO: Rename this to OpenAICompatibleHandler. Also, I think the @@ -380,16 +382,21 @@ export class OpenAiHandler extends BaseProvider implements SingleCompletionHandl /** * Builds the per-request options for completePrompt. CompletePromptOptions * is not ApiHandlerCreateMessageMetadata (required taskId), so the merged - * abort/timeout signal goes through setOption instead of setAbortSignal. + * abort/timeout signal and the per-request SDK timeout (a positive timeoutMs + * overrides the client-level default) go through setOption instead of + * setAbortSignal. */ private buildCompletePromptRequestConfig( isAzureAiInference: boolean, options?: CompletePromptOptions, ): OpenAI.RequestOptions { + const requestTimeout = + typeof options?.timeoutMs === "number" && options.timeoutMs > 0 ? options.timeoutMs : undefined return ( new RequestConfigBuilder() .setOption("path", isAzureAiInference ? OPENAI_AZURE_AI_INFERENCE_PATH : undefined) .setOption("signal", mergeAbortSignalAndTimeout(options?.abortSignal, options?.timeoutMs)) + .setOption("timeout", requestTimeout) .build() ?? {} ) } diff --git a/src/api/providers/zai.ts b/src/api/providers/zai.ts index 98da44d898..c388a1a889 100644 --- a/src/api/providers/zai.ts +++ b/src/api/providers/zai.ts @@ -32,6 +32,8 @@ type ZAiChatCompletionParams = Omit model === "glm-5.3" || model === "glm-5.3-flash" @@ -172,10 +174,15 @@ export class ZAiHandler extends BaseOpenAiCompatibleProvider { } // CompletePromptOptions is not ApiHandlerCreateMessageMetadata (required - // taskId), so the abort/timeout merge goes through setOption; the helper - // treats timeoutMs <= 0 as "no explicit timeout". + // taskId), so the abort/timeout merge and the per-request SDK timeout go + // through setOption; a positive timeoutMs overrides the client default and + // values <= 0 mean "no explicit timeout". const requestConfig = new RequestConfigBuilder() .setOption("signal", mergeAbortSignalAndTimeout(options?.abortSignal, options?.timeoutMs)) + .setOption( + "timeout", + typeof options?.timeoutMs === "number" && options.timeoutMs > 0 ? options.timeoutMs : undefined, + ) .build() try { diff --git a/src/eslint-suppressions.json b/src/eslint-suppressions.json index 4c3955d3f3..7b31f3cd89 100644 --- a/src/eslint-suppressions.json +++ b/src/eslint-suppressions.json @@ -281,7 +281,7 @@ }, "api/providers/base-openai-compatible-provider.ts": { "@typescript-eslint/no-explicit-any": { - "count": 6 + "count": 5 } }, "api/providers/base-provider.ts": { From c9311dc4234e8b1c97dadf7b7d3c53a492e1c0b5 Mon Sep 17 00:00:00 2001 From: Eason Liang Date: Thu, 3 Sep 2026 03:59:26 +0800 Subject: [PATCH 11/11] fix(api): forward per-request timeoutMs and harden abort specs - base-openai-compatible-provider.completePrompt now forwards a positive timeoutMs as the per-request SDK timeout (RequestOptions.timeout); without it the OpenAI client falls back to the client-level default and can expire before a larger per-request timeoutMs. - base spec adds a timeout-only test (no caller signal) that exercises the timeout branch alone: the request signal aborts when timeoutMs elapses and the pending request rejects with the normalized abort error; the merged-signal test also asserts the forwarded timeout. - zai GLM-5.3 spec now keeps the mocked request pending, aborts the caller signal before awaiting, and asserts the in-flight request rejects with the normalized abort error. - kimi-code pre-abort tests use OAuth authentication so the OAuth-mock skip assertions are not vacuous (resolveAccessToken would invoke the mocks without the guards). --- .../base-openai-compatible-provider.spec.ts | 35 +++++++++++++++++++ src/api/providers/__tests__/kimi-code.spec.ts | 8 +++-- src/api/providers/__tests__/zai.spec.ts | 32 ++++++++++++----- .../base-openai-compatible-provider.ts | 9 ++++- 4 files changed, 72 insertions(+), 12 deletions(-) diff --git a/src/api/providers/__tests__/base-openai-compatible-provider.spec.ts b/src/api/providers/__tests__/base-openai-compatible-provider.spec.ts index 31494eb4ef..532778022c 100644 --- a/src/api/providers/__tests__/base-openai-compatible-provider.spec.ts +++ b/src/api/providers/__tests__/base-openai-compatible-provider.spec.ts @@ -434,12 +434,47 @@ describe("BaseOpenAiCompatibleProvider", () => { const requestOptions = mockCreate.mock.calls.at(-1)?.[1] expect(requestOptions?.signal).toBeInstanceOf(AbortSignal) expect(requestOptions?.signal.aborted).toBe(false) + // A positive timeoutMs must be forwarded as the per-request SDK timeout. + expect(requestOptions?.timeout).toBe(5000) // A timeout-only merged signal would still pass the assertions above; // aborting the caller's controller proves caller cancellation survives. controller.abort() expect(requestOptions?.signal.aborted).toBe(true) }) + it("should abort the request when a timeout-only completePrompt timeoutMs elapses", async () => { + // Emulate the OpenAI SDK: the pending request rejects when its signal aborts. + let capturedOptions: { signal?: AbortSignal; timeout?: number } | undefined + mockCreate.mockImplementationOnce( + async (_params: unknown, options?: { signal?: AbortSignal; timeout?: number }) => { + capturedOptions = options + await new Promise((resolve) => { + if (options?.signal?.aborted) { + resolve() + } else { + options?.signal?.addEventListener("abort", () => resolve(), { once: true }) + } + }) + throw new APIUserAbortError() + }, + ) + + // No caller signal: the timeout branch is exercised on its own. AbortSignal.timeout + // is backed by a native self-managed timer (not the fakeable global), so poll with + // vi.waitFor the same way abort-signal.spec.ts does. + const requestPromise = handler.completePrompt("test prompt", { timeoutMs: 50 }) + // Attach the rejection handler immediately so the timeout rejection + // is never observed as unhandled while we poll for the abort. + const resultPromise = captureError(requestPromise) + + await vi.waitFor(() => expect(capturedOptions?.signal?.aborted).toBe(true)) + expect(capturedOptions?.timeout).toBe(50) + + const result = await resultPromise + expect(result.name).toBe("AbortError") + expect(result.message).toBe("TestProvider request aborted") + }) + it("should not set a request signal for zero completePrompt timeoutMs", async () => { mockCreate.mockResolvedValueOnce({ choices: [{ message: { content: "response" } }] }) diff --git a/src/api/providers/__tests__/kimi-code.spec.ts b/src/api/providers/__tests__/kimi-code.spec.ts index bbd1e1b4a9..99262db641 100644 --- a/src/api/providers/__tests__/kimi-code.spec.ts +++ b/src/api/providers/__tests__/kimi-code.spec.ts @@ -274,7 +274,9 @@ describe("KimiCodeHandler", () => { }) it("rejects before any request when the createMessage abort signal is already aborted", async () => { - const handler = new KimiCodeHandler({ kimiCodeAuthMethod: "api-key", kimiCodeApiKey: "key" }) + // OAuth auth so the token assertions below are not vacuous: without the + // cancellation guard, resolveAccessToken would invoke the OAuth mocks. + const handler = new KimiCodeHandler({ kimiCodeAuthMethod: "oauth" }) const controller = new AbortController() controller.abort() const createCompletion = completionsCreate(handler) @@ -313,7 +315,9 @@ describe("KimiCodeHandler", () => { }) it("rejects before any request when the completePrompt signal is already aborted", async () => { - const handler = new KimiCodeHandler({ kimiCodeAuthMethod: "api-key", kimiCodeApiKey: "key" }) + // OAuth auth so the token assertions below are not vacuous: without the + // cancellation guard, resolveAccessToken would invoke the OAuth mocks. + const handler = new KimiCodeHandler({ kimiCodeAuthMethod: "oauth" }) const controller = new AbortController() controller.abort() const createCompletion = completionsCreate(handler) diff --git a/src/api/providers/__tests__/zai.spec.ts b/src/api/providers/__tests__/zai.spec.ts index 02751434ce..d0edf12092 100644 --- a/src/api/providers/__tests__/zai.spec.ts +++ b/src/api/providers/__tests__/zai.spec.ts @@ -742,31 +742,45 @@ describe("ZAiHandler", () => { expect(result.message).toBe("Z.ai request aborted") }) - it("glm-5.3 completePrompt should merge the abort signal and timeoutMs on the thinking path", async () => { + it("glm-5.3 completePrompt should cancel the in-flight request when the caller signal aborts", async () => { const h53 = new ZAiHandler({ apiModelId: "glm-5.3", zaiApiKey: "test-zai-api-key", zaiApiLine: "international_coding", }) const controller = new AbortController() - mockCreate.mockResolvedValueOnce({ choices: [{ message: { content: "response" } }] }) + // Emulate the OpenAI SDK: the pending request rejects when its signal aborts. + mockCreate.mockImplementationOnce( + async (_params: unknown, options?: { signal?: AbortSignal; timeout?: number }) => { + await new Promise((resolve) => { + if (options?.signal?.aborted) { + resolve() + } else { + options?.signal?.addEventListener("abort", () => resolve(), { once: true }) + } + }) + throw new APIUserAbortError() + }, + ) + + // The request stays pending while the caller's signal is still live. + const requestPromise = h53.completePrompt("prompt", { abortSignal: controller.signal, timeoutMs: 5000 }) + controller.abort() - const result = await h53.completePrompt("prompt", { abortSignal: controller.signal, timeoutMs: 5000 }) + const result = await captureError(requestPromise) + expect(result.name).toBe("AbortError") + expect(result.message).toBe("Z.ai request aborted") - expect(result).toBe("response") const requestCall = mockCreate.mock.calls.at(-1) expect(requestCall?.[0]).toEqual( expect.objectContaining({ model: "glm-5.3", thinking: { type: "enabled", clear_thinking: false } }), ) const requestOptions = requestCall?.[1] expect(requestOptions?.signal).toBeInstanceOf(AbortSignal) - expect(requestOptions?.signal.aborted).toBe(false) // A positive timeoutMs must be forwarded as the per-request SDK timeout. expect(requestOptions?.timeout).toBe(5000) - // A timeout-only merged signal would still pass the assertions above; - // aborting the caller's controller proves caller cancellation survives. - controller.abort() - expect(requestOptions?.signal.aborted).toBe(true) + // Aborting the caller's controller propagates to the merged request signal. + expect(requestOptions?.signal?.aborted).toBe(true) }) it("glm-5.3 completePrompt should normalize the SDK APIUserAbortError", async () => { diff --git a/src/api/providers/base-openai-compatible-provider.ts b/src/api/providers/base-openai-compatible-provider.ts index bba5b33da8..289d088dae 100644 --- a/src/api/providers/base-openai-compatible-provider.ts +++ b/src/api/providers/base-openai-compatible-provider.ts @@ -28,6 +28,8 @@ type BaseOpenAiCompatibleProviderOptions = ApiHandlerO /** Subset of OpenAI.RequestOptions built per request for the abort-signal wiring. */ type OpenAiRequestConfig = { signal?: AbortSignal + /** Per-request SDK timeout (ms); overrides the client-level default when set. */ + timeout?: number } export abstract class BaseOpenAiCompatibleProvider @@ -265,9 +267,14 @@ export abstract class BaseOpenAiCompatibleProvider // CompletePromptOptions is not ApiHandlerCreateMessageMetadata (required taskId), // so the abort/timeout merge goes through setOption; the helper treats - // timeoutMs <= 0 as "no explicit timeout". + // timeoutMs <= 0 as "no explicit timeout". The per-request timeout is + // forwarded as well: without it the SDK falls back to the client-level + // default, which can still expire before a larger per-request timeoutMs. + const requestTimeout = + typeof options?.timeoutMs === "number" && options.timeoutMs > 0 ? options.timeoutMs : undefined const requestConfig = new RequestConfigBuilder() .setOption("signal", mergeAbortSignalAndTimeout(options?.abortSignal, options?.timeoutMs)) + .setOption("timeout", requestTimeout) .build() try {