fix(core): tolerate repeated invalid model streams - #6712
Conversation
|
Re-triage requested by @wenshao after author addressed review feedback. Template ✓ — all required headings present, bilingual. Problem: Observed, well-documented. DashScope intermittent empty-stream failures are tracked across #6670, #3888, #1700, and #6007. The regression test ( Direction: Aligned. Client-side retry resilience for transient provider-side stream anomalies is core infrastructure — reliable model streaming is the product. The #6670 triage itself suggested increasing the retry budget. Size: 127 production lines (79+48 in Approach: Grew appropriately beyond the initial one-line change. The original PR was Moving on to code review. 🔍 中文说明应 @wenshao 要求,在作者处理 review 反馈后重新分类。 模板 ✓ — 所有必需标题齐全,双语。 问题:已观测、有充分记录。 DashScope 间歇性空流故障在 #6670、#3888、#1700、#6007 中均有追踪。回归测试( 方向:对齐。 瞬态 provider 端流异常的客户端重试弹性属于核心基础设施——可靠的模型流式传输就是产品本身。#6670 的分类本身就建议增加重试预算。 规模:127 行生产代码( 方案:从初始一行改动合理扩展。 原始 PR 是 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Code ReviewIndependent proposal: Given DashScope intermittent empty streams exhausting a 2-retry budget, I'd increase Diff review (re-run against current head Key correctness points verified:
No correctness bugs, no stale references, no AGENTS.md violations. The code is straightforward — no over-abstraction, no drive-by refactors. Reuse check: No new utility functions or abstractions. The retry pattern extends the existing structure in place. TestingUnit tests (worktree, PR head All 225 tests pass, including the 8 new/modified tests covering:
Real-scenario tmux test: not applicable. This addresses intermittent DashScope server-side behavior that cannot be triggered via CLI on demand. The maintainer @wenshao independently verified this PR locally (225/225 pass, typecheck clean, lint clean) and confirmed the new tests genuinely fail on pre-PR code. 中文说明代码审查独立方案: 针对 DashScope 间歇性空流耗尽 2 次重试预算的问题,我会将 Diff 审查(对当前 head 验证的关键正确性点:计数器独立(transient 和 protocol-leak 分别声明、分别递增);退避按计数器线性计算;流校验移到历史推入之前;telemetry 字段内部一致;每次无效流重试前调用 未发现正确性 bug、过时引用或 AGENTS.md 违规。 测试单元测试(worktree,PR head 真实场景 tmux 测试:不适用。 此 PR 解决的是间歇性 DashScope 服务端行为,无法通过 CLI 按需触发。维护者 @wenshao 已独立在本地验证(225/225 通过,typecheck 干净,lint 干净),并确认新测试在改动前代码上确实失败。 — Qwen Code · qwen3.7-max |
|
This PR has matured well across four review iterations. What started as a one-line constant bump evolved into a proper fix that separates retry budgets by error type — the kind of improvement you only see after someone actually reviews the code carefully. The author was responsive to every round of feedback without scope-creeping beyond the retry-budget problem. The core change is sound: DashScope empty streams and protocol-tag leaks are genuinely different failure modes with different recovery profiles. Giving transient errors 4 retries (20s window) while keeping protocol-tag leaks at 2 (6s window) reflects that distinction. The stream-validation-before-history-push fix is a real correctness improvement — failed attempts no longer leave residue that could corrupt Tests are thorough and genuinely pin the behavior: 8 new/modified tests, confirmed failing on pre-PR code (by both the CI triage and @wenshao's independent local verification), all passing on the PR head. The mixed-error-type and abort-during-delay tests cover edge cases that would otherwise be silent regressions. My independent proposal was "increase the constant, maybe separate the budgets." The PR did exactly that plus the history-push ordering fix, which I wouldn't have caught. The implementation is straightforward — no over-abstraction, no speculative features. One minor note: the PR body still says "217 tests" but the current head has 225. Not a concern, just a doc lag. Approving. ✅ 中文说明这个 PR 在四轮 review 迭代中成熟得很好。从最初的一行常量修改进化成了按错误类型分离重试预算的正确修复——这种改进只有在认真审查代码后才会发现。作者对每轮反馈都积极响应,且没有超出重试预算问题的范围。 核心改动合理:DashScope 空流和 protocol-tag leak 是真正不同的故障模式,有不同的恢复特征。瞬态错误给 4 次重试(20 秒窗口),protocol-tag leak 保持 2 次(6 秒窗口),反映了这种区别。流校验前移到历史推入之前是一个真正的正确性改进——失败的尝试不再留下可能污染 测试全面且真正锁定行为:8 个新增/修改测试,在改动前代码上确认失败(CI 分类和 @wenshao 独立本地验证均确认),在 PR head 上全部通过。混合错误类型和延迟期间 abort 测试覆盖了否则会静默回归的边缘情况。 我的独立方案是"增加常量,也许分离预算"。PR 做到了这些加上历史推入顺序修复,后者我不会注意到。实现直白——无过度抽象,无投机性功能。 小提示:PR 正文仍写"217 个测试"但当前 head 已有 225 个。不是问题,只是文档滞后。 批准 ✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No review findings. Downgraded from Approve to Comment: CI failing: Test (ubuntu-latest, Node 22.x).
— qwen3.7-max via Qwen Code /review
|
本轮处理了 continuation retry 路径的 Critical 问题: 另外两条建议(budget exhausted 额外日志、把现有二分支 ternary 改成 switch 以防未来新增类型)属于诊断/泛化改进,不是当前红 CI 或 PR correctness 缺口;按本轮最小修复边界不扩大实现。 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
— qwen3.7-max via Qwen Code /review
|
本轮按 review comments 做了最小 closeout:
验证: |
|
本轮继续处理 14:51 自动 review 反馈:
验证: |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
| invalidStreamRetryCount; | ||
| nextInvalidStreamRetryCount; | ||
| debugLogger.warn( | ||
| `Invalid stream [${(error as InvalidStreamError).type}] ` + |
There was a problem hiding this comment.
[Suggestion] attempt_number in ContentRetryEvent is derived from the per-type counter (nextInvalidStreamRetryCount - 1), so it resets to 0 when switching error types. After 2 transient retries log attempt_number: 0 and attempt_number: 1, a subsequent protocol-tag-leak retry logs attempt_number: 0 again — the counter is non-monotonic within a single send operation.
At 3 AM, an engineer reading structured logs to trace a request lifecycle will see the counter go backwards and assume a bug. Consider renaming to error_type_attempt_number for clarity.
— qwen3.7-max via Qwen Code /review
| const continuationRetryCount = | ||
| error.type === 'PROTOCOL_TAG_LEAK' | ||
| ? protocolTagLeakRetryCount | ||
| : transientRetryCount; |
There was a problem hiding this comment.
[Suggestion] The continuation path has a protocol-tag-leak exhaustion test but no transient exhaustion test (4 retries with NO_FINISH_REASON then throw error). The throw error at this line when maxContinuationRetries === transientMaxRetries (4) is untested. Adding a continuation-path test analogous to the initial-path "should retry on invalid stream responses" test would close this gap.
— qwen3.7-max via Qwen Code /review
✅ Local verification — reproduced & passingI built and ran this PR locally in an isolated worktree at the PR head ( Environment: macOS (darwin 24.6.0) · Node Results
The 8 new/modified tests exercise specifically:
Before / After — the tests really test the changeTo confirm the new tests aren't vacuous, I overlaid Notes for the merge decision
中文说明✅ 本地验证 —— 已复现并通过作为合并参考,我在 PR head( 环境: macOS(darwin 24.6.0)· Node 结果
8 个新增/修改的测试具体覆盖了:
修改前 / 修改后 —— 测试确实在测这个改动为确认新测试不是「空测试」,我把 合并决策备注
|
|
@qwen-code /triage |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅


What this PR does
Increase the independent retry budget for transient invalid model streams from two retries to four while preserving the existing linear backoff and cancellation behavior. Add regression coverage for recovery on the fifth attempt after four invalid streams, as well as correct telemetry when the retry budget is exhausted.
Why it's needed
DashScope and some OpenAI-compatible providers occasionally return semantically incomplete streaming responses, including empty content, missing finish reasons, or empty choices containing only usage metadata. The client already detects these anomalies correctly, but the existing two-retry budget provides only a six-second recovery window and still surfaces
InvalidStreamErrorafter three consecutive invalid responses. This change expands the client-side recovery window without changing stream validation rules or claiming to fix the underlying server-side behavior.Reviewer Test Plan
How to verify
Run the core streaming tests and confirm that four consecutive empty responses with a
STOPfinish reason are retried and followed by a successful fifth response. Verify that conversation history contains only the user message and final model response. Also confirm that persistent invalid streams stop after five total attempts, with four retry telemetry events and one terminal failure event.Evidence (Before & After)
Before: The new regression test failed after the third API attempt with
Model stream ended with empty response text.After: The fifth API attempt succeeds. The four retry telemetry events report backoff delays of 2s, 4s, 6s, and 8s. The complete affected test file passes with 217/217 tests.
Tested on
Environment (optional)
Node.js v22.22.0. Invalid SSE responses were simulated at the protocol boundary using fresh async stream generators.
Risk & Scope
AbortSignal.Linked Issues
Resolves #6670
Related to #3888
中文说明
本 PR 的改动
将瞬态无效模型流的独立重试预算从 2 次提高到 4 次,同时保留现有线性退避和取消行为。补充回归测试,覆盖连续四次无效响应后第五次成功,以及持续失败时正确耗尽预算和上报 telemetry。
修改原因
DashScope 和部分 OpenAI-compatible provider 偶尔会返回语义不完整的流式响应,包括空内容、缺少 finish reason,或只有 usage metadata 的 empty choices。现有客户端可以正确识别这些异常,但两次重试只能提供约 6 秒恢复窗口,连续发生三次异常时仍会直接向用户暴露
InvalidStreamError。本变更扩大客户端恢复窗口,用于缓解这类偶发服务端响应问题;它不改变验证规则,也不宣称修复服务端根因。Reviewer 测试计划
验证方法
运行 core streaming tests,确认连续四次返回空内容且带
STOP的响应后,第五次有效响应能够正常返回,并且历史记录中只保留一次用户消息和最终模型响应。确认持续返回无效流时,总共尝试五次后停止,并记录四次 retry telemetry 和一次 terminal failure telemetry。修改前后证据
修改前:新增回归测试在第三次 API 调用后抛出
Model stream ended with empty response text.修改后:第五次 API 调用成功返回,四次 retry telemetry 分别记录 2s、4s、6s、8s 的退避时间;完整受影响测试文件 217/217 通过。
测试环境
Node.js v22.22.0;在协议边界使用 fresh async stream generators 模拟无效 SSE 响应。
风险与范围
AbortSignal中断。关联 Issue
Resolves #6670
Related to #3888