Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
6f00ee9
fix(core): recognize OpenAI SDK APIUserAbortError as an abort
harjothkhara Aug 2, 2026
2fad2fd
test(core): pin the abort match against sibling SDK errors
harjothkhara Aug 3, 2026
ad700bf
fix(core): suppress the api_error telemetry event on user cancels
harjothkhara Aug 3, 2026
1868550
test(core): pin the cancel gate's truth table and its stream call site
harjothkhara Aug 4, 2026
6fe680f
Merge branch 'main' into oss-find/qwen-code-2026-08-02
wenshao Aug 4, 2026
546e192
test(core): pin the setup and DOMException cancel paths, correct a ra…
harjothkhara Aug 4, 2026
a433efd
Merge branch 'main' into oss-find/qwen-code-2026-08-02
wenshao Aug 5, 2026
63237d2
fix(core): keep reporting timed-out side queries, not just user cancels
harjothkhara Aug 6, 2026
ebf497e
fix(core): signal internal deadlines as TimeoutError so they stay rep…
harjothkhara Aug 6, 2026
ea1cf10
Merge branch 'main' into oss-find/qwen-code-2026-08-02
wenshao Aug 7, 2026
ebb9c25
fix(core): share the timeout-abort contract and convert its fifth pro…
harjothkhara Aug 7, 2026
577c499
fix(cli): signal the voice-refine deadline as TimeoutError
harjothkhara Aug 8, 2026
fcd299c
Merge remote-tracking branch 'origin/main' into oss-find/qwen-code-20…
harjothkhara Aug 8, 2026
77d2d44
fix(core): signal the goal checkpoint verifier deadline as TimeoutError
harjothkhara Aug 8, 2026
262bfa5
docs(core): design note for the user-cancel vs internal-deadline cont…
harjothkhara Aug 8, 2026
1d8072b
docs(core): record the daemon prompt deadline as an open boundary case
harjothkhara Aug 9, 2026
aa6eca2
refactor(core): reduce #8399 to the #8398 fix, split out the invariant
harjothkhara Aug 9, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
import { isTelemetrySdkInitialized } from '../../telemetry/index.js';
import { OpenAILogger } from '../../utils/openaiLogger.js';
import type OpenAI from 'openai';
import { APIUserAbortError } from 'openai';
import { setGenAiUsageProvenance } from '../../telemetry/gen-ai-usage.js';

const activeOtelContext = vi.hoisted(() => ({ current: 'root' }));
Expand Down Expand Up @@ -764,6 +765,64 @@ describe('LoggingContentGenerator', () => {
).toHaveBeenCalledWith(false);
});

it('does not emit an api_error event when the user cancelled the request', async () => {
// A user cancel surfaces as the SDK's APIUserAbortError with the caller's
// signal aborted. It must not be reported as a qwen-code.api_error — the
// span already records the cancellation. Regression for #8356 at the layer
// the bug is about: the util-level isAbortError fix alone does not gate this
// separate telemetry path.
const wrapped = createWrappedGenerator(
vi
.fn()
.mockRejectedValue(
new APIUserAbortError({ message: 'Request was aborted.' }),
),
vi.fn(),
);
const generator = new LoggingContentGenerator(wrapped, createConfig(), {
model: 'test-model',
authType: AuthType.USE_OPENAI,
enableOpenAILogging: false,
});
const controller = new AbortController();
controller.abort();
const request = {
model: 'test-model',
contents: 'Hello',
config: { abortSignal: controller.signal },
} as unknown as GenerateContentParameters;

await expect(
generator.generateContent(request, 'prompt-cancel'),
).rejects.toBeInstanceOf(APIUserAbortError);

expect(logApiError).not.toHaveBeenCalled();
});

it('still emits an api_error event for a real failure that is not a cancel', async () => {
// The gate is scoped to genuine user cancels: a non-abort failure, even
// with an unrelated aborted flag absent, must still be reported.
const wrapped = createWrappedGenerator(
vi.fn().mockRejectedValue(new Error('upstream-down')),
vi.fn(),
);
const generator = new LoggingContentGenerator(wrapped, createConfig(), {
model: 'test-model',
authType: AuthType.USE_OPENAI,
enableOpenAILogging: false,
});
const request = {
model: 'test-model',
contents: 'Hello',
} as unknown as GenerateContentParameters;

await expect(
generator.generateContent(request, 'prompt-real-error'),
).rejects.toThrow('upstream-down');

expect(logApiError).toHaveBeenCalledTimes(1);
});

it('forwards usage attached to the final response after it was yielded', async () => {
const response = createResponse('r1', 'test-model', [{ text: 'a' }]);
const streamFn = vi.fn().mockResolvedValue(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ import {
getErrorMessage,
getErrorStatus,
getErrorType,
isAbortError,
} from '../../utils/errors.js';
import {
startLLMRequestSpan,
Expand Down Expand Up @@ -262,7 +263,18 @@ export class LoggingContentGenerator implements ContentGenerator {
error: unknown,
model: string,
prompt_id: string,
abortSignal?: AbortSignal,
): void {
// A user cancel is not an API error. When the caller's signal is aborted
// and the error is abort-shaped, skip the api_error event entirely — the
// span already records the cancellation through its aborted status, so the
// signal isn't lost. Without this gate a user cancel is still emitted as a
// `qwen-code.api_error` event with error_type `APIUserAbortError`, which is
// exactly what #8356 reported; `isAbortError` gating the debug log alone
// does not cover this separate telemetry path.
if (abortSignal?.aborted && isAbortError(error)) {
return;
}
Comment on lines +274 to +276

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Critical] The gate keys only on the state of the caller's signal, never on who fired it. Several in-repo callers compose timeout-based abort signals into model requests — permissions/classifier.ts:170 composes AbortSignal.any([input.signal, AbortSignal.timeout(stage1TimeoutMs)]), memory/relevanceSelector.ts:113 uses AbortSignal.timeout(30_000), and memory/forget.ts:193 uses AbortSignal.timeout(8_000), all routing through runSideQuery → baseLlmClient → req.config.abortSignal. When the timeout fires, Node sets signal.reason to a DOMException named TimeoutError and AbortSignal.any propagates it — but this gate checks only abortSignal?.aborted (true) and isAbortError(error) (true for APIUserAbortError, which the SDK throws for any aborted signal). — Failure scenario: provider latency spike → classifier/memory side query blows its AbortSignal.timeout budget → SDK rejects abort-shaped → gate suppresses the api_error event → at 3 AM the incident shows zero api_errors and a clean model-health chart while background LLM work silently fails. Probe-verified: with a TimeoutError-reason signal and APIUserAbortError, logApiError is called 0 times; adding signal.reason.name !== 'TimeoutError' flips the probe to pass with all 8 existing cancel tests still green.

Suggested change
if (abortSignal?.aborted && isAbortError(error)) {
return;
}
const reason: unknown = abortSignal?.reason;
const isTimeoutAbort = reason instanceof Error && reason.name === 'TimeoutError';
if (abortSignal?.aborted && !isTimeoutAbort && isAbortError(error)) {
return;
}

Node sets signal.reason to a DOMException named TimeoutError for AbortSignal.timeout(), and AbortSignal.any propagates the firing source's reason; please also add a test pinning the timeout case.

中文说明

[Critical] 该门控只依赖调用方 signal 的状态,从不区分是谁触发了它。仓库中多个调用方会向模型请求传入基于超时的 abort signal——permissions/classifier.ts:170 组合 AbortSignal.any([input.signal, AbortSignal.timeout(stage1TimeoutMs)]),memory/relevanceSelector.ts:113 使用 AbortSignal.timeout(30_000),memory/forget.ts:193 使用 AbortSignal.timeout(8_000),均经 runSideQuery → baseLlmClient → req.config.abortSignal 传入。超时触发时,Node 会把 signal.reason 设为名为 TimeoutError 的 DOMException,AbortSignal.any 会传播该 reason——但此门控仅检查 abortSignal?.aborted(true)和 isAbortError(error)(对 APIUserAbortError 为 true,SDK 对任何已中止 signal 都会抛出)。——失败场景:provider 延迟尖峰 → classifier/memory 侧查询超出超时预算 → SDK 以中止形态拒绝 → 门控抑制 api_error 事件 → 凌晨 3 点的故障中 api_error 为零、模型健康图表干净,后台 LLM 任务悄无声息地失败。已用探针验证:TimeoutError reason 的 signal 加 APIUserAbortError 时,logApiError 被调用 0 次;增加 signal.reason.name !== 'TimeoutError' 后探针翻转为通过,全部 8 个已有取消测试保持绿色。

— qwen3.7-max via Qwen Code /review (v0.21.9)

try {
this._logApiError(responseId, durationMs, error, model, prompt_id);
} catch (loggingError) {
Expand Down Expand Up @@ -402,7 +414,14 @@ export class LoggingContentGenerator implements ContentGenerator {
config: this.config,
});
await context.with(spanContext, async () => {
this.safelyLogApiError('', durationMs, error, req.model, userPromptId);
this.safelyLogApiError(
'',
durationMs,
error,
req.model,
userPromptId,
req.config?.abortSignal,
);
try {
await this.safelyLogOpenAIInteraction(
await session.resolve(req),
Expand Down Expand Up @@ -490,7 +509,14 @@ export class LoggingContentGenerator implements ContentGenerator {
const durationMs = Date.now() - startTime;
const observedFinishReasons = exchange.controller.finalize(false);
context.with(spanContext, () =>
this.safelyLogApiError('', durationMs, error, req.model, userPromptId),
this.safelyLogApiError(
'',
durationMs,
error,
req.model,
userPromptId,
req.config?.abortSignal,
),
);
const aborted = req.config?.abortSignal?.aborted ?? false;
endLLMRequestSpan(llmSpan, {
Expand Down Expand Up @@ -802,6 +828,7 @@ export class LoggingContentGenerator implements ContentGenerator {
error,
firstModelVersion || model,
userPromptId,
abortSignal,
),
);
const openaiRequest = await openaiRequestPromise;
Expand Down
40 changes: 40 additions & 0 deletions packages/core/src/utils/errors.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
*/

import { describe, it, expect } from 'vitest';
import { APIUserAbortError as AnthropicAPIUserAbortError } from '@anthropic-ai/sdk';
import { APIConnectionError, APIUserAbortError } from 'openai';
import { getErrorMessage, isAbortError, isNodeError } from './errors.js';

describe('getErrorMessage cause unwrapping', () => {
Expand Down Expand Up @@ -231,6 +233,44 @@ describe('isAbortError', () => {

expect(isAbortError(networkError)).toBe(false);
});

it('should return true for the OpenAI SDK APIUserAbortError (user cancel)', () => {
// The OpenAI SDK is the request path for auth_type=openai; a user cancel
// surfaces as APIUserAbortError. It does not set `.name` (stays 'Error')
// and has no ABORT_ERR code, so the checks above miss it.
const error = new APIUserAbortError({ message: 'Request was aborted.' });

// Assert the requirement (the name-based branch can't match it) rather than
// the SDK internal `.name === 'Error'`, which would break if OpenAI/Anthropic
// ever set a name without changing the correct behavior here.
expect(error.name).not.toBe('AbortError');
expect(isAbortError(error)).toBe(true);
});

it('should return true for the Anthropic SDK APIUserAbortError', () => {
// Both SDKs this package depends on are Stainless-generated and share the
// abort class name, so the same check covers auth_type=anthropic. Pinned so
// the cross-SDK coverage is intentional rather than incidental.
const error = new AnthropicAPIUserAbortError({
message: 'Request was aborted.',
});

// Assert the requirement (the name-based branch can't match it) rather than
// the SDK internal `.name === 'Error'`, which would break if OpenAI/Anthropic
// ever set a name without changing the correct behavior here.
expect(error.name).not.toBe('AbortError');
expect(isAbortError(error)).toBe(true);
});

it('should return false for other SDK errors such as APIConnectionError', () => {
// Guards the abort match against being broadened (e.g. to any `API*`
// class): a transient connection failure must stay retryable, not be
// reported as a user cancellation.
const error = new APIConnectionError({ message: 'Connection error.' });

expect(error.constructor.name).toBe('APIConnectionError');
expect(isAbortError(error)).toBe(false);
});
});

describe('isNodeError', () => {
Expand Down
21 changes: 20 additions & 1 deletion packages/core/src/utils/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ export function isNodeError(error: unknown): error is NodeJS.ErrnoException {

/**
* Check if the error is an abort error (user cancellation).
* This handles both DOMException-style AbortError and Node.js abort errors.
* This handles DOMException-style AbortError, Node.js abort errors, and the
* provider SDKs' `APIUserAbortError` (matched by class name).
*/
export function isAbortError(error: unknown): boolean {
if (!error || typeof error !== 'object') {
Expand All @@ -35,6 +36,24 @@ export function isAbortError(error: unknown): boolean {
return true;
}

// A user cancel on a provider SDK path surfaces as `APIUserAbortError`. That
// class does not set `.name` (it stays 'Error') and carries no ABORT_ERR
// code, so the checks above miss it — leaving user cancels logged as
// api_errors and classified 'unknown' rather than 'abort'. Both SDKs this
// package depends on (`openai`, `@anthropic-ai/sdk`) are Stainless-generated
// and share this class name, so one check covers both. Match on the class
// name — as `getErrorType` below already does for SDK errors — so this
// provider-agnostic util needs no SDK import. The CLI bundle preserves class
// names (esbuild `keepNames: true` in esbuild.config.js); other bundles that
// minify without keepNames (e.g. vscode-ide-companion) do not, but they don't
// drive provider SDK requests, so the match holds where it runs.
if (
error instanceof Error &&
error.constructor?.name === 'APIUserAbortError'
) {
return true;
}

return false;
}

Expand Down
12 changes: 12 additions & 0 deletions packages/core/src/utils/retryErrorClassification.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

import { APIUserAbortError } from 'openai';
import { describe, expect, it } from 'vitest';
import { AuthType } from '../core/contentGenerator.js';
import {
Expand All @@ -23,6 +24,17 @@ describe('classifyRetryError', () => {
});
});

it('classifies the OpenAI SDK APIUserAbortError as an abort, not unknown', () => {
// A user cancel on the auth_type=openai path surfaces as APIUserAbortError.
// It must be treated as an abort so retries stop and it is not logged as an
// api_error — otherwise it falls through to the 'unknown' classification.
expect(
classifyRetryError(
new APIUserAbortError({ message: 'Request was aborted.' }),
),
).toMatchObject({ kind: 'abort' });
});

it('classifies HTTP 503 as retryable rate limiting to match stream retry semantics', () => {
expect(
classifyRetryError({ status: 503, message: 'Provider overloaded' }),
Expand Down
Loading