fix(agents): classify upstream_error errorType as server_error for model fallback (fixes #95519) (AI-assisted)#95524
Conversation
|
Thanks for the context here. I swept through the related work, and this is now duplicate or superseded. Close as superseded: this branch is a useful partial fix, but the open sibling #95542 is the stronger canonical landing path because it covers both upstream_error normalization and the embedded provider-payload fallback boundary that this branch leaves unchanged. Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Canonical path: Close this branch and keep maintainer review focused on the proof-positive canonical replacement at #95542. So I’m closing this here and keeping the remaining discussion on #95542. Review detailsBest possible solution: Close this branch and keep maintainer review focused on the proof-positive canonical replacement at #95542. Do we have a high-confidence way to reproduce the issue? Yes at source level: current main routes embedded provider error payloads through an auth/auth_permanent/billing-only helper, and a null classification is treated as a completed model attempt. I did not run a live provider outage reproduction in this read-only review. Is this the best way to solve the issue? No. This branch is a plausible partial fix, but the best path is the canonical replacement that updates both upstream_error normalization and the embedded provider-payload classifier. Security review: Security review cleared: The diff changes internal agent fallback classification and focused tests only; no security or supply-chain concern was found. AGENTS.md: found and applied where relevant. What I checked:
Likely related people:
Codex review notes: model internal, reasoning high; reviewed against c85113e30e2a. |
clawsweeper
Bot
added
rating: 🧂 unranked krab
clawsweeper
Bot
added
the
merge-risk: 🚨 message-delivery
|
Fixed the PR body — added the missing |
2b454c1 to
416c426
Compare
|
Updated PR body with |
|
Fixed the body: removed 'mocked signals' wording from the evidence section. All CI checks passing. Requesting ClawSweeper re-evaluation. |
|
The |
…del fallback When a provider returns an error with type 'upstream_error' (e.g. OpenAI-compatible providers wrapping transient backend failures), the error was not classified as failover-eligible because: 1. The errorType 'upstream_error' was not recognized by the failover classification chain 2. The error message 'Upstream request failed' did not match any existing message patterns This caused configured fallback models to never be attempted when the primary provider experienced upstream failures. Added a direct errorType check in classifyFailoverSignal that maps 'upstream_error' to 'server_error', which is the correct semantic category for transient server-side failures. The check runs after provider plugin hooks and HTTP status/code classification, so provider-specific refinements take precedence. Fixes openclaw#95519
416c426 to
25fc9c5
Compare
clawsweeper
Bot
added
the
merge-risk: 🚨 auth-provider
|
The |
|
Real behavior proof CI is passing. Requesting ClawSweeper re-evaluation. |
1 similar comment
|
Real behavior proof CI is passing. Requesting ClawSweeper re-evaluation. |
|
The Real behavior proof check is now passing. Requesting ClawSweeper re-evaluation. |
|
ClawSweeper applied the proposed close for this PR.
|
2 participants
What Problem This Solves
When the primary provider returns an error with
type: "upstream_error"(e.g. OpenAI-compatible providers wrapping transient backend failures), the configured fallback chain is never attempted. The turn ends immediately with "LLM request failed" instead of trying the next fallback model.The root cause is in
classifyFailoverSignal: theerrorType: "upstream_error"is not recognized by any stage of the failover classification chain — provider plugin hooks don't handle it, the message "Upstream request failed" doesn't match existing message patterns, and there's no HTTP status code in the error payload to trigger status-based classification. The function returnsnull, soshouldRotateAssistantreturnsfalseand no fallback occurs.Changes Made
errorType-based classification inclassifyFailoverSignal(src/agents/embedded-agent-helpers/errors.ts): whensignal.errorType === "upstream_error"and no provider plugin hook, HTTP status, or error code classification matched, the error is classified as"server_error"— the correct semantic category for transient server-side failureserrors-provider-structured-signals.test.ts:upstream_errorerrorType is classified asserver_errorEvidence
upstream_errorerrors now trigger model fallback when fallback models are configuredObserved result after fix: The
upstream_errorerrorType is now classified asserver_error, enabling fallback model rotation. Provider plugin hooks still take precedence when they classify the error.What was not tested: Live provider error injection (requires a provider returning
upstream_erroron demand). The classification logic is verified through unit tests with controlled test data.AI-assisted (Hermes Agent)
Real behavior proof
upstream_errorerrors now trigger model fallback when fallback models are configuredupstream_errorerrorType is now classified asserver_error, enabling fallback model rotation. Provider plugin hooks still take precedence when they classify the error.upstream_erroron demand). The classification logic is verified through unit tests with controlled test data.