Skip to content

fix(llm): when every fallback link fails, report the primary's error - #418

Merged
plombeer31 merged 1 commit into
mainfrom
fix/fallback-primary-error
Sep 13, 2026
Merged

fix(llm): when every fallback link fails, report the primary's error#418
plombeer31 merged 1 commit into
mainfrom
fix/fallback-primary-error

Conversation

@plombeer31

Copy link
Copy Markdown
Collaborator

What

Before: runWithFallback discarded each failed link's error and, once the chain was exhausted, threw only the last link's. The common shape — chain [cloud provider, auto-appended local llama-server], cloud answers 404/400, no local model running — reached the user as Turn failed [transport]: fetch failed, the trace error row said fetch failed, and the outage wait logged provider unreachable; parking the turn {"error":"fetch failed"} for five minutes. The cloud's real error was logged nowhere, not even at debug.

After, for the same failure:

Turn failed [transport]: fetch failed (after "openrouter" failed: openai provider 404: No endpoints found for z-ai/glm-5.3-flash.)

A single-link failure reads exactly as before. Every advance is logged once at warn (from, to, status, a reason capped at 180 characters, session id).

Why

The operator cannot act on fetch failed — it names a local server they never started, not the provider refusal that actually ended the turn. During the investigation behind #411 the primary's 404 had to be recovered by rerunning with the local link disabled, because nothing had recorded it.

Classification is untouched on purpose. The thrown error is still the last link's error, unmodified: category, shouldAdvance, the outage wait and its provider_waiting reason, the status line and the transcript record all see exactly what they saw before (pinned by a test written against the old code first). The earlier attempts ride alongside it in a module-private WeakMap (src/llm/fallback/failed-attempts.ts), found by walking the cause chain — the step executor already re-wraps in TransportError(…, {cause}). Rewriting err.message was rejected: isNetworkError matches ^fetch failed$ anchored, so a mutated message turns a network failure into category tool (confirmed by mutation — the classification and integration tests go red).

The sticky override needed the same memory. With the provider wait on (the default), the chain sticks to the local link after the first fallover, so every retry of the parked turn skips the primary and would still end on a bare fetch failed. The chain now remembers the primary failure that caused the switch — refreshed by each failed probe, cleared on recovery — and carries it onto the fallback link's failure. An end-to-end test covers that park-and-give-up path.

Surfaces that show the note: the TUI chat line (formatAgentErrorForChat, appended after the capped body; the HTML-wall check and the dropped-connection hint still judge only the last link's message), Telegram and Discord failure text (Discord's token scrub covers the note), and the trace error row (a new optional fallbackFailures field — message stays verbatim — printed by trace show). Not changed, possible follow-ups: the TUI outage row during the wait, the sidecar session_failed event, and the OpenAI-compatible SSE error.

Merge note: #411 and #412 also edit AGENTS.md §"Provider fallback chain" (the per-session isolation bullet / invariant 9, and the section intro); this PR adds one sentence under invariant 5 — adjacent lines, trivial to rebase. No code overlap with either.

provider-fallback-chain.ts was already over the 300-line limit on main; describeReason moved to describe-reason.ts (which already had a test file by that name) and the breaker/partition state to partition-state.ts, bringing it to 300.

How it was verified

  • npm run lint clean
  • npx vitest run src/llm/fallback src/llm/reliability src/runtime/llm-fallback-seam.test.ts src/tui/format-agent-error-for-chat.test.ts src/tui/agent-event-reducer.test.ts src/channels/telegram/inbound-handler.test.ts src/channels/discord/discord-inbound-handler.test.ts src/tracing src/cli/trace-formatter.test.ts src/agent/agent-loop.test.ts src/error-reporting — 31 files / 627 tests green
  • classification pinned before refactoring (same object rethrown, same message, keys, category, shouldAdvance verdict and network-error verdict) — green on the old code and the new
  • new tests: attempt store, chain wrapper, TUI formatter + reducer, Telegram, Discord, trace recorder and formatter, and an end-to-end run through the real agent loop → step executor → fallback seam → trace recorder, including the park-and-give-up case
  • vacuity (against a 48 / 48 green baseline of the chain, integration and formatter tests): not attaching the failed attempts turns 5 tests red; not recording the override cause turns 3 red (including the park-and-give-up integration case); dropping the note from the chat formatter turns 4 red

runWithFallback dropped each failed link's error on advance and rethrew
only the last one. On the common chain [cloud provider, auto-appended
llama-server], a cloud 404 (OpenRouter: "No endpoints found for ...")
followed by a local server that is not running reached the operator as
`Turn failed [transport]: fetch failed`, the trace error row said the
same, and the cloud's own refusal was logged nowhere, not even at debug.

The last link's error is still the one thrown, unmodified: it decides
classification, fallover and the outage wait, and those match on its
class, cause, status and an anchored `fetch failed`, so rewriting its
message would reclassify a bare TypeError as `tool`. The failed links
are kept in a WeakMap beside it instead, found through the cause chain,
which survives the step executor's TransportError re-wrap:

- the TUI, Telegram and Discord failure lines append
  `(after "<id>" failed: <reason>)`; a single-link failure renders
  byte-for-byte as before
- the trace error row carries them as `fallbackFailures` and keeps
  `message` verbatim; `trace show` prints them
- the chain logs every advance at warn with from, to, status and the
  capped reason the switch notice already shows

A turn already on a sticky override never retries the primary, and every
retry of a parked turn is such a call, so the chain also remembers the
primary failure that put the partition there and runWithFallback carries
it onto a failure of the override link. Without that the turn still
ended on a bare `fetch failed` once the five-minute wait ran out.

describeReason and the breaker/partition state move to their own
modules so provider-fallback-chain.ts stays within 300 lines.
@plombeer31
plombeer31 merged commit cf4e372 into main Sep 13, 2026
2 checks passed
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.

1 participant