From c9f2bad9f6ed2b235d43898b7b35fd8251700af4 Mon Sep 17 00:00:00 2001 From: William Wang Date: Wed, 29 Jul 2026 11:25:24 +0800 Subject: [PATCH] fix: retry transient turn.failed and degrade gracefully instead of stopping --- docs/TROUBLESHOOTING.md | 37 ++++++ src/handlers/session.ts | 221 ++++++++++++++++++++++---------- src/translators/index.ts | 1 + src/translators/tool-helpers.ts | 61 +++++++++ tests/turn-retry.test.ts | 101 +++++++++++++++ 5 files changed, 354 insertions(+), 67 deletions(-) create mode 100644 tests/turn-retry.test.ts diff --git a/docs/TROUBLESHOOTING.md b/docs/TROUBLESHOOTING.md index 9f9ad93..1fe2a9b 100644 --- a/docs/TROUBLESHOOTING.md +++ b/docs/TROUBLESHOOTING.md @@ -165,6 +165,43 @@ a hardcoded version string — read the message text to identify the root cause. - `⚠ elicitation/create failed: ...` → the client does not support it or the request failed - `⚠ request_permission failed: ...` → the fallback path failed +### Turn fails with `model_request_failed` / network error + +**Symptom:** A turn ends with an error like `model_request_failed` / "Network +connection failed for the provider request" / "Turn execution failed". Instead +of stopping the session, the bridge retries transient failures automatically. + +**What happens:** + +When the ZCode backend emits `turn.failed` with a transient cause (provider +network blip, rate limit, brief outage), the bridge retries the turn up to +**5 times** (6 total attempts) with exponential backoff capped at 4s +(1s / 2s / 4s / 4s / 4s). Each retry re-sends the prompt and surfaces a +`[网络异常，正在重试 (n/5)…]` hint so the user knows the turn is being retried +rather than hanging. + +Transient errors are identified by the nested `error.cause.code` +(`model_request_failed`, `provider_not_configured`, `rate_limit`, `timeout`, +`ECONNRESET`, etc.) or by network/connection/timeout keywords in +`error.cause.message`. Non-transient failures (e.g. `prompt is running`) still +surface as hard errors immediately. + +After retries are exhausted, the bridge **degrades gracefully**: it emits a +user-visible `[请求失败：…。会话仍可用，请重新发送消息重试。]` message and returns +`end_turn`, so the session stays usable — resend the message to try again. + +**Debugging:** + +``` +ZCODE_ACP_DEBUG=1 +``` + +Look for `[retry] transient turn failed, re-sending (attempt N/6)` lines to +confirm the retry path is active. If transient failures persist across all +retries, the underlying provider/network issue needs investigation (see +[Authentication / credential errors](#authentication--credential-errors-401-provider-auth-failed) +and check the provider endpoint reachability). + ### `/` completion menu is empty **Symptom:** Typing `/` shows no command completion. diff --git a/src/handlers/session.ts b/src/handlers/session.ts index 0b86ccb..c9ddac8 100644 --- a/src/handlers/session.ts +++ b/src/handlers/session.ts @@ -28,6 +28,7 @@ import { EventTranslator, extractLocations, formatTurnError, + isTransientTurnError, ProjectionDiffer, } from "../translators/index.js"; import type { InternalEvent } from "../translators/index.js"; @@ -332,77 +333,145 @@ export async function prompt( void snapshot; backend.registerEventListener(zcodeSid, listener); - const chunkMsgId = randomUUID(); try { - const sendResp = await backend.request( - server.nextId(), - "session/send", - attachments.length > 0 - ? { sessionId: zcodeSid, content: text, attachments } - : { sessionId: zcodeSid, content: text }, - 15000, - ); - if (sendResp.error) { - // send failed/timeout. Don't fire stop here: a send failure usually - // means the turn never started (no lock to leak). Mirrors Python which - // just returns the error without stopping. - throw new Error(`zcode send failed: ${sendResp.error.message ?? ""}`); - } - const accepted = (sendResp.result ?? {}) as { accepted?: boolean }; - if (!accepted.accepted) throw new Error("zcode send not accepted"); - - // Event-driven turn loop: translate events via EventTranslator + dispatch. - const result = await runEventTurn( - server, - listener, - monitor, - differ, - cx, - params.sessionId, - chunkMsgId, - turn, - ); + // Transient turn failures (e.g. provider network blips surfaced as + // turn.failed with cause code model_request_failed) are retried by + // re-sending the prompt and re-running the event loop, instead of + // surfacing a hard error that stops the session. Non-transient failures + // (send rejected, non-transient turn error) propagate immediately. After + // exhausting retries on a transient error we degrade gracefully: emit a + // user-visible message and return end_turn so the session stays usable. + // 1 initial attempt + 5 retries. Backoff grows exponentially then caps so + // later retries don't keep stretching: 1s, 2s, 4s, 4s, 4s. + const MAX_TURN_ATTEMPTS = 6; + const MAX_BACKOFF_MS = 4000; + const backoffMs = (attempt: number): number => + Math.min(1000 * 2 ** (attempt - 1), MAX_BACKOFF_MS); + let lastTurnError: Record | null = null; + + for (let attempt = 1; attempt <= MAX_TURN_ATTEMPTS; attempt++) { + if (attempt > 1) { + // A prior transient turn ended the backend turn; before re-sending, + // reconcile the differ baseline so the retried turn's new messages + // aren't treated as already-seen, surface a retry hint, then back off. + if (turn.cancelled) { + stopBackendTurn(server, zcodeSid); + return { stopReason: "cancelled" }; + } + differ.markSeen(await fetchMessages(server, zcodeSid)); + await sendTextChunk( + cx, + params.sessionId, + `[网络异常，正在重试 (${attempt - 1}/${MAX_TURN_ATTEMPTS - 1})…]`, + randomUUID(), + ); + log( + ` [retry] transient turn failed, re-sending (attempt ${attempt}/${MAX_TURN_ATTEMPTS})`, + ); + await sleep(backoffMs(attempt - 1)); + } - // Session title: set once on the first end_turn, but ONLY for freshly - // created sessions. Resumed/loaded sessions already carry a title from - // their history and must not be overwritten by the first post-load message. - // sessionTitles enforces set-once within a session; titleEligibleSessions - // gates which sessions are titled at all. - if ( - result.stopReason === "end_turn" && - server.titleEligibleSessions.has(params.sessionId) && - !server.sessionTitles.has(params.sessionId) - ) { - // Title = first non-empty line of the prompt, truncated to 80 chars. - // Multi-line prompts must not leak newlines into the session title. - // Split on any line break (\r\n, \n, \r) so all platforms are covered. - const title = - text - .split(/\r\n|\r|\n/) - .map((l) => l.trim()) - .find((l) => l.length > 0) - ?.slice(0, 80) ?? text.slice(0, 80); - server.sessionTitles.set(params.sessionId, title); - const { updateSessionTitle } = await import("../tasks-index.js"); - void updateSessionTitle(zcodeSid, title, text); - await sendSessionUpdate(cx, params.sessionId, { - sessionUpdate: "session_info_update", - title, - updatedAt: new Date().toISOString(), - }); - } + const chunkMsgId = randomUUID(); + const sendResp = await backend.request( + server.nextId(), + "session/send", + attachments.length > 0 + ? { sessionId: zcodeSid, content: text, attachments } + : { sessionId: zcodeSid, content: text }, + 15000, + ); + if (sendResp.error) { + // send failed/timeout. Don't fire stop here: a send failure usually + // means the turn never started (no lock to leak). Mirrors Python which + // just returns the error without stopping. + throw new Error(`zcode send failed: ${sendResp.error.message ?? ""}`); + } + const accepted = (sendResp.result ?? {}) as { accepted?: boolean }; + if (!accepted.accepted) throw new Error("zcode send not accepted"); + + try { + // Event-driven turn loop: translate events via EventTranslator + dispatch. + const result = await runEventTurn( + server, + listener, + monitor, + differ, + cx, + params.sessionId, + chunkMsgId, + turn, + ); + + // Session title: set once on the first end_turn, but ONLY for freshly + // created sessions. Resumed/loaded sessions already carry a title from + // their history and must not be overwritten by the first post-load + // message. sessionTitles enforces set-once within a session; + // titleEligibleSessions gates which sessions are titled at all. + if ( + result.stopReason === "end_turn" && + server.titleEligibleSessions.has(params.sessionId) && + !server.sessionTitles.has(params.sessionId) + ) { + // Title = first non-empty line of the prompt, truncated to 80 chars. + // Multi-line prompts must not leak newlines into the session title. + // Split on any line break (\r\n, \n, \r) so all platforms are covered. + const title = + text + .split(/\r\n|\r|\n/) + .map((l) => l.trim()) + .find((l) => l.length > 0) + ?.slice(0, 80) ?? text.slice(0, 80); + server.sessionTitles.set(params.sessionId, title); + const { updateSessionTitle } = await import("../tasks-index.js"); + void updateSessionTitle(zcodeSid, title, text); + await sendSessionUpdate(cx, params.sessionId, { + sessionUpdate: "session_info_update", + title, + updatedAt: new Date().toISOString(), + }); + } - // Auto-compact: if context usage exceeds the threshold, compact before - // returning so the next prompt has room. Configured via - // ZCODE_ACP_AUTO_COMPACT_THRESHOLD (absolute token count; 0/unset = disabled). - // Only on end_turn — cancelled/max_turn_requests skips compaction. - // Best-effort: failures are logged inside maybeAutoCompact, never thrown. - if (result.stopReason === "end_turn") { - const { maybeAutoCompact } = await import("../config/auto-compact.js"); - await maybeAutoCompact(server, cx, params.sessionId, zcodeSid); + // Auto-compact: if context usage exceeds the threshold, compact before + // returning so the next prompt has room. Configured via + // ZCODE_ACP_AUTO_COMPACT_THRESHOLD (absolute token count; 0/unset = + // disabled). Only on end_turn — cancelled/max_turn_requests skips + // compaction. Best-effort: failures are logged inside + // maybeAutoCompact, never thrown. + if (result.stopReason === "end_turn") { + const { maybeAutoCompact } = await import("../config/auto-compact.js"); + await maybeAutoCompact(server, cx, params.sessionId, zcodeSid); + } + + return result; + } catch (e) { + // Only a transient TurnFailedError is retryable; everything else (send + // failures, non-transient turn errors, exhausted retries, cancellation) + // propagates to the caller. + if ( + e instanceof TurnFailedError && + attempt < MAX_TURN_ATTEMPTS && + !turn.cancelled && + isTransientTurnError(e.turnError) + ) { + lastTurnError = e.turnError; + continue; + } + throw e; + } } - return result; + // All retries exhausted on a transient error → degrade gracefully. Keep the + // session usable so the user can resend the message instead of the editor + // surfacing a hard error and stopping. Skip auto-compact here: compaction + // after a failed turn is more likely to confuse state than help. + const errMsg = formatTurnError(lastTurnError) || "turn failed after retries"; + await sendTextChunk( + cx, + params.sessionId, + `[请求失败：${errMsg}。会话仍可用，请重新发送消息重试。]`, + randomUUID(), + ); + return { stopReason: "end_turn" }; } finally { backend.unregisterEventListener(zcodeSid, listener); server.pendingTurns.delete(requestId); @@ -453,6 +522,21 @@ export async function cancel( log(`session/cancel → ${zcodeSid}`); } +/** + * Raised by `runEventTurn` when the backend emits `turn.failed`. Carries the + * structured error object (with its nested `cause`) so `prompt`'s retry loop + * can classify transient vs fatal via `isTransientTurnError`. The display + * message is derived from `formatTurnError` at construction time. + */ +class TurnFailedError extends Error { + readonly turnError: Record; + constructor(turnError: Record) { + super(formatTurnError(turnError) || "turn failed"); + this.name = "TurnFailedError"; + this.turnError = turnError; + } +} + /** * Fire-and-forget `session/stop` to the backend. Mirrors Python's * `_cancel_backend_turn`: send stop with an id (some backends route by id @@ -910,7 +994,10 @@ async function runEventTurn( if (translator.turnFailed) { // Best-effort stop in case the failed turn left a residual lock. stopBackendTurn(server, turn.zcodeSid); - throw new RequestError(-32603, formatTurnError(translator.turnError)); + // Throw a TurnFailedError carrying the structured error so the caller + // (prompt's retry loop) can classify transient vs fatal. The error + // message is formatted for display when it ultimately reaches the user. + throw new TurnFailedError(translator.turnError ?? {}); } // Fallback: if no text streamed, surface the last assistant reply. if (!emittedText) { diff --git a/src/translators/index.ts b/src/translators/index.ts index 355b3ee..c5ee9a2 100644 --- a/src/translators/index.ts +++ b/src/translators/index.ts @@ -11,6 +11,7 @@ export { buildDiffContent, extractLocations, formatTurnError, + isTransientTurnError, } from "./tool-helpers.js"; export type { InternalEvent, diff --git a/src/translators/tool-helpers.ts b/src/translators/tool-helpers.ts index 6db0afa..df3683e 100644 --- a/src/translators/tool-helpers.ts +++ b/src/translators/tool-helpers.ts @@ -371,3 +371,64 @@ export function formatTurnError(error: unknown): string { } return out; } + +/** + * `turn.failed` error `code`/`type` values that represent a transient failure + * the bridge can retry (provider network blip, rate limit, brief outage). The + * real cause usually lives under `error.cause` — the top-level code is almost + * always the generic `UNKNOWN_ERROR` wrapper ("Turn execution failed"), so + * matching the top level alone would misclassify every failure as fatal. + */ +const TRANSIENT_CAUSE_CODES = new Set([ + "model_request_failed", + "provider_not_configured", + "rate_limit", + "timeout", + "ECONNRESET", + "ETIMEDOUT", + "ENOTFOUND", + "fetch_failed", +]); + +/** + * Lowercased substrings that, when found in a turn.failed error message, + * indicate a transient network/provider issue. Acts as a fallback when the + * structured `code` is absent or unrecognised but the message clearly points + * at a recoverable condition. + */ +const TRANSIENT_MSG_KEYWORDS = [ + "network", + "connection", + "timeout", + "timed out", + "econnreset", + "enotfound", + "etimedout", + "temporarily unavailable", + "service unavailable", + "retry", +]; + +/** + * Whether a `turn.failed` error object represents a transient failure worth + * retrying. Inspects `error.cause` first (the structured root cause), then + * falls back to the top-level error fields. + */ +export function isTransientTurnError(error: unknown): boolean { + if (!error || typeof error !== "object" || Array.isArray(error)) return false; + const e = error as Record; + const cause = e["cause"]; + // Prefer the nested cause; only fall back to the top-level wrapper when no + // cause object is present. + return matchesTransient(cause) || (cause === undefined && matchesTransient(e)); +} + +function matchesTransient(node: unknown): boolean { + if (!node || typeof node !== "object" || Array.isArray(node)) return false; + const n = node as Record; + const code = String(n["code"] ?? n["type"] ?? "").trim(); + if (code && TRANSIENT_CAUSE_CODES.has(code)) return true; + const message = String(n["message"] ?? n["detail"] ?? "").toLowerCase(); + if (message && TRANSIENT_MSG_KEYWORDS.some((kw) => message.includes(kw))) return true; + return false; +} diff --git a/tests/turn-retry.test.ts b/tests/turn-retry.test.ts new file mode 100644 index 0000000..d6adc5a --- /dev/null +++ b/tests/turn-retry.test.ts @@ -0,0 +1,101 @@ +/** + * Tests for transient turn.failed classification. + * + * The retry loop itself (in `prompt`) is deeply coupled to a live backend and + * is not unit-tested here; instead we cover the predicate that drives it: + * `isTransientTurnError`. The real `turn.failed` payload is a nested object — + * the recoverable cause lives under `error.cause` while the top-level code is + * almost always the generic `UNKNOWN_ERROR` wrapper — so classification must + * look at `cause` and not be fooled by the wrapper. + */ + +import { describe, expect, it } from "vitest"; + +import { isTransientTurnError } from "../src/translators/tool-helpers.js"; + +describe("isTransientTurnError", () => { + it("matches a transient cause.code (model_request_failed)", () => { + // The real-world shape observed in transcripts. + const err = { + code: "UNKNOWN_ERROR", + message: "Turn execution failed", + cause: { + code: "model_request_failed", + message: "Network connection failed for the provider request.", + }, + }; + expect(isTransientTurnError(err)).toBe(true); + }); + + it("matches other whitelisted cause codes", () => { + for (const code of [ + "provider_not_configured", + "rate_limit", + "timeout", + "ECONNRESET", + "ETIMEDOUT", + "ENOTFOUND", + "fetch_failed", + ]) { + expect(isTransientTurnError({ cause: { code } })).toBe(true); + } + }); + + it("matches via message keyword when code is absent/unrecognised", () => { + expect( + isTransientTurnError({ + cause: { message: "Network connection failed for the provider request." }, + }), + ).toBe(true); + expect(isTransientTurnError({ cause: { message: "upstream timed out (110)" } })).toBe(true); + expect(isTransientTurnError({ cause: { message: "service unavailable" } })).toBe(true); + }); + + it("falls back to the top-level error when no cause is present", () => { + // Some failures may not nest a cause; the top-level fields should still be + // inspected as a fallback. + expect(isTransientTurnError({ code: "timeout" })).toBe(true); + expect(isTransientTurnError({ message: "network unreachable" })).toBe(true); + }); + + it("returns false for the UNKNOWN_ERROR wrapper WITHOUT a cause", () => { + // The generic wrapper alone must not be classified as transient — that + // would retry every business error. + expect(isTransientTurnError({ code: "UNKNOWN_ERROR", message: "Turn execution failed" })).toBe( + false, + ); + }); + + it("returns false for non-transient business errors", () => { + expect(isTransientTurnError({ code: "prompt is running" })).toBe(false); + expect(isTransientTurnError({ code: "1308", message: "prompt is running" })).toBe(false); + expect(isTransientTurnError({ cause: { code: "INVALID_PARAMS", message: "bad input" } })).toBe( + false, + ); + }); + + it("returns false for non-object / malformed input", () => { + expect(isTransientTurnError(null)).toBe(false); + expect(isTransientTurnError(undefined)).toBe(false); + expect(isTransientTurnError("model_request_failed")).toBe(false); + expect(isTransientTurnError([])).toBe(false); + expect(isTransientTurnError({})).toBe(false); + }); + + it("inspects cause.type as an alias for cause.code", () => { + // The translator also surfaces `type` on some payloads. + expect(isTransientTurnError({ cause: { type: "rate_limit" } })).toBe(true); + }); + + it("does NOT fall back to top-level when a non-transient cause exists", () => { + // A non-transient cause must short-circuit before the top-level fallback, + // otherwise a transient-looking top-level message would override it. + expect( + isTransientTurnError({ + code: "timeout", // transient-looking top level + message: "network blip", + cause: { code: "INVALID_PARAMS", message: "bad input" }, // fatal cause + }), + ).toBe(false); + }); +});