fix(agent-runner): classify transient provider errors as fallback-worthy#95594
Conversation
Extend the embedded error payload classifier to treat server_error and timeout failover reasons as fallback-worthy alongside the existing auth/auth_permanent/billing classifications. Previously, transient provider errors such as upstream_error / HTTP 500 were discarded by the classifier (returning null), which prevented runWithModelFallback from advancing to the next configured fallback model. The result was an immediate turn failure even when healthy fallback models were configured. Changes: - Rename classifyBusinessDenialErrorPayloadReason to classifyProviderErrorPayloadReason - Add server_error and timeout to the pass-through switch cases - Update the corresponding test: HTTP 500 now triggers fallback instead of returning null Fixes openclaw#95519
|
Thanks for the context here. I did a careful shell check against current Close: current main already implements the linked upstream_error fallback fix through #95542, and this stale conflicting branch is no longer the right landing path. Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. So I’m closing this as already implemented rather than keeping a duplicate issue open. Review detailsBest possible solution: Keep the current main implementation from #95542; any broader bare HTTP 500/timeout fallback policy should be proposed separately with focused proof. Do we have a high-confidence way to reproduce the issue? Yes. The linked bug is source-reproducible through the structured upstream_error payload classifier and the runWithModelFallback caller path, and current main now has regression coverage for that path. Is this the best way to solve the issue? No. This PR is no longer the best landing path because current main already has the narrower canonical fix, while this branch is conflicting and also changes bare HTTP 500 behavior that current tests still keep conservative. Security review: Security review cleared: The PR diff only changes internal TypeScript fallback classification and tests, with no dependency, workflow, secret, package, or code-execution surface changes. AGENTS.md: found and applied where relevant. What I checked:
Likely related people:
Codex review notes: model internal, reasoning high; reviewed against 328fb70ba730; fix evidence: release v2026.7.1-beta.1, commit cfd8ba8fc81d. |
clawsweeper
Bot
added
rating: 🧂 unranked krab
…ification
Add classifyFailoverReasonFromErrorType to parse structured provider
error payloads by their errorType field (upstream_error, server_error),
and integrate it into the classifyFailoverSignal pipeline so that
transient upstream failure payloads like
{"error":{"type":"upstream_error",...}} are classified as server_error
instead of falling through unclassified.
Also extend isStructuredServerErrorMessage to match upstream_error
type/code patterns in raw error text.
This complements the downstream classifier change (classifyProviderErrorPayloadReason)
that allows server_error and timeout to trigger model fallback.
Adds test coverage for:
- classifyAssistantFailoverReason with upstream_error payloads
- classifyEmbeddedAgentRunResultForModelFallback with structured
upstream_error payloads
Refs openclaw#95519
|
Updated with upstream error type classification (classifyFailoverReasonFromErrorType) plus extended downstream classifier allowlist. All 191 tests pass across 4 test suites. Please re-review. |
clawsweeper
Bot
added
rating: 🦪 silver shellfish
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
clawsweeper
Bot
added
status: 📣 needs proof
|
@clawsweeper re-review |
…on-provider-upstream-error-llm-r
|
@mmyzwl thanks for the PR. ClawSweeper is still waiting on real behavior proof before this can move forward. Useful proof can be a screenshot, short video, terminal output, copied live output, linked artifact, or redacted logs that show the changed behavior after the fix. Please redact private tokens, phone numbers, private endpoints, customer data, and anything else sensitive. Once proof is added to the PR body or a comment, ClawSweeper or a maintainer can re-check it. |
|
ClawSweeper applied the proposed close for this PR.
|
1 participant
Summary
When a primary provider returns an
upstream_error/HTTP 500/ transient server error, the embedded agent run result classifier discards it (returns null), sorunWithModelFallbacknever advances to the next configured fallback model. The turn ends immediately withLLM request failedeven when healthy fallback models are configured and ready.Fixes #95519
Changes
result-fallback-classifier.ts: RenamedclassifyBusinessDenialErrorPayloadReason→classifyProviderErrorPayloadReasonand extended the switch statement to also pass throughserver_errorandtimeoutfailover reasons (previously onlyauth/auth_permanent/billingwere passed through).result-fallback-classifier.test.ts: Updated the "does not retry non-business transport error payloads" test case to expect a proper fallback classification forHTTP 500: internal server errorinstead of null.Real behavior proof
Behavior addressed: Provider transient errors (
upstream_error,HTTP 500, server timeout) now correctly trigger configured model fallbacks instead of ending the turn immediately.Evidence type: terminal
Real setup tested: Node v24.13.1, Linux x86_64, worktree branch on openclaw/openclaw@main
Exact steps or command run after this patch:
After-fix evidence:
Observed result after the fix: All 103 tests pass across 3 related test suites. The classifier now correctly identifies
server_errorandtimeoutprovider errors as fallback-worthy alongside the existingauth/auth_permanent/billingclassifications.What was not tested: Live end-to-end verification with a real provider returning
upstream_errorwas not performed. The fix is validated at the unit-test layer which directly covers the classification code path.Tests and validation
Risk checklist
Did user-visible behavior change? (
Yes/No)Yes — users with configured fallback models will now see automatic fallback on transient provider errors (
upstream_error,HTTP 500, timeouts) instead of immediate turn failure. This is the intended fix.Did config, environment, or migration behavior change? (
Yes/No)No
Did security, auth, secrets, network, or tool execution behavior change? (
Yes/No)No — only the internal fallback classification path is changed; auth/auth_permanent errors already triggered fallback before this change.
What is the highest-risk area?
How is that risk mitigated?
server_errorandtimeout(two specific transient errors already identified by the existingclassifyFailoverReasonfunction) are added. All other classifications (format, rate_limit, overloaded, model_not_found, etc.) remain filtered out as before. The non-GPT model fallthrough path is unchanged.Current review state
What is the next action?