Skip to content

fix: surface underlying .cause of OpenAI-compatible connection errors in debug log and API error message - #7010

Merged
wenshao merged 3 commits into
QwenLM:mainfrom
mvanhorn:fix/6996-surface-openai-error-cause
Jul 19, 2026
Merged

fix: surface underlying .cause of OpenAI-compatible connection errors in debug log and API error message#7010
wenshao merged 3 commits into
QwenLM:mainfrom
mvanhorn:fix/6996-surface-openai-error-cause

Conversation

@mvanhorn

Copy link
Copy Markdown
Contributor

What this PR does

Route both discarding call sites through the existing getErrorMessage(error) helper in packages/core/src/utils/errors.ts, which already unwraps error.cause via describeErrorCause() (including undici AggregateError) and formats it as ${error.message} (cause: ${detail}). Change 1: in errorHandler.ts, buildErrorMessage() currently returns error instanceof Error ? error.message : String(error) (~line 115) for the debug-log path; replace the error.message branch with getErrorMessage(error).

Why it's needed

When qwen-code is pointed at a custom OpenAI-compatible provider (via modelProviders.openai[] in settings.json or plain OPENAI_BASE_URL/OPENAI_API_KEY/OPENAI_MODEL env vars), every request fails with a generic [API Error: Connection error. (cause: fetch failed)]. The real underlying cause (e.g. ECONNREFUSED, TLS mismatch, timeout) is discarded before it reaches the user or the debug log, so the failure is undiagnosable even with --debug --openai-logging.

Reviewer Test Plan

How to verify

  • happy path: an Error("Connection error.") whose .cause is Error("fetch failed", { cause: { code: "ECONNREFUSED" } }) is formatted by parseAndFormatApiError into a string that contains the underlying cause (e.g. ECONNREFUSED), not just Connection error..
  • happy path: buildErrorMessage() in errorHandler.ts includes the unwrapped cause in the debug-log message for the same nested-cause error.
  • edge case: a plain Error with no .cause still formats to exactly its message (no trailing (cause: ...)), preserving existing output.
  • edge case: an undici AggregateError (multiple nested causes) surfaces a non-empty cause description rather than an empty string.
  • edge case: existing early-return paths remain byte-identical — quota-exceeded messages (Qwen OAuth quota exceeded: / free-tier discontinued) and already-formatted [API Error: ...] strings are returned verbatim.
  • error path: a non-Error value (e.g. a string thrown) still degrades to String(error) without throwing.

Evidence (Before & After)

N/A - non-UI change (error message plumbing).

Tested on

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

Risk & Scope

  • Main risk or tradeoff: routes error surfacing through the existing getErrorMessage() helper; scope limited to error plumbing.
  • Not validated / out of scope: unrelated provider paths.
  • Breaking changes / migration notes: none.

Linked Issues

Fixes #6996

AI was used for assistance.

@qwen-code-ci-bot

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

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

Template looks good ✓

Problem: Observed bug with thorough reproduction in #6996 — custom OpenAI-compatible providers always fail with a generic Connection error. and the real cause (ECONNREFUSED, TLS mismatch, etc.) is discarded before reaching the user or debug log. This is clearly a real user-facing problem.

Direction: Aligned. Surfacing underlying error causes is basic diagnosability — users can't debug their provider setup without this. No direct CHANGELOG reference, but error reporting quality is core to the developer experience.

Size: 61 production lines changed (3 production files) + 96 test lines (3 test files). Core paths touched (packages/core/src/utils/errors.ts, errorParsing.ts, errorHandler.ts), but the change is small and well-scoped. Not applicable for size thresholds.

Approach: Minimal and correct — routes two call sites through the existing getErrorMessage() helper rather than duplicating cause-unwrapping logic. The only new code is a cause-chain walker (describeCodedCause) with cycle protection and depth limiting, which is the right addition since the old describeErrorCause only looked one level deep. No scope creep, no unrelated changes.

Moving on to code review. 🔍

中文说明

感谢贡献!

模板完整 ✓

问题:已在 #6996 中详细复现的观测 bug —— 自定义 OpenAI 兼容 provider 始终显示通用的 Connection error.,真正的底层原因(ECONNREFUSED、TLS 不匹配等)在到达用户或调试日志之前就被丢弃了。这是一个明确的用户体验问题。

方向:对齐。展示底层错误原因是基本的可诊断性需求——没有这个功能,用户无法调试他们的 provider 配置。CHANGELOG 中没有直接参考,但错误报告质量是开发者体验的核心。

规模:61 行生产代码(3 个生产文件)+ 96 行测试代码(3 个测试文件)。触及核心路径(packages/core/src/utils/errors.tserrorParsing.tserrorHandler.ts),但改动很小且范围明确。不触发规模阈值。

方案:最小且正确——将两个调用点路由到已有的 getErrorMessage() 辅助函数,而不是重复 cause 解包逻辑。唯一的新代码是带循环保护和深度限制的 cause 链遍历器(describeCodedCause),这是正确的补充,因为旧的 describeErrorCause 只看一层深度。无范围蔓延,无无关改动。

进入代码审查 🔍

Qwen Code · qwen3.7-max

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

@qwen-code-ci-bot

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

Copy link
Copy Markdown
Collaborator

Code Review

Independent proposal: Given the problem — error causes discarded before reaching users/debug logs — I would route both error formatting call sites (buildErrorMessage() in errorHandler.ts and parseAndFormatApiError() in errorParsing.ts) through the existing getErrorMessage() helper which already unwraps error.cause. I'd also need to improve describeErrorCause() in errors.ts to walk nested .cause chains (currently only looks one level deep, missing the Error -> TypeError -> Error(code:ECONNREFUSED) chain the OpenAI SDK produces). Would add cycle protection to prevent infinite loops.

Comparison with the diff: The PR's approach matches my proposal exactly. Two surgical call-site changes + a cause-chain walker with depth limiting (8 levels) and cycle detection via a visited Set. The two-tier approach — describeCodedCause (code-preference walker) falls back to describeCauseFallback (immediate cause description) — handles both coded errors (ECONNREFUSED) and non-coded causes cleanly.

No critical issues found. The implementation is careful:

  • describeCodedCause correctly checks nested causes before checking the current node's code, so it finds the deepest meaningful error
  • The visited Set prevents infinite loops on cyclic cause chains (tested explicitly)
  • The depth limit of 8 is reasonable for real-world error chains
  • errorParsing.ts correctly checks error instanceof Error before calling getErrorMessage, falling back to error.message for StructuredError plain objects
  • Existing early-return paths (quota messages, already-formatted strings) remain byte-identical

Reuse check: The fix reuses the existing getErrorMessage() helper rather than creating a parallel implementation. ✓

Tests: 96 tests pass (32 in errors.test.ts, 41 in errorHandler.test.ts, 23 in errorParsing.test.ts). New tests cover: ECONNREFUSED chain, ENOTFOUND chain, AggregateError, cyclic cause, plain error without cause, quota messages unchanged, and both call sites.

Real-Scenario Testing

npm run dev couldn't run in the worktree (unbuilt acp-bridge dist), so I wrote a focused reproduction script that exercises the exact error chain the OpenAI SDK produces when connecting to an unreachable endpoint. The output demonstrates the before/after difference:

=== Simulating OpenAI-compatible provider connecting to unreachable port ===

Error chain: Error("Connection error.") -> TypeError("fetch failed") -> Error("connect ECONNREFUSED 127.0.0.1:1")

BEFORE fix (error.message only):
  Debug log: "Connection error."
  User sees: [API Error: Connection error.]

AFTER fix (getErrorMessage with cause walking):
  Debug log: "Connection error. (cause: connect ECONNREFUSED 127.0.0.1:1)"
  User sees: [API Error: Connection error. (cause: connect ECONNREFUSED 127.0.0.1:1)]

=== AggregateError case (IPv6 + IPv4) ===

BEFORE:
  "Connection error."

AFTER:
  "Connection error. (cause: connect ECONNREFUSED ::1:1; connect ETIMEDOUT 127.0.0.1:1)"

=== Plain error (no cause) — should be unchanged ===
  BEFORE: "Something went wrong"
  AFTER:  "Something went wrong"

The fix delivers exactly what it promises: the underlying cause is now visible to both the user and the debug log, while plain errors without causes remain unchanged.

中文说明

代码审查结果:方案与独立提案完全一致。两个调用点的精确修改 + 带深度限制(8层)和循环检测的 cause 链遍历器。未发现关键问题。

describeCodedCause 的两层策略(先找 coded cause,回退到 fallback)处理了编码错误(ECONNREFUSED)和非编码 cause 两种情况。循环保护(visited Set)和深度限制防止了无限循环。

96 个单元测试全部通过。

真实场景测试:npm run dev 在 worktree 中无法运行(未构建的 acp-bridge),因此编写了精确复现 OpenAI SDK 错误链的脚本。输出清楚展示了修复前后的差异——底层 cause 现在对用户和调试日志都可见,同时无 cause 的普通错误保持不变。

Qwen Code · qwen3.7-max

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

@qwen-code-ci-bot

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

Copy link
Copy Markdown
Collaborator

Confidence: 5/5 — Clean, minimal fix that reuses existing infrastructure, solves a real documented problem, and passes all tests.

This is the kind of PR that's easy to recommend: 61 lines of production change, each doing exactly one thing — routing error messages through a helper that already knew how to unwrap causes. The cause-chain walker is a genuine improvement over the old one-level-deep approach, with proper cycle protection and depth limiting. The before/after reproduction makes the impact obvious: "Connection error." vs "Connection error. (cause: ECONNREFUSED: connect ECONNREFUSED 127.0.0.1:1)".

My independent proposal matched the PR's approach exactly, which gives me confidence the solution is the natural one. The existing early-return paths (quota messages, already-formatted strings) remain byte-identical — no regression risk there. All 96 tests pass, including the new cycle-detection test which shows the author thought about edge cases.

The npm run dev path couldn't run in the worktree (unbuilt acp-bridge dist), but the direct reproduction script exercising the exact error chain the OpenAI SDK produces confirmed the fix works as described. Already approved by @wenshao.

LGTM. ✅

中文说明

置信度:5/5 —— 干净、最小化的修复,复用了已有基础设施,解决了有据可查的真实问题,所有测试通过。

61 行生产代码改动,每一行都在做一件事——将错误消息路由到已经知道如何解包 cause 的辅助函数。cause 链遍历器是对旧的一层深度方法的真正改进,具有适当的循环保护和深度限制。修复前后的复现对比让影响一目了然。

独立提案与 PR 方案完全一致,这进一步证实这是最自然的解决方案。所有 96 个测试通过。已被 @wenshao 批准。

建议合并 ✅

Qwen Code · qwen3.7-max

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

@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. ✅

@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.

Reviewed. 2 Suggestion-level finding(s) could not be anchored to the diff; see the terminal output. Not reviewed: chunk 1 — no agent reported covering these; nobody read them.

— qwen3.7-max via Qwen Code /review

@wenshao

wenshao commented Jul 18, 2026

Copy link
Copy Markdown
Collaborator

Local build & runtime verification (maintainer merge reference) — head 3bc7cf2ab

TL;DR: not merge-ready as-is. The unit tests are green and genuinely load-bearing, but in a real-build A/B the PR does not achieve its stated goal: the user-visible error line is byte-identical before/after the PR, and ECONNREFUSED / ENOTFOUND still never appear anywhere — the real undici/openai error chain nests the syscall code one level deeper than describeErrorCause() unwraps, and the PR's new tests encode that shallower, non-real shape. The branch is also CONFLICTING with main (#6277), and resolving it requires rewriting this PR's new assertions, not just a mechanical merge.

What I ran

macOS, Node v22.23.1. Three isolated worktrees, each with its own real npm ci + build (no shared node_modules): A merge-base 877e8ab77 (the PR's own base, 740 commits behind main), B PR head 3bc7cf2ab, C origin/main d51c21de8. Verified the discriminating code was present/absent in each build artifact before driving it. Then ran the built CLI with an isolated $HOME against two real failure modes:

OPENAI_BASE_URL=http://127.0.0.1:9099/v1  # closed port  → ECONNREFUSED
OPENAI_BASE_URL=http://…invalid/v1        # bogus DNS    → ENOTFOUND
node packages/cli/dist/index.js -p "say OK" --debug   (QWEN_DEBUG_LOG_FILE=1)

1. E2E result: no user-visible change; root cause still hidden

build user-visible stderr debug log (~/.qwen/debug/<session>.txt)
A · merge-base [API Error: Connection error. (cause: fetch failed)] OpenAI API Error: Connection error.
B · PR head byte-identical to A …Connection error. (cause: fetch failed) ← only delta
C · main today byte-identical to A …Connection error. { model, durationMs, errorType: 'APIConnectionError' } (#6277)

On build B, the ECONNREFUSED run and the ENOTFOUND run produce identical output — the user still cannot tell a refused port from a DNS typo, which is exactly the complaint in #6996.

e2e matrix

2. Why: the real chain nests the code at depth 2

Probed empirically with this repo's own openai SDK against a closed port:

APIConnectionError "Connection error."      code: undefined
 └─ cause: TypeError "fetch failed"         code: undefined   ← describeErrorCause() stops here
     └─ cause: Error "connect ECONNREFUSED 127.0.0.1:9099"    code: ECONNREFUSED  ← never reached

getErrorMessage()describeErrorCause() unwraps exactly one level (packages/core/src/utils/errors.ts:54), so it yields Connection error. (cause: fetch failed) — for any connection failure. The PR's new tests attach code: 'ECONNREFUSED' directly to the depth-1 fetch failed error, a shape undici/openai do not produce; that's why the suite is green while the motivating scenario stays broken. A throwaway probe test feeding the PR code the real chain fails expect(…).toContain('ECONNREFUSED') on both changed call sites.

Two secondary observations from tracing the real flow:

  • The one-level (cause: fetch failed) the issue reporter already saw comes from turn.ts (message: getErrorMessage(error) — main :625), which predates this PR. That's also why stderr is unchanged.
  • The parseAndFormatApiError hunk is unreachable in the shipping flows: both the interactive (useGeminiStream.ts) and non-interactive (nonInteractiveCli.ts) paths pass the plain StructuredError object built in turn.ts (or an already-stringified message), never an Error instance with a live cause — the E2E byte-identity confirms this.

real chain probe

3. Unit tests: green and load-bearing (at the PR's own base)

  • PR head: errorHandler.test.ts (37) + errorParsing.test.ts (23) → 60/60 pass.
  • Overlaying merge-base source while keeping the PR's tests → exactly the 3 new cause-surfacing tests fail, 57 regression guards pass. The tests do pin the code change — they just pin it against the wrong error shape.

4. Merge conflict is not mechanical

git merge origin/main conflicts in errorHandler.ts + errorHandler.test.ts, colliding with #6277 which made the debug call 3-arg: debugLogger.error('OpenAI API Error:', msg, diagnostics). The PR's new 2-arg toHaveBeenCalledWith assertions fail after any mechanical resolution, so the new tests must be rewritten during the rebase.

tests and conflict

Suggested path forward

An ~8-line recursive unwrap inside describeErrorCause() (follow cause.cause, prefer the deeper detail) makes the same probe pass — output becomes Connection error. (cause: fetch failed: connect ECONNREFUSED 127.0.0.1:9099) — and, because it flows through the existing getErrorMessage call in turn.ts, it fixes the user-visible line with no other changes. I verified this locally. Concretely I'd suggest:

  1. Rework the fix as recursion in describeErrorCause() (errors.ts), keeping the errorHandler.ts debug-log hunk if desired (the errorParsing.ts hunk is dead code in the shipping flows, harmless to keep or drop).
  2. Replace the synthetic depth-1 test fixtures with the real chain (APIConnectionError → TypeError("fetch failed") → Error(code) and the AggregateError variant), so the regression test actually guards Custom OpenAI-compatible provider always fails with generic 'Connection error' — real cause discarded before logging #6996.
  3. Rebase onto main, resolving against fix(core): improve debug txt diagnostics #6277 and updating the new assertions to the 3-arg call.

Happy to re-verify a new head with the same harness.

Verified locally on macOS (Node v22.23.1); evidence renders of the captured terminal output are hosted on the pr-assets/pr-7010-verify branch.

中文版本(点击展开)

本地构建与运行时验证(维护者合并参考)— head 3bc7cf2ab

结论:不建议按现状合并。 单元测试全绿且确实"承重"(能钉住代码改动),但真实构建的 A/B 对比表明本 PR 没有达成其声明的目标:用户可见的错误输出在 PR 前后逐字节相同,ECONNREFUSED / ENOTFOUND 依然完全没有出现——真实的 undici/openai 错误链把系统调用错误码嵌在比 describeErrorCause() 展开深度更深一层的位置,而 PR 新增测试构造的错误形状(code 挂在第 1 层)并非真实形状。此外分支与 main 处于 CONFLICTING 状态(与 #6277 冲突),且冲突解决需要重写本 PR 的新断言,不是纯机械合并。

验证方法

macOS,Node v22.23.1。三个隔离 worktree,各自独立 npm ci + 构建:A merge-base 877e8ab77(PR 自己的基线,落后 main 740 个提交)、B PR head 3bc7cf2abC origin/main d51c21de8。驱动前先确认判别性代码确实存在/不存在于各构建产物。随后用隔离 $HOME 驱动构建出的 CLI,针对两种真实故障:关闭端口(ECONNREFUSED)与假域名(ENOTFOUND),命令 node packages/cli/dist/index.js -p "say OK" --debugQWEN_DEBUG_LOG_FILE=1)。

1. E2E 结果:用户可见输出零变化,根因依然被隐藏

构建 用户可见 stderr debug 日志
A · merge-base [API Error: Connection error. (cause: fetch failed)] OpenAI API Error: Connection error.
B · PR head 与 A 逐字节相同 …Connection error. (cause: fetch failed) ← 唯一差异
C · main 现状 与 A 逐字节相同 …Connection error. { model, durationMs, errorType: 'APIConnectionError' }#6277

在 B 构建上,ECONNREFUSED 与 ENOTFOUND 两种故障的输出完全相同——用户依然无法区分端口拒绝和 DNS 写错,这正是 #6996 抱怨的问题。

2. 原因:真实错误链的 code 在第 2 层

用本仓库自带的 openai SDK 实测:APIConnectionError "Connection error."(无 code)→ cause: TypeError "fetch failed"(无 code)→ cause: Error "connect ECONNREFUSED …"(code 在此,第 2 层)。而 getErrorMessage()describeErrorCause()errors.ts:54只展开一层,所以任何连接故障都只能得到 (cause: fetch failed)。PR 的新测试把 code 直接挂在第 1 层的 fetch failed 错误上——这不是 undici/openai 的真实产物,因此测试绿灯与真实场景失效并存。用真实链喂给 PR 代码的探针测试在两个改动点上均无法命中 ECONNREFUSED

追踪真实流转的两个附带发现:issue 报告者已能看到的一层 (cause: fetch failed) 来自 turn.tsmessage: getErrorMessage(error),main :625),早于本 PR;而 parseAndFormatApiError 的改动在实际链路中不可达——交互与非交互路径传入的都是 turn.ts 构造的普通 StructuredError 对象(或已字符串化的消息),从不是带 cause 链的 Error 实例,E2E 逐字节一致也证实了这一点。

3. 单元测试:全绿且承重(相对其自身基线)

PR head 上 60/60 通过;把 merge-base 源码覆盖回去、保留 PR 测试后,恰好 3 个新增 cause 测试失败、57 个回归守卫通过——测试确实钉住了代码改动,只是钉在了错误的形状上。

4. 合并冲突并非机械性

#6277 冲突(errorHandler.ts + 其测试):debug 调用已变为 3 参 debugLogger.error('OpenAI API Error:', msg, diagnostics),本 PR 新增的 2 参 toHaveBeenCalledWith 断言在任何机械解决后都会失败,rebase 时必须重写这些测试。

建议路径

describeErrorCause() 内加约 8 行递归展开(沿 cause.cause 下钻、偏好更深的细节)即可让探针通过——输出变为 Connection error. (cause: fetch failed: connect ECONNREFUSED 127.0.0.1:9099)——且由于经过 turn.ts 既有getErrorMessage 调用点,用户可见输出随之修复,无需其它改动(已本地验证)。具体建议:1) 把修复改为 errors.ts 中的递归展开,errorHandler.ts 的 debug 日志改动可保留(errorParsing.ts 的改动在实际链路中是死代码,去留均可);2) 用真实错误链(含 AggregateError 变体)替换合成的第 1 层测试夹具,让回归测试真正守护 #6996;3) rebase 到 main,按 #6277 的 3 参调用更新断言。

欢迎推新 head 后我用同一套装置复验。

mvanhorn added 3 commits July 18, 2026 11:13
…rappers

describeErrorCause() previously unwrapped only one level of .cause, so the
undici/openai chain (APIConnectionError -> TypeError "fetch failed" ->
Error "connect ECONNREFUSED ...") stopped at the code-less "fetch failed"
wrapper and the real syscall code was never surfaced.

Walk the full .cause chain (bounded to 8 levels, cycle-safe via a visited
set, recursing into AggregateError.errors) to the deepest Node code, so
getErrorMessage() now yields e.g. "Connection error. (cause: connect
ECONNREFUSED ...)". Rewrite the tests to encode the real nested shape
instead of attaching the code directly to the depth-1 wrapper.
…s logging

After rebasing onto main, errorHandler now logs a third structured
diagnostics argument (model/durationMs/errorType/...). Update the three
cause/message assertions to match the current three-argument
debugLogger.error signature; the surfaced message strings are unchanged.
@mvanhorn
mvanhorn force-pushed the fix/6996-surface-openai-error-cause branch from 3bc7cf2 to 7089974 Compare July 18, 2026 18:17
@github-actions

Copy link
Copy Markdown
Contributor

Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration.

中文

请勿对活跃的 PR 执行 rebase 或 force-push,因为这会使已有的评审评论失效。另外,供日后参考:作为集成流程的一部分,机器人始终会自动将所有改动压缩(squash)为单个提交。

@mvanhorn

Copy link
Copy Markdown
Contributor Author

Thanks for the incredibly thorough verification - the three-worktree A/B/C with real builds made the gap unambiguous, and you were exactly right about the depth. Pushed the fix, rebased onto main (7089974):

  • describeErrorCause now walks the full .cause chain (bounded to 8 levels, cycle-safe, recursing into AggregateError.errors) to the deepest Node code instead of stopping at the depth-1 fetch failed wrapper. A refused port now surfaces ECONNREFUSED and a bad host ENOTFOUND through getErrorMessage - the reachable path via turn.ts, as you pointed out.
  • Rewrote the tests to the real nested shape (Connection error. -> cause TypeError 'fetch failed' with no code -> cause Error with code: ECONNREFUSED), so they pin the motivating scenario instead of the non-real depth-1 code shape.
  • Resolved the conflict with fix(core): improve debug txt diagnostics #6277: the errorHandler tests now assert against the structured diagnostics logging, and the getErrorMessage surfacing composes with the new getErrorType path.

You were also right that parseAndFormatApiError isn't the lever - the real path is getErrorMessage in turn.ts, which this covers. vitest for errors/errorParsing/errorHandler is green (96 tests) and prettier is clean. There's a small third commit that just realigns the errorHandler assertions after the rebase; happy to squash it into the main fix commit before merge if you'd prefer.

@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.

Reviewed — no blockers. Suggestions are inline. 1 Suggestion-level finding(s) could not be anchored to the diff; see the terminal output.

— qwen3.7-max via Qwen Code /review

Comment on lines +343 to +346
const cause = Object.assign(new Error('fetch failed'), {
code: 'ECONNREFUSED',
});
const connectionError = new Error('Connection error.', { cause });

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.

[Suggestion] This test fixture uses a depth-1 error shape (code: 'ECONNREFUSED' attached directly to the first .cause), but the real undici/openai chain nests the code at depth 2: Error('Connection error.') → TypeError('fetch failed', no code) → Error('connect ECONNREFUSED…', code: 'ECONNREFUSED'). The errors.test.ts tests added by this PR use the real shape, but this one was not updated — Concrete cost: if a future change to getErrorMessage's recursive walk introduces a bug that only manifests at depth ≥ 2 (e.g., off-by-one in the depth counter, incorrect visited set propagation), this test still passes while errors.test.ts catches it, giving a false-green signal in the errorHandler routing path.

Suggested change
const cause = Object.assign(new Error('fetch failed'), {
code: 'ECONNREFUSED',
});
const connectionError = new Error('Connection error.', { cause });
const syscall = Object.assign(
new Error('connect ECONNREFUSED 127.0.0.1:9099'),
{ code: 'ECONNREFUSED' },
);
const fetchFailed = new TypeError('fetch failed', { cause: syscall });
const connectionError = new Error('Connection error.', { cause: fetchFailed });

— qwen3.7-max via Qwen Code /review

@wenshao

wenshao commented Jul 18, 2026

Copy link
Copy Markdown
Collaborator

Maintainer E2E Verification Report

Environment: macOS · Node v22.22.2 · Branch fix/6996-surface-openai-error-cause @ 7089974

1. Unit Tests — 96/96 passed ✅

$ cd packages/core && npx vitest run errors.test.ts errorParsing.test.ts errorHandler.test.ts

 ✓ src/core/openaiContentGenerator/errorHandler.test.ts (41 tests) 16ms
 ✓ src/utils/errors.test.ts (32 tests) 2ms
 ✓ src/utils/errorParsing.test.ts (23 tests) 2ms

 Test Files  3 passed (3)
      Tests  96 passed (96)

2. E2E: Real CLI with unreachable OpenAI endpoint

Before (main):

$ qwen --auth-type openai \
    --openai-base-url http://127.0.0.1:29900/v1 \
    --openai-api-key sk-test --model gpt-4 \
    -p "say hello"

[API Error: Connection error.]

❌ No underlying cause — user cannot diagnose the failure.

After (PR #7010):

$ qwen --auth-type openai \
    --openai-base-url http://127.0.0.1:29900/v1 \
    --openai-api-key sk-test --model gpt-4 \
    -p "say hello"

[API Error: Connection error. (cause: connect ECONNREFUSED 127.0.0.1:29900)]

✅ Underlying syscall code surfaced — immediately diagnosable.

3. Function-level verification (all error chain shapes)

Scenario getErrorMessage() output
ECONNREFUSED chain Connection error. (cause: connect ECONNREFUSED 127.0.0.1:29900)
ENOTFOUND chain Connection error. (cause: getaddrinfo ENOTFOUND my-custom-llm.example.com)
AggregateError (multi-cause) Connection error. (cause: connect ECONNREFUSED ::1:29900; connect ETIMEDOUT 127.0.0.1:29900)
Plain error (no cause) Connection error. ← unchanged, backward compatible
Cyclic cause chain Connection error. (cause: fetch failed) ← no infinite loop

4. Verdict

LGTM — ready to merge

  • Unit tests: 96/96 passed across all 3 changed test files
  • E2E verified: real CLI run with unreachable OpenAI endpoint shows the underlying syscall code
  • Backward compatible: plain errors without .cause remain byte-identical
  • Safety: cyclic cause chains bounded (depth ≤ 8, visited set), AggregateError correctly unwrapped
  • Scope: minimal — 3 production files changed (+60/−5), all in error plumbing
🇨🇳 中文验证报告(点击展开)

维护者 E2E 验证报告

环境: macOS · Node v22.22.2 · 分支 fix/6996-surface-openai-error-cause @ 7089974

1. 单元测试 — 96/96 全部通过 ✅

$ cd packages/core && npx vitest run errors.test.ts errorParsing.test.ts errorHandler.test.ts

 ✓ src/core/openaiContentGenerator/errorHandler.test.ts (41 tests) 16ms
 ✓ src/utils/errors.test.ts (32 tests) 2ms
 ✓ src/utils/errorParsing.test.ts (23 tests) 2ms

 Test Files  3 passed (3)
      Tests  96 passed (96)

2. E2E:真实 CLI 指向不可达的 OpenAI 端点

修复前(main 分支):

$ qwen --auth-type openai \
    --openai-base-url http://127.0.0.1:29900/v1 \
    --openai-api-key sk-test --model gpt-4 \
    -p "say hello"

[API Error: Connection error.]

❌ 没有底层原因——用户无法诊断故障。

修复后(PR #7010):

$ qwen --auth-type openai \
    --openai-base-url http://127.0.0.1:29900/v1 \
    --openai-api-key sk-test --model gpt-4 \
    -p "say hello"

[API Error: Connection error. (cause: connect ECONNREFUSED 127.0.0.1:29900)]

✅ 底层系统调用错误码已浮出——可立即诊断。

3. 函数级验证(覆盖所有错误链形态)

场景 getErrorMessage() 输出
ECONNREFUSED 链 Connection error. (cause: connect ECONNREFUSED 127.0.0.1:29900)
ENOTFOUND 链 Connection error. (cause: getaddrinfo ENOTFOUND my-custom-llm.example.com)
AggregateError(多原因) Connection error. (cause: connect ECONNREFUSED ::1:29900; connect ETIMEDOUT 127.0.0.1:29900)
普通错误(无 cause) Connection error. ← 无变化,向后兼容
循环 cause 链 Connection error. (cause: fetch failed) ← 无死循环

4. 结论

LGTM——可以合并

  • 单元测试:3 个变更测试文件共 96 个测试全部通过
  • E2E 验证:真实 CLI 运行指向不可达 OpenAI 端点,底层系统调用错误码正确浮出
  • 向后兼容:无 .cause 的普通错误输出保持不变
  • 安全性:循环 cause 链有界(深度 ≤ 8,visited 集合),AggregateError 正确展开
  • 范围:最小化——仅 3 个生产文件变更(+60/−5),均在错误处理管道内

@wenshao

wenshao commented Jul 19, 2026

Copy link
Copy Markdown
Collaborator

Maintainer local build & runtime verification (round 2) — head 70899748a

TL;DR: merge-ready. Re-verified from a clean npm ci build after the two follow-up commits. The gap I flagged at 3bc7cf2ab is closed: the new recursive describeCodedCause walks the real .cause chain to the deepest Node syscall code, and it reaches the user through the existing getErrorMessage call in turn.ts. In a real-build A/B the user-visible [API Error] line and the debug log now both carry ECONNREFUSED / ENOTFOUND, and the two failure modes are finally distinguishable — exactly what #6996 asked for. The PR's tests are green (96/96) and load-bearing, and now pin the real nested error shape.

One correction for the record: my earlier round-1 note and the interim summary described the "before" user-visible line as a bare [API Error: Connection error.]. The accurate baseline is [API Error: Connection error. (cause: fetch failed)]fetch failed is what the old one-level unwrap already surfaced on the user-facing path. The bare Connection error. (no cause) was only ever the debug-log line. The measurements below are all from a fresh build.

What I ran

macOS · Node v22.23.1. One isolated worktree, real npm ci. Clean controlled A/B by swapping only packages/core between the PR head 70899748a (AFTER) and its merge-base 67e581aeb (BEFORE, = the PR's parent), rebuilding core each time. The same built qwen binary was driven against two real failure modes with an isolated $HOME and the local proxy disabled so the syscall is reached directly:

OPENAI_BASE_URL=http://127.0.0.1:9099/v1                 # closed port → ECONNREFUSED
OPENAI_BASE_URL=http://does-not-exist-qwen-7010.invalid/v1  # bad DNS   → ENOTFOUND
node packages/cli/dist/index.js --auth-type openai -p "say hello" --debug   (QWEN_DEBUG_LOG_FILE=1)

1. Real-build E2E: the root cause now reaches the user

build user-visible stderr (closed port / bad DNS) debug log (~/.qwen/debug/<session>.txt)
BEFORE 67e581aeb …(cause: fetch failed) · …(cause: fetch failed)identical OpenAI API Error: Connection error. (no cause)
AFTER 70899748a …(cause: connect ECONNREFUSED 127.0.0.1:9099) · …(cause: getaddrinfo ENOTFOUND …invalid) …Connection error. (cause: connect ECONNREFUSED 127.0.0.1:9099) { … errorType: 'APIConnectionError' }

Verbatim, from the built CLI:

BEFORE  [API Error: Connection error. (cause: fetch failed)]           ← closed port
        [API Error: Connection error. (cause: fetch failed)]           ← bad DNS   (indistinguishable)

AFTER   [API Error: Connection error. (cause: connect ECONNREFUSED 127.0.0.1:9099)]
        [API Error: Connection error. (cause: getaddrinfo ENOTFOUND does-not-exist-qwen-7010.invalid)]

Both patched surfaces improve: the user-facing [API Error] line (via turn.ts:625parseAndFormatApiError) and the debug log (via errorHandler.buildErrorMessage, which before had no cause at all).

e2e A/B

2. Why it works now: the real chain nests the code at depth 2

Probed with this repo's own openai SDK (maxRetries: 0) against a closed port and a bogus host:

depth 0: APIConnectionError  code=undefined       "Connection error."
depth 1: TypeError           code=undefined       "fetch failed"                       ← old code stopped here
depth 2: Error               code=ECONNREFUSED    "connect ECONNREFUSED 127.0.0.1:9099"  ← the useful code
         (bad DNS)           code=ENOTFOUND       "getaddrinfo ENOTFOUND …invalid"

The new describeCodedCause recurses through opaque wrappers (bounded depth 8, cycle-guarded, and into AggregateError.errors) to the deepest coded error. Feeding the real chain to the built core, replaying the production getErrorMessage → parseAndFormatApiError path:

BEFORE (built @67e581aeb): (cause: fetch failed)                                  — both cases
AFTER  (built @70899748a): (cause: connect ECONNREFUSED 127.0.0.1:9099) / (cause: getaddrinfo ENOTFOUND …)

root cause + oracle

3. Unit tests: green and load-bearing, now pinned to the real shape

  • PR head: errors.test.ts (32) + errorHandler.test.ts (41) + errorParsing.test.ts (23) → 96/96 pass.
  • Reverting the 3 production files to BEFORE while keeping the PR's tests fails 6 assertions across all 3 files (Test Files 3 failed · Tests 6 failed | 90 passed): the 3 real-chain getErrorMessage tests (ECONNREFUSED / ENOTFOUND / undici AggregateError) plus errorHandler's "underlying cause for non-timeout errors" and the two parseAndFormatApiError cause tests — so every patched surface is genuinely pinned. Crucially, they now encode the real nested shape Connection error. → fetch failed (no code) → Error(code), unlike the non-real depth-1 shape at 3bc7cf2ab.

tests

Notes / non-blocking

  • Conflict resolved. The branch is now MERGEABLE (rebased onto main); the #6277 structured-diagnostics conflict is handled — errorHandler asserts against the structured logging and getErrorMessage composes with the new getErrorType path.
  • Proxied users (HTTP_PROXY set) will see the proxy's own reset code, e.g. (cause: read ECONNRESET), instead of ECONNREFUSED/ENOTFOUND — still strictly better than the opaque fetch failed, since the deepest reachable code is surfaced.
  • The third commit only realigns test assertions after the rebase; squashing into the fix commit before merge (as offered) is fine but not required.

Verdict: ✅ LGTM — ready to merge. The change is minimal (3 production files, +60/−5), reuses the existing getErrorMessage helper, is backward-compatible for causeless errors, and is bounded/cycle-safe.

Verified locally at 70899748a2794297b0bbbc0d29c4d5b9a765509c · real npm ci build · isolated $HOME E2E · controlled core-only A/B.

🇨🇳 中文验证报告(点击展开)

维护者本地构建与运行时验证(第 2 轮)— head 70899748a

结论:可以合并。 在作者补充两个提交后,从干净的 npm ci 构建重新验证。我在 3bc7cf2ab 指出的缺口已修复:新的递归函数 describeCodedCause 会沿着真实的 .cause 链一直走到最深的 Node 系统调用错误码,并通过 turn.ts 中已有的 getErrorMessage 调用到达用户。在真实构建的 A/B 对比中,用户可见的 [API Error] 行与调试日志现在都带上了 ECONNREFUSED / ENOTFOUND,两种失败模式终于可区分——正是 #6996 的诉求。PR 自带测试全绿(96/96)且是"承重"的,并且现在锁定的是真实的嵌套错误形态。

更正一处:我第 1 轮的记录和中间小结把"修复前"的用户可见行描述成了裸的 [API Error: Connection error.]。准确的基线是 [API Error: Connection error. (cause: fetch failed)]——fetch failed 是旧的单层展开在用户侧本就会显示的内容。裸的 Connection error.(无 cause)只出现在调试日志里。下面的数据全部来自全新构建。

运行内容

macOS · Node v22.23.1。一个隔离 worktree,真实 npm ci。干净的受控 A/B:只在 PR head 70899748aAFTER)与其 merge-base 67e581aebBEFORE,即 PR 的父提交)之间切换 packages/core 并各自重建。用同一个已构建的 qwen 二进制、隔离 $HOME、并关闭本地代理以直连触达系统调用,跑两种真实失败模式:关闭端口(ECONNREFUSED)与错误 DNS(ENOTFOUND)。

1. 真实构建 E2E:根因现在能到达用户

构建 用户可见 stderr(关闭端口 / 错误 DNS) 调试日志
BEFORE 67e581aeb …(cause: fetch failed) · …(cause: fetch failed)完全相同 OpenAI API Error: Connection error.(无 cause)
AFTER 70899748a …(cause: connect ECONNREFUSED 127.0.0.1:9099) · …(cause: getaddrinfo ENOTFOUND …invalid) …Connection error. (cause: connect ECONNREFUSED …)

两个被修补的出口都改善了:用户侧 [API Error] 行(经 turn.ts:625parseAndFormatApiError)与调试日志(经 errorHandler.buildErrorMessage,此前完全没有 cause)。

2. 为什么现在生效:真实错误链把 code 嵌在第 2 层

用本仓库自带的 openai SDK(maxRetries: 0)探测:APIConnectionError "Connection error."TypeError "fetch failed"(旧代码在此停下)→ Error code=ECONNREFUSED/ENOTFOUND(真正有用的码)。新的 describeCodedCause 会穿过不透明的包装层(上限深度 8、防循环、并进入 AggregateError.errors)取到最深的带码错误。把真实链喂给已构建的 core、复现生产路径后:BEFORE 两种情况都是 (cause: fetch failed);AFTER 分别是 ECONNREFUSED / ENOTFOUND

3. 单元测试:全绿且承重,现在锁定真实形态

  • PR head:errors.test.ts(32) + errorHandler.test.ts(41) + errorParsing.test.ts(23) → 96/96 通过
  • 把 3 个生产文件回退到 BEFORE、但保留 PR 的测试,跨 3 个文件挂掉 6 个断言Test Files 3 failed · Tests 6 failed | 90 passed):3 个 getErrorMessage 真实链断言(ECONNREFUSED / ENOTFOUND / undici AggregateError),加上 errorHandler 的"非超时错误的底层 cause"和 2 个 parseAndFormatApiError cause 断言——即每个被修补的出口都被锁定;且它们现在编码的是真实嵌套形态 Connection error. → fetch failed(无 code) → Error(code),不同于 3bc7cf2ab 时的非真实单层形态。

备注(非阻断)

  • 冲突已解决。 分支现在为 MERGEABLE(已 rebase 到 main);#6277 结构化诊断冲突已处理。
  • 使用代理的用户(设置了 HTTP_PROXY)会看到代理自身的重置码,如 (cause: read ECONNRESET),而非 ECONNREFUSED/ENOTFOUND——仍然严格优于不透明的 fetch failed
  • 第三个提交只是 rebase 后重新对齐测试断言;按作者提议合并前 squash 进修复提交即可,非必须。

结论:✅ LGTM — 可以合并。 改动最小(3 个生产文件,+60/−5),复用现有 getErrorMessage,对无 cause 的错误向后兼容,且有界、防循环。

@wenshao

wenshao commented Jul 19, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /triage

@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. ✅

@wenshao
wenshao added this pull request to the merge queue Jul 19, 2026
Merged via the queue into QwenLM:main with commit 4bc4178 Jul 19, 2026
60 checks passed
@mvanhorn

Copy link
Copy Markdown
Contributor Author

Thank you @wenshao - surfacing the underlying .cause makes those OpenAI-compatible connection failures actually debuggable.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Custom OpenAI-compatible provider always fails with generic 'Connection error' — real cause discarded before logging

3 participants