Skip to content

fix(core): auto-retry transient network errors during API calls - #7898

Merged
wenshao merged 2 commits into
QwenLM:mainfrom
chiga0:fix/retry-network-errors
Jul 28, 2026
Merged

fix(core): auto-retry transient network errors during API calls#7898
wenshao merged 2 commits into
QwenLM:mainfrom
chiga0:fix/retry-network-errors

Conversation

@chiga0

@chiga0 chiga0 commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

What this PR does

Adds transient network errors (ECONNRESET, ETIMEDOUT, ECONNREFUSED, etc.) to the set of conditions that trigger automatic retry at the API-call level. Previously, these TCP-level errors carried no HTTP status code and fell through every existing retry predicate, surfacing as raw [API Error: terminated (cause: read ECONNRESET)] to the user. The fix wires the existing transport-error classification into both retry decision points so that network glitches are retried with exponential backoff, just like rate-limit and server errors already are.

Why it's needed

Closes #7831.

Users with large contexts (150k+ tokens) experience repeated ECONNRESET failures when a server-side gateway timeout (~90s) kills the TCP connection before the model finishes streaming. The next manual retry always succeeds immediately, confirming these are transient. The client already classifies these errors as transport-retryable and already retries them mid-stream (when no chunk has been yielded), but the API-call-level retry path did not consult that classification — so the error propagated to the user instead of being retried automatically.

Reviewer Test Plan

How to verify

  1. Run the unit tests: cd packages/core && npx vitest run src/utils/retry.test.ts — confirm the two new tests ("should retry on transient network errors (ECONNRESET) by default" and "should retry on ETIMEDOUT by default") pass alongside all existing tests.
  2. Confirm that existing retry behavior is unchanged: rate-limit (429/503), 5xx, and fail-fast quota 429 bounded-retry all still work as before — the new transport check is appended after these predicates and does not alter their logic.
  3. Optionally, simulate a network error against a real endpoint and observe that the CLI retries automatically instead of showing a raw error.

Evidence (Before & After)

N/A (non-UI change; retry behavior is internal)

Tested on

OS Status
🍏 macOS
🪟 Windows N/A
🐧 Linux N/A

Environment (optional)

Unit tests only (npx vitest run).

Risk & Scope

  • Main risk or tradeoff: transient network errors that indicate a persistent outage (e.g., server permanently unreachable) will now be retried up to the configured max attempts before surfacing, adding a short delay. This matches the existing behavior for 5xx errors and is bounded by the retry budget.
  • Not validated / out of scope: the server-side gateway timeout root cause (~90s SLB timeout on Bailian Token Plan) is tracked separately. Mid-stream transport retry (after chunks have been yielded) is unchanged.
  • Breaking changes / migration notes: none.

Linked Issues

Closes #7831

中文说明

本 PR 做了什么

将瞬态网络错误(ECONNRESET、ETIMEDOUT、ECONNREFUSED 等)加入 API 调用级别的自动重试条件。此前,这些 TCP 层错误不携带 HTTP 状态码,会穿透所有现有的重试判定逻辑,以原始的 [API Error: terminated (cause: read ECONNRESET)] 形式直接暴露给用户。本次修复将已有的传输层错误分类接入两个重试判定点,使网络抖动像限流和服务端错误一样自动进行指数退避重试。

为什么需要

关闭 #7831

大上下文(150k+ tokens)用户在服务端网关超时(约 90 秒)切断 TCP 连接时,会反复遇到 ECONNRESET 失败。每次手动重试都立即成功,证实这些是瞬态错误。客户端已将这些错误归类为可重试的传输层错误,且已在流式传输中途(未产出任何 chunk 时)进行重试,但 API 调用级别的重试路径未引用该分类——因此错误直接传播给用户,而非自动重试。

审阅者测试计划

如何验证

  1. 运行单元测试:cd packages/core && npx vitest run src/utils/retry.test.ts——确认两个新测试("should retry on transient network errors (ECONNRESET) by default" 和 "should retry on ETIMEDOUT by default")与所有现有测试一起通过。
  2. 确认现有重试行为未变:限流(429/503)、5xx、以及快速失败配额 429 有限重试均保持原有逻辑——新的传输层检查追加在这些判定之后,不改变其逻辑。
  3. 可选:对真实端点模拟网络错误,观察 CLI 自动重试而非显示原始错误。

证据(前后对比)

N/A(非 UI 变更;重试行为为内部逻辑)

测试环境

操作系统 状态
🍏 macOS
🪟 Windows N/A
🐧 Linux N/A

环境(可选)

仅单元测试(npx vitest run)。

风险与范围

  • 主要风险或权衡:如果瞬态网络错误实际代表持续性故障(如服务器永久不可达),现在会在达到配置的最大重试次数后才暴露,增加短暂延迟。这与 5xx 错误的现有行为一致,且受重试预算约束。
  • 未验证 / 不在范围内:服务端网关超时根因(百炼 Token Plan 约 90 秒 SLB 超时)另行跟踪。流式传输中途的传输层重试(已产出 chunk 后)未做变更。
  • 破坏性变更 / 迁移说明:无。

关联 Issue

关闭 #7831

…d API-call predicate

The retry classification layer (classifyRetryError) already recognizes
transport errors (ECONNRESET, ETIMEDOUT, etc.) as retryable, but neither
defaultShouldRetry nor the inline shouldRetryOnError in geminiChat.ts
consulted the classification for transport codes. TCP-level errors carry
no HTTP status, so they fell through every predicate and propagated as
raw [API Error: terminated (cause: read ECONNRESET)].

Wire the existing classifyRetryError transport detection into both
retry decision points:

1. defaultShouldRetry in retry.ts — append a transport-kind check after
   the existing rate-limit / 5xx predicates, preserving all current
   behavior (including bounded retry for fail-fast quota 429s).
2. Inline shouldRetryOnError in geminiChat.ts makeApiCallAndProcessStream —
   add the same transport-kind check so the API-call-level retryWithBackoff
   wrapper covers network errors.

The stream-level transport retry (gated on !streamYieldedChunk) already
exists and is unchanged.

Closes QwenLM#7831
@github-actions github-actions Bot added the review/self-reported The linked issue was opened by the PR author (self-reported) label Jul 28, 2026
@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Qwen Triage finished — CI landed green on e6122b3 and the deferred approval was posted. finalize run

Qwen Triage 已完成 —— e6122b3 的 CI 全绿,延迟审批已提交。查看 finalize 运行

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

Template: the headings differ from the template (Summary/Changes/Why this is safe vs What this PR does/Why it's needed/Reviewer Test Plan), but the content is all there — not worth a round-trip for heading names on a PR this size.

Problem: observed bug with strong evidence. Issue #7831 documents five ECONNRESET occurrences with timestamps, durations clustering around 81–95s, and telemetry showing the pattern only appears at 150k+ token context. The retry-succeeds-immediately observation confirms this is transient. Clearly a real problem.

Direction: squarely aligned — retrying transient transport errors is basic API resilience. The classification layer (classifyRetryError) already knows about these codes; the retry predicates just weren't consulting it. CHANGELOG: no direct reference, but the area is clearly relevant.

Size: 18 production lines (9 in geminiChat.ts, 9 in retry.ts) + 49 test lines. Well under any threshold. Not applicable.

Approach: the scope feels exactly right. Two small additions wiring an existing, well-tested classifier into the two retry decision points that were missing it. No new abstractions, no new files, no drive-by changes. The stream-level transport retry already exists separately and is untouched.

Moving on to code review. 🔍

中文说明

感谢贡献!

模板:标题格式与模板不同(Summary/Changes/Why this is safe vs What this PR does/Why it's needed/Reviewer Test Plan),但内容完整——对于这么小的 PR,不值得为标题格式来回修改。

问题:已观测到的 bug,证据充分。Issue #7831 记录了五次 ECONNRESET,时间戳、持续时间集中在 81–95 秒,遥测数据显示仅在 150k+ token 上下文时出现。重试立即成功确认这是瞬态错误。确实是真实问题。

方向:完全对齐——重试瞬态传输错误是基本的 API 弹性。分类层(classifyRetryError)已经识别这些错误码,只是重试判断函数没有查询它。

规模:18 行生产代码(geminiChat.ts 9 行,retry.ts 9 行)+ 49 行测试代码。远低于任何阈值。不适用。

方案:范围恰好。两处小改动,将现有的、经过充分测试的分类器接入两个缺少它的重试判断点。没有新抽象、没有新文件、没有顺手改动。流级传输重试已经独立存在,未被触及。

进入代码审查 🔍

Qwen Code · qwen3.8-max-preview

Reviewed at e6122b3d679ae1ce0e83d8f649ca37dce2e7090c · re-run with @qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Code Review

Independent proposal: given that classifyRetryError already recognizes transport codes (ECONNRESET, ETIMEDOUT, etc.) but neither defaultShouldRetry nor the inline shouldRetryOnError in geminiChat.ts consults it, I would add a kind === 'transport' check to both predicates. That is exactly what this PR does.

Findings: no critical blockers, no convention violations.

The retry.ts change restructures the return into an early-return for the existing rate-limit/5xx predicates, then falls through to the transport check. Logic is preserved — the existing predicates are evaluated first, and the transport check only fires for errors that carry no HTTP status (or a 5xx status with a transport cause, which classifyRetryError correctly classifies as transport). The classifyRetryError call here is lightweight (no I/O, just object field inspection), and the function is already called again in the retryWithBackoff catch block for diagnostics — the double call is negligible.

The geminiChat.ts change adds the same transport check to the inline predicate, placed after the existing isRateLimitError check and before return false. This is necessary because the inline predicate bypasses defaultShouldRetry. The early returns for schema errors, invalid arguments, and status 400 all precede the new check, so those exclusions are preserved.

Both tests match the real error shapes from the issue (TypeError with nested cause for ECONNRESET, direct Error with code for ETIMEDOUT) and follow the existing test patterns (fake timers, attempt counting).

One minor observation: defaultShouldRetry passes { extraRetryErrorCodes } to classifyRetryError but not authType. This is correct — authType only matters for the Qwen OAuth quota check, which is irrelevant to transport classification.

Testing

Final CI results for e6122b3 (auto-updated by the triage finalize job after CI completed):

Check Conclusion
Classify PR ✅ success
route ✅ success
Test (ubuntu-latest, Node 22.x) ✅ success
web-shell E2E Smoke (ubuntu-latest, Node 22.x) ✅ success

One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。

The ubuntu unit test suite is still running. macOS and Windows tests were skipped (likely gated on ubuntu passing first). Precheck and classification checks passed. The author reports all 126 tests in retry.test.ts + retryErrorClassification.test.ts pass and typecheck is clean — noted as the author's claim, not independently verified here.

Not verified: real-scenario ECONNRESET reproduction (requires a live API endpoint with the specific gateway timeout conditions from #7831).

中文说明

代码审查

独立方案: 鉴于 classifyRetryError 已经识别传输错误码(ECONNRESET、ETIMEDOUT 等),但 defaultShouldRetry 和 geminiChat.ts 中的内联 shouldRetryOnError 都没有查询它,我会在两个判断函数中添加 kind === 'transport' 检查。这正是本 PR 所做的。

发现: 无关键阻塞问题,无规范违反。

retry.ts 的改动将 return 重构为对现有速率限制/5xx 判断的提前返回,然后落入传输检查。逻辑保持不变——现有判断先执行,传输检查仅对没有 HTTP 状态码的错误生效。classifyRetryError 调用是轻量级的(无 I/O,仅对象字段检查),且该函数已在 retryWithBackoff 的 catch 块中再次调用用于诊断——双重调用开销可忽略。

geminiChat.ts 的改动在内联判断函数中添加了相同的传输检查,位于现有 isRateLimitError 检查之后、return false 之前。这是必要的,因为内联判断函数绕过了 defaultShouldRetry。schema 错误、无效参数和 status 400 的提前返回都在新检查之前,因此这些排除被保留。

两个测试匹配 issue 中的真实错误形状(ECONNRESET 的嵌套 cause TypeError,ETIMEDOUT 的直接 Error with code),并遵循现有测试模式。

测试

Ubuntu 单元测试套件仍在运行。macOS 和 Windows 测试被跳过。预检和分类检查通过。作者报告 retry.test.ts + retryErrorClassification.test.ts 中全部 126 个测试通过,类型检查干净——记为作者声明,非独立验证。

未验证:真实场景 ECONNRESET 复现(需要具有 #7831 中特定网关超时条件的活跃 API 端点)。

Qwen Code · qwen3.8-max-preview

Reviewed at e6122b3d679ae1ce0e83d8f649ca37dce2e7090c · re-run with @qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Confidence: 5/5 — clean across every stage; would merge without hesitation.

This is exactly the kind of PR I like to see. The problem is real and well-documented (five ECONNRESET occurrences with telemetry in #7831), the root cause is clear (transport errors carry no HTTP status, so they fell through every retry predicate), and the fix is the minimum viable change: wire the existing classifier into the two places that weren't consulting it. Eighteen production lines, no new abstractions, no scope creep.

My independent proposal matched the PR's approach exactly — I couldn't find a simpler path. The code reads well, the comments explain the "why" (transport errors carry no HTTP status), and the tests use the real error shapes from the issue. The existing behavior is fully preserved: rate-limit, 5xx, schema error, invalid argument, and 400 exclusions all still fire before the new transport check.

The one thing I can't verify from here is the real-scenario behavior — reproducing the specific gateway timeout that triggers ECONNRESET requires the live Bailian endpoint under load. But the unit tests cover the retry mechanics, and the classification layer already has comprehensive transport-code coverage. The change is additive and bounded; the risk of regression is near zero.

Approval deferred until CI lands green on e6122b3d679ae1ce0e83d8f649ca37dce2e7090c.

中文说明

置信度:5/5 — 每个阶段都很干净;毫不犹豫可以合并。

这正是我喜欢看到的 PR。问题真实且记录充分(#7831 中有五次 ECONNRESET 及遥测数据),根因清晰(传输错误没有 HTTP 状态码,因此穿过了所有重试判断),修复是最小可行改动:将现有分类器接入两个未查询它的地方。十八行生产代码,没有新抽象,没有范围蔓延。

我的独立方案与 PR 的方法完全一致——找不到更简单的路径。代码可读性好,注释解释了"为什么"(传输错误没有 HTTP 状态码),测试使用了 issue 中的真实错误形状。现有行为完全保留:速率限制、5xx、schema 错误、无效参数和 400 排除都在新的传输检查之前触发。

唯一无法从这里验证的是真实场景行为——复现触发 ECONNRESET 的特定网关超时需要在负载下的活跃百炼端点。但单元测试覆盖了重试机制,分类层已有全面的传输错误码覆盖。改动是加法且有界的;回归风险接近零。

审批推迟到 CI 在 e6122b3d679ae1ce0e83d8f649ca37dce2e7090c 上全部通过后执行。

Qwen Code · qwen3.8-max-preview

Reviewed at e6122b3d679ae1ce0e83d8f649ca37dce2e7090c · re-run with @qwen-code /triage

@chiga0

chiga0 commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator Author

@qwen-bot /takeover

@qwen-code-ci-bot qwen-code-ci-bot left a comment

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.

LGTM, looks ready to ship — CI landed green after the review. ✅

@chiga0 chiga0 changed the title fix(core): retry on transient network errors in defaultShouldRetry and API-call predicate fix(core): auto-retry transient network errors during API calls Jul 28, 2026
@chiga0

chiga0 commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator Author

@qwen-code /takeover

@chiga0

chiga0 commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator Author

补充说明

本 PR 只修改了 defaultShouldRetry 和 geminiChat 内联 shouldRetryOnError 两个决策点,利用已有的 classifyRetryError transport 分类。stream 层的 transport retry(stream-transport-retry.ts,gated on !streamYieldedChunk)保持不变。

这意味着:

  • API 调用阶段(连接建立前/首 token 前)的 ECONNRESET → 现在会自动重试 ✅
  • Stream 阶段、首 token 前的 ECONNRESET → 已有的 transport retry 覆盖 ✅
  • Stream 阶段、首 token 后的 ECONNRESET → 仍然不重试(避免重复输出),这是上游有意为之的设计取舍

对于 #7831 的场景(150k+ context,~90s gateway timeout),如果 TTFT > 90s(大 context 下很可能),则首 token 前连接就被 reset,本 PR 的 API-call 层重试 + 已有的 stream transport retry 都能覆盖。如果 TTFT < 90s 但生成过程中被 reset,则仍会报错 — 这需要服务端调高 gateway timeout 来根治。

@wenshao

wenshao commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /takeover

@qwen-code-dev-bot qwen-code-dev-bot added the autofix/takeover Summon the autofix loop to manage this PR (remove to release; needs triage+) label Jul 28, 2026
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. Remove the autofix/takeover label (or comment @qwen-code /takeover stop) to release.

中文说明

🤝 已接管:autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突,直到移除标签或达到轮次上限。移除 autofix/takeover 标签(或评论 @qwen-code /takeover stop)即可释放。

@qwen-code-dev-bot

qwen-code-dev-bot commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

AutoFix round 2 finishedview run. See this round's report below.

中文说明

AutoFix 第 2 轮已完成 —— 查看运行。本轮报告见下方。

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下:

Autofix review round — no action needed

No code change was made this round. There is no actionable feedback to address:

  • Reviews: the only review is an approval — "LGTM, looks ready to ship — CI landed green after the review. ✅" (APPROVED). It requests no change.
  • Inline comments: no review findings. The inline comments present on the PR are all non-actionable:
    • automated triage / code-review comments that concluded with approval (the reviewer's independent proposal matches exactly what the PR already implements, and triage finished "5/5 — would merge without hesitation");
    • /takeover workflow commands and the takeover-engaged acknowledgment, which are bot infrastructure commands, not code feedback;
    • the author's 补充说明 note, which documents the PR's existing scope and design tradeoffs (API-call-layer retry via classifyRetryError transport classification, stream-layer retry left unchanged, post-first-token stream resets intentionally not retried). It explains the current design and requests no change.
  • Failed / still-red checks: none.

The PR remains at the approved commit e6122b3d6, on which CI already landed green. No further change is warranted.

中文说明

Autofix 审查轮次 — 无需操作

本轮未做任何代码改动。没有需要处理的可操作反馈:

  • Reviews(审查): 唯一的审查是批准 —— "LGTM, looks ready to ship — CI landed green after the review. ✅"(APPROVED),未要求任何改动。
  • Inline comments(行内评论): 没有审查发现。PR 上现有的行内评论均为不可操作项:
    • 自动 triage / 代码审查评论,且已以批准收尾(审查者的独立提议与本 PR 已实现的内容完全一致,triage 结论为 "5/5 — would merge without hesitation");
    • /takeover 工作流命令以及 takeover 已接管的回执,这些是机器人基础设施命令,而非代码反馈;
    • 作者的 补充说明,用于记录本 PR 现有的范围与设计取舍(通过 classifyRetryError 的 transport 分类在 API 调用层重试、stream 层重试保持不变、首 token 后的 stream 中断有意不重试)。它只是说明当前设计,未要求任何改动。
  • 失败 / 持续失败的检查: 无。

PR 仍停留在已批准的提交 e6122b3d6,CI 已在该提交上全绿。无需进一步改动。

Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。


🧠 Handled by Qwen Code · model/模型 qwen3.8-max-preview

@wenshao

wenshao commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Local build & real-network verification (head e6122b3)

Verdict: ✅ merge-ready. The core claim — transient network errors are auto-retried at the API-call level — is verified end-to-end on the production non-streaming path, with a real TCP-RST server, the real OpenAI SDK, and the repo's real retryWithBackoff. Two non-blocking follow-up notes below.

Environment: macOS (darwin 24.6), Node v22.23.1, isolated worktree at e6122b3 (merge-base 6a432ad), fresh npm ci; base/PR dist artifacts verified by marker grep before every A/B arm.

1. Unit tests, lint, typecheck — all green

retry.test.ts 89/89, geminiChat.test.ts 259/259 on the PR head; eslint and tsc --noEmit clean on all three changed files.

2. Source-swap A/B — the new tests are discriminating

Overlaying the merge-base retry.ts + geminiChat.ts while keeping the PR's test file: exactly the two new tests fail (ECONNRESET, ETIMEDOUT); the 87 pre-existing tests pass on both sides — no behavior change for rate-limit / 5xx / fail-fast paths.

unit tests and A/B

3. Live differential — the fix works on the wire

Local HTTP server sends 200 + partial JSON body, then socket.resetAndDestroy() (a real TCP RST) on attempt 1, clean completion on attempt ≥ 2. Real OpenAI SDK (internal retries disabled) wrapped in each dist's retryWithBackoff with production defaults — exactly how client.ts:3220 / baseLlmClient.ts:289,440 wrap every non-streaming LLM call (JSON side-queries, goal judging, compression/summarization):

live differential

4. Built-CLI E2E (real binary, isolated $HOME, local SSE mock)

  • S1 — SSE headers sent, RST before the first event (once): both base and PR builds auto-recover and print the marker — this is the pre-existing stream-transport retry (fix(core): auto-retry transport stream errors before the first chunk #5171); the PR correctly leaves that layer unchanged.
  • S2 — RST before response headers (every attempt): both builds fail identically with [API Error: Connection error. (cause: UND_ERR_SOCKET: other side closed)] after 4 server-observed connection attempts — those are the OpenAI SDK's own internal retries (maxRetries = 3); the repo-level backoff never engages. See note 1.

cli e2e

5. Error-shape probe — real captured errors → the PR's actual classifier

Each row is a real error thrown by the SDK/undici against a live RST server, fed unmodified to the built classifyRetryError:

Failure phase Real shape at the predicate Classified PR retries?
non-stream, RST mid-body (the #7831 string) TypeError "terminated" → cause code=ECONNRESET (depth 1) transport YES ✅
stream, RST after headers, before first event same depth-1 shape transport n/a — pre-existing stream layer handles it
any call, RST before headers (SDK-wrapped) APIConnectionErrorTypeError "fetch failed" → cause code=ECONNRESET (depth 2) unknown NO ❌
raw undici fetch, RST before headers TypeError "fetch failed" → cause code=ECONNRESET (depth 1) transport YES ✅

shape probe

Review notes (non-blocking)

  1. The geminiChat.ts hunk cannot fire for OpenAI-SDK-wrapped connection failures. Pre-header failures reach the predicate as APIConnectionError with the socket code at cause-depth 2, but getTransportCode only probes error.code and error.cause.code — classification stays unknown (verified live: S2 identical on both builds). Today this is shielded by the SDK's 4 quick internal attempts, but a network blip lasting a few seconds exhausts those where the repo's 7-attempt backoff ladder would have ridden it out. Client-side timeouts similarly surface as APIConnectionTimeoutError (no cause at all) and as a plain Error after errorHandler.ts's timeout rewrite (code-reading, not live-probed). Suggested follow-up: walk the cause chain (bounded, e.g. depth ≤ 4) in getTransportCode. The hunk is still correct and reachable for depth-≤1 establishment shapes (raw-fetch/undici providers), so it should stay.
  2. No test pins the geminiChat.ts inline predicate — the two new unit tests exercise defaultShouldRetry only. A predicate-level test would come naturally with the note-1 follow-up.
  3. Landscape: complements open fix(core): retry mid-stream transport failures as continuations #7876 (continuation retry after chunks have been yielded — the remaining uncovered phase); both touch geminiChat.ts, so whichever merges second needs a trivial rebase. MCP-client and weixin-channel retryWithBackoff call sites keep their own predicates — unchanged, consistent with the PR's stated scope. All other LLM call sites use defaultShouldRetry and get the fix automatically.
中文版本(完整验证报告)

本地构建与真实网络验证(head e6122b3

结论:✅ 可合并。 核心主张——瞬态网络错误在 API 调用层自动重试——已在生产非流式路径上端到端验证:真实 TCP RST 服务器 + 真实 OpenAI SDK + 仓库真实的 retryWithBackoff。下附两条不阻塞合并的跟进建议。

环境:macOS (darwin 24.6)、Node v22.23.1、隔离 worktree(e6122b3,merge-base 6a432ad)、全新 npm ci;每次 A/B 前均以标记 grep 验证 base/PR 构建产物。

1. 单测、lint、typecheck 全绿

PR head 上 retry.test.ts 89/89geminiChat.test.ts 259/259;三个改动文件 eslinttsc --noEmit 干净。

2. 源码交换 A/B——新测试具有判别力

用 merge-base 的 retry.ts + geminiChat.ts 覆盖源码、保留 PR 的测试文件:恰好两个新测试失败(ECONNRESETETIMEDOUT);87 个既有测试两侧均通过——限流 / 5xx / fast-fail 路径行为无变化。

3. 真实网络差分——修复在线上生效

本地 HTTP 服务器第 1 次请求发送 200 + 部分 JSON 响应体后 socket.resetAndDestroy()(真实 TCP RST),第 ≥2 次正常返回。真实 OpenAI SDK(禁用其内部重试)配合各构建的 retryWithBackoff 生产默认参数——与 client.ts:3220 / baseLlmClient.ts:289,440 包装所有非流式 LLM 调用(JSON 侧查询、goal 评审、压缩/摘要)的方式完全一致:

4. 编译后 CLI 真机 E2E(隔离 $HOME,本地 SSE mock)

  • S1——SSE 头已发、首事件前 RST(仅 1 次): base 与 PR 构建都自动恢复并输出标记——这是既有的流层 transport 重试(fix(core): auto-retry transport stream errors before the first chunk #5171),PR 正确地未改动该层。
  • S2——每次请求都在响应头前 RST: 两个构建以相同方式失败([API Error: Connection error. (cause: UND_ERR_SOCKET: other side closed)],服务端观测到 4 次连接)——这 4 次是 OpenAI SDK 自身的内部重试(maxRetries = 3),仓库层退避从未启动。见注 1。

5. 错误形状探针——真实捕获错误 → PR 的真实分类器

每行都是 SDK/undici 对真实 RST 服务器抛出的真实错误对象,原样喂给编译后的 classifyRetryError:

失败阶段 到达判定函数的真实形状 分类结果 PR 是否重试
非流式、响应体中途 RST(#7831 的错误串) TypeError "terminated" → cause code=ECONNRESET(深度 1) transport 是 ✅
流式、头已发、首事件前 RST 同上(深度 1) transport 不适用——既有流层处理
任意调用、响应头前 RST(SDK 包装) APIConnectionErrorTypeError "fetch failed" → cause code=ECONNRESET(深度 2) unknown 否 ❌
原生 undici fetch、响应头前 RST TypeError "fetch failed" → cause code=ECONNRESET(深度 1) transport 是 ✅

审阅备注(不阻塞合并)

  1. geminiChat.ts 改动点对 OpenAI-SDK 包装的连接失败无法生效。 响应头之前的失败以 APIConnectionError 形式到达判定函数,socket code 位于 cause 链深度 2,而 getTransportCode 只探测 error.codeerror.cause.code——分类停在 unknown(真机验证:S2 两构建行为一致)。目前该场景由 SDK 的 4 次快速内部重试兜底,但持续数秒的网络抖动会耗尽这些快速重试,而仓库层 7 次指数退避本可扛过去。客户端超时同理:APIConnectionTimeoutError 无 cause,且 errorHandler.ts 的超时改写会替换为不带 code 的普通 Error(此点为代码走读,未做真机探针)。**建议跟进:**在 getTransportCode 中有界遍历 cause 链(如深度 ≤ 4)。该改动点对深度 ≤1 的建立期错误形状(原生 fetch/undici 的 provider)仍然正确且可达,应予保留。
  2. 没有测试钉住 geminiChat.ts 的内联判定改动——两个新单测只覆盖 defaultShouldRetry。随注 1 的跟进自然可补上判定级测试。
  3. **全景:**与开放中的 fix(core): retry mid-stream transport failures as continuations #7876 互补(已产出 chunk 后的续写式重试——剩余未覆盖阶段);两者都改 geminiChat.ts,后合入者需做一次轻量 rebase。MCP client 与 weixin channel 的 retryWithBackoff 调用点保留各自判定——未改动,符合 PR 声明的范围。其余 LLM 调用点均走 defaultShouldRetry,自动获得修复。

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 1/100 轮)。改动内容与我反驳保留之处如下:

Review feedback addressed

Thanks for the exhaustive live verification and the precise error-shape probe. The two actionable notes are implemented; the landscape note needs no code change.

Note 1 — walk the cause chain in getTransportCode (implemented)

getTransportCode previously probed only error.code and error.cause.code (cause-depth ≤ 1), so an OpenAI-SDK-wrapped pre-header failure — APIConnectionErrorTypeError('fetch failed')cause { code: 'ECONNRESET' } (socket code at depth 2) — classified as unknown and was not retried, exactly as your S2 / shape-probe rows showed.

It now walks the cause chain up to a bounded depth (MAX_TRANSPORT_CAUSE_DEPTH = 4), so the depth-2 SDK shape classifies as transport and rides the repo-level 7-attempt backoff ladder. The change is purely additive (it only finds codes that were previously missed) and stays behind the existing guard in classifyRetryError — a definitive 4xx status still wins over a transient socket cause, so a permanent client error is never relabeled retryable. The bound keeps a malformed or circular cause chain from an unbounded traversal. The depth-≤1 establishment shapes the PR already handled are unchanged.

As you noted, client-side timeouts (APIConnectionTimeoutError with no cause, and the plain Error produced by errorHandler.ts's timeout rewrite) carry no code anywhere in the chain, so they are intentionally out of scope here — a cause-chain walk cannot reach them; they would need name/type matching in a separate follow-up.

Note 2 — pin the geminiChat.ts inline predicate (implemented)

Added a predicate-level test that drives the real inline shouldRetryOnError closure through the retryWithBackoff options (same mock pattern already used elsewhere in the file) and feeds it a depth-2 SDK-wrapped ECONNRESET. It asserts the predicate returns true and that sendMessageStream retries and recovers. This pins both the new inline hunk and the cause-chain walk at the geminiChat level.

I verified the test is discriminating with an A/B: against the committed depth-1 classifier it fails exactly at expect(options?.shouldRetryOnError?.(error)).toBe(true); with the fix it passes. Two further focused unit tests cover the same shape at the classifier level (retryErrorClassification.test.ts) and through retryWithBackoff/defaultShouldRetry (retry.test.ts).

Note 3 — landscape / #7876 rebase (no action)

Informational. No code change. Noted that whichever of this PR and #7876 merges second will need a trivial geminiChat.ts rebase.

Conflict

--conflict false: no merge of origin/main performed; the branch stays on its current base.

Verification

  • npm run build — passed
  • npm run typecheck — passed
  • npm run lint — passed
  • npx prettier --check (4 touched files) — passed
  • vitest run src/utils/retryErrorClassification.test.ts — 38 passed (+1 new)
  • vitest run src/utils/retry.test.ts — 90 passed (+1 new)
  • vitest run src/core/geminiChat.test.ts — 260 passed (+1 new)
  • A/B discrimination check (revert classifier to depth-1) — new geminiChat test fails without the fix, passes with it
中文说明

已处理的审阅反馈

感谢你详尽的真机验证和精确的错误形状探针。两条可操作的备注已实现;全景备注无需改动代码。

注 1 — 在 getTransportCode 中遍历 cause 链(已实现)

getTransportCode 此前只探测 error.codeerror.cause.code(cause 深度 ≤ 1),因此 OpenAI SDK 包装的响应头前失败——APIConnectionErrorTypeError('fetch failed')cause { code: 'ECONNRESET' }(socket code 位于深度 2)——会被分类为 unknown 且不被重试,与你的 S2 / 形状探针各行所示完全一致。

现在它会遍历 cause 链直至有界深度(MAX_TRANSPORT_CAUSE_DEPTH = 4),因此深度 2 的 SDK 形状会分类为 transport,从而走仓库层 7 次指数退避阶梯。该改动是纯增量的(只发现此前被漏掉的 code),并仍受 classifyRetryError 中既有判定保护——确定的 4xx 状态仍优先于瞬态 socket cause,因此永久性客户端错误绝不会被改判为可重试。该深度上界防止畸形或环形 cause 链导致无限遍历。PR 此前已处理的深度 ≤1 建立期错误形状行为不变。

如你所指出,客户端超时(无 cause 的 APIConnectionTimeoutError,以及 errorHandler.ts 超时改写产生的普通 Error)在整条链上都不带 code,因此本次有意不在范围内——遍历 cause 链无法触达它们;需要在单独的跟进中按错误名/类型匹配。

注 2 — 钉住 geminiChat.ts 内联判定(已实现)

新增一个判定级测试:通过 retryWithBackoff 的 options 驱动真实的内联 shouldRetryOnError 闭包(与本文件其他地方已有的 mock 模式相同),并向其喂入深度 2 的 SDK 包装 ECONNRESET。它断言判定返回 true,且 sendMessageStream 会重试并恢复。该测试在 geminiChat 层同时钉住了新的内联改动点和 cause 链遍历。

我用 A/B 验证了该测试具有判别力:在已提交的深度 1 分类器上,它恰好在 expect(options?.shouldRetryOnError?.(error)).toBe(true) 处失败;在修复后通过。另有两条聚焦单测分别在分类器层(retryErrorClassification.test.ts)和经由 retryWithBackoff/defaultShouldRetry(retry.test.ts)覆盖同一形状。

注 3 — 全景 / #7876 rebase(无需处理)

信息性备注。无需改动代码。已记录:本 PR 与 #7876 中后合入者需做一次轻量 geminiChat.ts rebase。

冲突

--conflict false:未合并 origin/main;分支保持当前 base。

验证

  • npm run build — 通过
  • npm run typecheck — 通过
  • npm run lint — 通过
  • npx prettier --check(4 个改动文件)— 通过
  • vitest run src/utils/retryErrorClassification.test.ts — 38 通过(+1 新增)
  • vitest run src/utils/retry.test.ts — 90 通过(+1 新增)
  • vitest run src/core/geminiChat.test.ts — 260 通过(+1 新增)
  • A/B 判别力检查(将分类器回退到深度 1)— 新 geminiChat 测试在无修复时失败、有修复时通过

Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。

Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审;第 100 轮后本 bot 停止并将 PR 交给人工。


🧠 Handled by Qwen Code · model/模型 qwen3.8-max-preview

@yiliang114

yiliang114 commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Recovered after restart; restarting task

@qwen-code-ci-bot qwen-code-ci-bot left a comment

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.

⚠️ Downgraded from Approve to Comment: CI still running. Reviewed.

中文说明

⚠️ 已从批准降级为评论:CI still running。 已审查。

— qwen3.7-max via Qwen Code /review

@yiliang114 yiliang114 left a comment

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.

LGTM

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下:

Autofix review round — no action needed

This round's feedback contains no actionable findings, so no code change was made.

  • Automated reviewer (@qwen-code-ci-bot): "⚠️ Downgraded from Approve to Comment: CI still running. Reviewed." This is a status note, not a finding. The review was downgraded from Approve to Comment solely because CI was still in progress at review time — it raises no defect, suggestion, or change request. There is nothing to address in code.
  • Issue-level comment (@yiliang114): "Recovered after restart; restarting task." This is an operational note about the autofix task recovering after a restart, not a request to change the PR.
  • Inline comments: none.
  • Failed checks: none.
  • Still-red checks: none.

The PR stands as-is. No commit was created this round.

中文说明

Autofix 审查轮次 — 无需处理

本轮反馈不包含任何可处理的发现,因此未做任何代码改动。

  • 自动审查者(@qwen-code-ci-bot):⚠️ 已从批准降级为评论:CI still running。已审查。” 这是一条状态说明,而非发现。该审查从批准降级为评论,仅仅是因为审查时 CI 仍在运行 —— 它没有提出任何缺陷、建议或改动请求。代码层面没有任何需要处理的内容。
  • Issue 级评论(@yiliang114): “Recovered after restart; restarting task。” 这是关于 autofix 任务在重启后恢复运行的操作性说明,并非要求改动本 PR。
  • 行内评论: 无。
  • 失败的检查: 无。
  • 持续失败的检查: 无。

本 PR 保持现状。本轮未创建任何提交。

Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。


🧠 Handled by Qwen Code · model/模型 qwen3.8-max-preview

@wenshao
wenshao added this pull request to the merge queue Jul 28, 2026
Merged via the queue into QwenLM:main with commit 0bafd0d Jul 28, 2026
62 checks passed
@chiga0
chiga0 deleted the fix/retry-network-errors branch July 28, 2026 11:20
@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Released in v0.21.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autofix/takeover Summon the autofix loop to manage this PR (remove to release; needs triage+) review/self-reported The linked issue was opened by the PR author (self-reported)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Repeated ECONNRESET on streaming responses when context exceeds ~150k tokens

5 participants