From f76ec9dabf3da3969c650d5368d354f7fb8cb570 Mon Sep 17 00:00:00 2001 From: vastsa Date: Wed, 16 Sep 2026 01:51:42 +0800 Subject: [PATCH 1/2] fix(agent-runtime): classify the network cause behind NETWORK_ERROR Issue #234: one turn retried ten times with `NETWORK_ERROR: fetch failed` (phase=stream, streamMs=1-2) while a new turn in the same session recovered immediately, and nothing in the logs identified the failing layer because no `error.cause` was kept. `classifyAgentError` already walked the cause chain to detect a network failure, but then returned a bare `result("NETWORK_ERROR", true)` with no cause detail. - Summarize the transport failure in bounded `details`: `networkCategory` (dns | tls | timeout | refused | unreachable | reset | proxy | unknown), `networkCode` (the errno from the cause chain, including undici's happy-eyeballs `AggregateError.errors`), `networkSyscall`, and `networkHost`. - Keep the existing user-visible code. Per-layer codes (DNS_ERROR, TLS_ERROR, SOCKET_RESET, PROXY_ERROR, STREAM_OPEN_FAILED) would each need a spec entry and strings in all eight shipped locales; the category splits the layers inside the already-spec'd, non-user-visible `details` channel instead. - Redaction: only errno-shaped codes, a lowercase syscall, and a bare hostname (never a URL, port, path, query, or credential) can reach the details, and `providerCode` is omitted when it would repeat `networkCode`. - Correlate a failure that produced no response: `requestMessages`, `requestBytes` (measured at the fetch wrapper - byte size only, the body is never read) and `compactionGeneration`. - Surface the errno where the user already is: the retry-reason popover and the assistant error card render `NETWORK_ERROR - ENOTFOUND` in the existing code chip, so no new locale strings are needed. Suggestion 3 of the issue (write the sanitized cause chain to `agent/timing.log`) is obsolete and is deliberately not implemented: ADR 0212 (Accepted 2026-09-10) removed the sidecar `[timing]` lines, the `timing` log category, and `PI_DESKTOP_TIMING`. Failures stay diagnosable through the channels ADR 0212 keeps - the `agent/session.log` error record (its `details` pass through unwhitelisted), the stable error code, and the transcript. No timing record is reintroduced. Verified against the code and deliberately unchanged: every retry already builds a fresh AbortController, stream, and SDK client (`retryPendingProviderFailure` -> `agent.continue()` -> `createProviderRetryStream` -> pi-ai), so suggestion 1 needed no change. A process-global undici dispatcher does exist (`node-proxy.ts`), but rebuilding it after N failures is a behavior change for every session and needs its own ADR; the reported evidence does not prove that retries reused a dead pooled socket, so it is left alone. Specs updated: 03-runtime/08-error-codes.md (details enumeration and the new network diagnosis), 03-runtime/01-ipc-protocol.md, 03-runtime/02-agent-runtime.md and their zh-CN pairs. Refs #234 --- .../chat/transcript/ActivityGroup.tsx | 5 + .../src/features/chat/transcript/shared.tsx | 14 +- docs/spec/03-runtime/01-ipc-protocol.md | 6 +- docs/spec/03-runtime/02-agent-runtime.md | 7 +- docs/spec/03-runtime/08-error-codes.md | 23 +- docs/zh-CN/spec/03-runtime/01-ipc-protocol.md | 7 +- .../zh-CN/spec/03-runtime/02-agent-runtime.md | 5 +- docs/zh-CN/spec/03-runtime/08-error-codes.md | 18 +- .../agent-runtime/src/agent-errors.test.ts | 132 +++++++++- packages/agent-runtime/src/agent-errors.ts | 231 +++++++++++++++++- .../agent-runtime/src/provider-retry.test.ts | 24 ++ packages/agent-runtime/src/provider-retry.ts | 42 +++- packages/agent-runtime/src/runtime.test.ts | 107 ++++++++ packages/agent-runtime/src/runtime.ts | 38 ++- packages/shared/src/types/sessions.ts | 2 + 15 files changed, 631 insertions(+), 30 deletions(-) diff --git a/apps/desktop/src/features/chat/transcript/ActivityGroup.tsx b/apps/desktop/src/features/chat/transcript/ActivityGroup.tsx index 9018094476..b4a20e3114 100644 --- a/apps/desktop/src/features/chat/transcript/ActivityGroup.tsx +++ b/apps/desktop/src/features/chat/transcript/ActivityGroup.tsx @@ -503,6 +503,11 @@ export function RunActivityIndicator({ activity }: { activity: AgentActivity }) {retryErrorSummary} {retryError.code} + {/* The transport errno names the failing layer (ENOTFOUND, a + TLS code, a dropped socket) while the localized summary + cannot; it is a technical token in the same style as the + code beside it, so it needs no translation (issue #234). */} + {retryError.networkCode ? ` · ${retryError.networkCode}` : ""} {retryError.providerStatus !== undefined ? ` · HTTP ${retryError.providerStatus}` : ""} diff --git a/apps/desktop/src/features/chat/transcript/shared.tsx b/apps/desktop/src/features/chat/transcript/shared.tsx index 89dcc66884..4fe311e50b 100644 --- a/apps/desktop/src/features/chat/transcript/shared.tsx +++ b/apps/desktop/src/features/chat/transcript/shared.tsx @@ -149,6 +149,15 @@ export function AssistantErrorMessage({ message }: { message: UiMessage }) { "PROVIDER_SECRET_MISSING", "PROVIDER_UNAUTHORIZED", ].includes(error.code); + // The transport errno is what separates "DNS did not resolve" from "TLS was + // rejected" from "the socket died" for the user; the localized summary can + // only say "can't reach the provider" (issue #234). + const networkCode = (() => { + const details = error.details; + if (!details || typeof details !== "object") return undefined; + const value = (details as { networkCode?: unknown }).networkCode; + return typeof value === "string" ? value : undefined; + })(); return (
@@ -158,7 +167,10 @@ export function AssistantErrorMessage({ message }: { message: UiMessage }) {
{summary} - {error.code} + + {error.code} + {networkCode ? ` · ${networkCode}` : ""} +