fix #95519: [Bug]: Fallback should trigger on provider upstream_error / LLM request failed by mikasa0818 · Pull Request #95542 · openclaw/openclaw

mikasa0818 · 2026-06-21T11:10:44Z

Summary

This PR fixes the provider-error classification gap exposed by issue #95519. When a primary model provider returned a structured transient upstream error, embedded agent execution could stop with LLM request failed instead of advancing through the already configured alternate model candidates.

Behavior or issue addressed: Structured provider responses shaped like {"error":{"message":"Upstream request failed","type":"upstream_error","param":"","code":null}} are now classified as the existing transient server-error reason used by model failover.
Fix classification: Root-cause bug fix in provider error-type normalization/mapping for model routing classification.
Maintainer-ready confidence: High: the change is limited to existing structured provider error classification paths and is covered by focused regression tests plus direct production-classifier proof.

What Problem This Solves

Provider-side transient failures shaped as upstream_error could end an embedded agent turn as LLM request failed even when additional model candidates were configured. In that path, OpenClaw did not classify the structured provider error as failoverable, so the model routing chain could stop after the primary model attempt.

Root Cause

Root cause: The source-of-truth classifier for provider/runtime errors did not normalize structured upstream_error payloads into any FailoverReason. Because that mapping was missing at the classification boundary, provider responses that were semantically transient upstream failures could be treated as non-deliverable terminal errors before the model routing code attempted the next configured candidate.
Why this is root-cause fix: The patch updates the shared classification boundary where provider error types become canonical FailoverReason values, and then allows the embedded result classifier to consume that same server_error reason for provider error payloads. This fixes the source mapping that decides whether model failover is allowed; it does not add a local retry, catch-all text match, or downstream workaround around the final LLM request failed symptom.

Changes

Teach assistant error classification to map structured errorType: "upstream_error" and JSON payloads with error.type: "upstream_error" to the existing server_error failover reason.
Teach embedded result classification to accept structured provider server_error payloads, including upstream_error, as candidate-switching provider failures.
Add focused regressions for both affected paths:
- assistant error message / structured JSON payload classification;
- embedded agent result payload classification.
Patch quality notes: The new default / return null branches are intentional conservative guards: unrecognized provider error types remain non-failoverable. The new tests and comments describe the existing model failover behavior being protected; they do not broaden arbitrary provider text handling.

Real behavior proof

Real environment tested: Local OpenClaw repository task worktree on Node v22.17.0 / pnpm 11.2.2, using the repository test runner and production TypeScript modules. No secret-bearing external provider call was required for the deterministic classifier proof.
Exact steps or command run after this patch: node scripts/run-vitest.mjs src/agents/embedded-agent-helpers.formatassistanterrortext.test.ts src/agents/embedded-agent-runner/result-fallback-classifier.test.ts was run after formatting. A direct production-classifier proof was also run with node --import tsx --input-type=module against the issue payload shape.
Evidence after fix: Focused Vitest exited 0; the direct production-classifier proof returned the expected server_error reason for both the assistant error path and the embedded payload path; git diff --check exited 0.
Observed result after fix: The direct classifier proof printed upstream_error assistant reason: server_error and embedded payload fallback reason: server_error, showing both production classification paths now feed the existing failover reason expected by the model routing chain.
What was not tested: No live external provider outage was triggered, so the PR does not claim an end-to-end hosted provider reproduction. The regression uses the structured payload shape from the issue and exercises the production classifiers that decide whether candidate switching is allowed.

Regression Test Plan

Target test file: src/agents/embedded-agent-helpers.formatassistanterrortext.test.ts and src/agents/embedded-agent-runner/result-fallback-classifier.test.ts.
Scenario locked in: A provider returns Upstream request failed with type: "upstream_error"; OpenClaw should classify it as server_error for model failover instead of treating it as a non-failoverable terminal error.
Why this is the smallest reliable guardrail: These focused tests cover the two classifier boundaries that can block model candidate switching before the next model is attempted, without running the full embedded-agent runner harness for a pure provider-error mapping regression.

User Impact

Why it matters / User impact: Users who configured alternate model candidates get the next model when the primary provider reports a transient upstream failure, rather than an immediate failed turn with LLM request failed. If every candidate fails, the existing summary behavior still reports the attempted models.

Merge risk

Risk labels considered: auth-provider, availability, compatibility, session-state.
Risk explanation: The touched code sits on provider/model routing and agent session failure handling, so over-broad classification could incorrectly skip terminal errors. This patch is constrained to structured provider server_error/upstream_error types and keeps auth, billing, hook blocks, replay-invalid turns, delivered replies, and unrelated terminal paths on their existing behavior.
Why acceptable: upstream_error is a transient provider-side failure equivalent to the already failover-safe server-error class. Mapping it to server_error uses the existing model routing mechanism and preserves conservative null behavior for unknown error types, so compatibility risk is lower than adding a new reason, schema field, or generic text-based rule.

Scope, contract, and compatibility

What did NOT change: No config, schema, protocol, public API, model list, candidate ordering, retry policy, hook behavior, or delivered-message handling changed. The patch only changes structured provider error classification for existing model failover logic; unclassified arbitrary provider text is still not made failoverable. Out of scope: provider-specific live outage simulation, new provider support, and any change to public configuration defaults.
Architecture / source-of-truth check: The source-of-truth boundary remains FailoverReason classification in the agent/provider error helpers and embedded result classifier. upstream_error is normalized to the existing server_error reason rather than introducing a new public contract surface, config schema, protocol field, migration, or model-consumed payload shape.
Related open PR scan: Related open PRs for this issue were checked before implementation; linked/competing PRs were treated as background only, and this patch remains scoped to the issue’s provider-error classification root cause.

clawsweeper · 2026-06-21T11:12:07Z

Codex review: needs maintainer review before merge. Reviewed June 29, 2026, 7:02 AM ET / 11:02 UTC.

Summary
The PR maps structured provider upstream_error and overloaded_error payloads into existing failover reasons and lets embedded provider error payloads with those reasons trigger model fallback, with focused regression tests.

PR surface: Source +40, Tests +72. Total +112 across 4 files.

Reproducibility: yes. at source level: current main routes embedded provider error payloads through a narrow allowlist and treats null classifications as completed primary attempts, so the reported upstream_error payload does not advance fallback. I did not run a live external provider outage reproduction in this read-only review.

Review metrics: 1 noteworthy metric.

Provider payload fallback reasons: 2 added: server_error and overloaded. The PR intentionally broadens which structured provider error payloads can advance the configured model fallback chain.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #95519
Summary: This PR is a candidate fix for the canonical embedded provider upstream_error fallback bug; several sibling PRs attempt narrower or broader classifier variants.

Members:

canonical: [Bug]: Fallback should trigger on provider upstream_error / LLM request failed #95519 - The PR uses closing syntax for this issue, and the issue body describes the same embedded provider upstream_error fallback gap.
partial_overlap: fix(agent-runner): classify transient provider errors as fallback-worthy #95594 - This open sibling targets the same result-classifier gap with broader transient provider-error scope, but live state is conflicting and proof-gated.
partial_overlap: fix(failover): classify upstream_error as server_error to trigger model fallback #96110 - This open sibling maps upstream_error in the shared helper but does not clearly cover the embedded payload allowlist boundary that this PR changes.
partial_overlap: fix(fallback): treat upstream_error as fallbackable #96245 - This open sibling targets the same fallbackable upstream error behavior but remains proof-gated in live labels.
partial_overlap: fix(agents): classify upstream_error as fallbackable to harden model fallback #96372 - This open sibling targets upstream fallback hardening but remains proof-gated in live labels.
1 more in the report.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🦞 diamond lobster
Proof: 🦞 diamond lobster
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Risk before merge

[P2] Merging intentionally changes provider/model routing so structured upstream/server/overloaded provider payloads can advance to a fallback model instead of surfacing the primary provider error.
[P1] Several open candidate PRs target the same canonical bug, so maintainers should pick one landing path and avoid stacking overlapping classifier changes.
[P2] Broader timeout, HTTP 503, or BAD_UPSTREAM embedded payload coverage remains a maintainer scope choice and is not proven by this narrower PR.

Maintainer options:

Land This As Canonical (recommended)
After exact-head required checks pass, maintainers can accept this PR as the canonical narrow fix because it covers both the structured error mapping and the embedded payload allowlist.
Fold Broader Transient Coverage First
If maintainers want HTTP 503, BAD_UPSTREAM, or timeout embedded payloads included, add that scope explicitly with focused tests before merge.
Defer To A Proven Sibling
If another open PR becomes the chosen path, pause or close this branch only after the replacement is proven, mergeable, and covers both classifier boundaries.

Next step before merge

[P2] Maintainers need to choose the canonical fix among overlapping PRs and accept the provider-routing merge risk; there is no narrow automated repair needed on this branch.

Security
Cleared: The diff changes internal TypeScript agent fallback classification and focused tests only; it does not alter secrets, dependencies, workflows, package scripts, or supply-chain surfaces.

Review details

Best possible solution:

Use this PR or one explicitly chosen sibling as the canonical fix; if landing this PR, keep the scope to structured upstream/server/overloaded payloads and close or supersede overlapping PRs after merge.

Do we have a high-confidence way to reproduce the issue?

Yes at source level: current main routes embedded provider error payloads through a narrow allowlist and treats null classifications as completed primary attempts, so the reported upstream_error payload does not advance fallback. I did not run a live external provider outage reproduction in this read-only review.

Is this the best way to solve the issue?

Yes. The PR fixes the shared structured error-type mapping and the embedded payload allowlist rather than adding a config option, provider-specific workaround, or broad generic text match; canonical PR selection is the remaining maintainer decision.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 389c355bcf3f.

Label changes

Label justifications:

P2: This is a normal-priority agents runtime fallback reliability fix for one concrete provider transient-error shape with limited blast radius.
merge-risk: 🚨 auth-provider: The diff changes provider/model fallback routing for structured provider error payloads, affecting model choice after provider failures.
rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes after-fix terminal proof from focused Vitest files and a direct production-classifier command showing the reported payload returns server_error for both changed paths.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix terminal proof from focused Vitest files and a direct production-classifier command showing the reported payload returns server_error for both changed paths.

Evidence reviewed

PR surface:

Source +40, Tests +72. Total +112 across 4 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	2	46	6	+40
Tests	2	72	0	+72
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	4	118	6	+112

What I checked:

Current main embedded payload allowlist: Current main only allows auth, auth_permanent, billing, and rate_limit provider error payload reasons through the embedded result classifier, so structured server/upstream provider payloads still return null at this boundary. (src/agents/embedded-agent-runner/result-fallback-classifier.ts:149, 389c355bcf3f)
Current main fallback loop behavior: When result classification is null, runFallbackAttempt treats the candidate as successful, so the fallback loop does not advance to the next configured model. (src/agents/model-fallback.ts:403, 389c355bcf3f)
Agent runtime entry point: The agent command path wires classifyEmbeddedAgentRunResultForModelFallback into runWithModelFallback and records fallback steps from that loop. (src/agents/agent-command.ts:1844, 389c355bcf3f)
PR head shared classifier change: The PR head adds structured errorType classification for upstream_error to server_error and overloaded_error to overloaded, then uses it after message/detail classification when no more specific message classification exists. (src/agents/embedded-agent-helpers/errors.ts:925, f5a70ef7bb10)
PR head structured JSON parsing: The PR head also recognizes parsed JSON error.type values of server_error and upstream_error as structured server errors, covering the reported payload shape. (src/agents/embedded-agent-helpers/errors.ts:1682, f5a70ef7bb10)
PR head embedded payload allowlist: The PR head expands the embedded provider error payload helper to accept server_error and overloaded in addition to existing auth, billing, and rate-limit reasons. (src/agents/embedded-agent-runner/result-fallback-classifier.ts:154, f5a70ef7bb10)

Likely related people:

steipete: Remote commit history shows recent work replacing terminal classifiers and documenting embedded runner/model fallback behavior for this boundary. (role: fallback feature owner by history; confidence: high; commits: 0314819f918a, 6719528316c0, 8f14a1c59aaf; files: src/agents/embedded-agent-runner/result-fallback-classifier.ts, src/agents/model-fallback.ts, docs/concepts/model-failover.md)
jason-allen-oneal: Remote history shows recent Codex usage-limit fallback coverage and visible-output guards in the same result fallback classifier. (role: recent adjacent classifier contributor; confidence: high; commits: b8f1961aaeba; files: src/agents/embedded-agent-runner/result-fallback-classifier.ts)
mikasa0818: Beyond this PR, remote current-main history shows recent merged work by this author on generic CLI failure fallback in the same result classifier file. (role: recent adjacent classifier contributor; confidence: medium; commits: aa0bdb901f4a; files: src/agents/embedded-agent-runner/result-fallback-classifier.ts)
takhoffman: The earlier embedded provider business-denial fallback classifier change records approval/co-authorship for the same classifier boundary. (role: adjacent reviewer and automerge requester; confidence: medium; commits: 18f94fc83a72; files: src/agents/embedded-agent-runner/result-fallback-classifier.ts)
altaywtf: Live PR timeline shows this assignee force-pushed the latest refactor commit clarifying provider fallback classification on this branch. (role: current PR follow-up owner; confidence: medium; commits: f5a70ef7bb10; files: src/agents/embedded-agent-helpers/errors.ts)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

NianJiuZst · 2026-06-21T11:33:50Z

Complementary observations on top of ClawSweeper's review:

This is the most thorough fix in the #95519 cluster. The classifyFailoverReasonFromErrorType function plus parseApiErrorInfo-based JSON parsing is the right shape — it classifies by structured error type instead of substring matching the raw text. That matters because provider payloads vary (single quotes, escaped JSON, code-attribute form, etc.) and substring matches are fragile.

A few small things worth tightening before merge:

genericErrorTypeClassification precedence. The new effectiveMessageClassification = … ?? genericErrorTypeClassification means an explicit errorType only kicks in when the message+detail classification returned null. That is the right precedence — structured fields win, message fallback last — but it deserves a comment near the merge so a future contributor doesn't re-order it. Suggest adding a // structured errorType takes precedence over free-text message line.
'overloaded_error' → 'overloaded'. The switch maps overloaded_error to the overloaded reason. Worth confirming that reason name matches the gateway's failover decision tree (e.g., whether overloaded is treated as retryable vs transient-fallback). If the gateway treats it differently from server_error, the current mapping is correct; if not, this is a separate bug fix and should be a separate PR.
Cluster overlap. This PR overlaps with fix(agents): classify 'upstream request failed' as server_error for failover #95521, fix(agents): classify provider upstream_error as fallbackable transient failure #95523, fix(agents): classify upstream_error errorType as server_error for model fallback (fixes #95519) (AI-assisted) #95524, fix(agents): expand embedded fallback classifier to accept transient provider errors #95537, all targeting the same [Bug]: Fallback should trigger on provider upstream_error / LLM request failed #95519 fix. Recommend maintainers pick this PR as the canonical landing and close the others as superseded — the JSON-parser approach subsumes the substring-based ones.

wangwllu · 2026-06-23T15:51:33Z

Additional live repro from a 2026.6.8 gateway setup, matching the failure mode this PR targets.

Config shape:

{
  "agents": {
    "defaults": {
      "model": {
        "primary": "<private-anthropic-compatible-primary>",
        "fallbacks": ["<private-openai-compatible-fallback>"]
      }
    }
  }
}

Observed user-visible result:

LLM request failed.

Session/error evidence for the failed primary attempt:

api: anthropic-messages
stopReason: error
status/code: 503 BAD_UPSTREAM
message: 模型服务异常
cause: Bedrock is unable to process your request.

A status check immediately after confirmed the fallback model was configured for the default primary, but the turn ended on the primary failure instead of advancing to the configured fallback. This was not a channel delivery failure; the same session could reply normally after being pinned to the fallback model.

This looks like the same classifier gap as #95519, with the upstream provider failure surfaced as a terminal LLM request failed rather than a fallback-worthy transient server/upstream error.

Local version where observed:

OpenClaw 2026.6.8 (844f405)

harjothkhara · 2026-06-24T03:47:01Z

Comparing the open #95519 fixes, this is the cleanest / most surgical of them, and it correctly handles the reported upstream_error → server_error → fallback path. 👍

One small thing I noticed while reviewing the cluster: the matcher now maps overloaded_error → "overloaded", but the embedded classifier gate (classifyBusinessDenialErrorPayloadReason in src/agents/embedded-agent-runner/result-fallback-classifier.ts) only returns for auth | auth_permanent | billing | server_error. So an overloaded reason falls through to default → null and won't actually trigger embedded fallback — the new overloaded_error mapping is currently dead for the embedded path. If overloaded is in scope, adding a case "overloaded": there closes the loop.

Minor: the gate's return type was widened to FailoverReason | null while the switch still only accepts those four reasons — keeping it a narrow Extract<FailoverReason, ...> of the accepted set would preserve the compile-time guarantee.

(Context: I'd opened #96260 for the same issue but am closing it in favor of this one — sharing the finding here rather than piling on.)

altaywtf · 2026-06-29T11:12:14Z

Merged via squash.

Prepared head SHA: f5a70ef7bb1072b5dcad935681484aa2de5dd195
Landed commit: cfd8ba8fc81defb9ad2af8e5c0981d65c3a5ab03

…am_error / LLM request failed (openclaw#95542) * fix(agents): fallback on upstream provider errors * refactor(agents): clarify provider fallback classification --------- Co-authored-by: Altay <[email protected]> (cherry picked from commit cfd8ba8)

…am_error / LLM request failed (openclaw#95542) * fix(agents): fallback on upstream provider errors * refactor(agents): clarify provider fallback classification --------- Co-authored-by: Altay <[email protected]>

…am_error / LLM request failed (openclaw#95542) * fix(agents): fallback on upstream provider errors * refactor(agents): clarify provider fallback classification --------- Co-authored-by: Altay <[email protected]> (cherry picked from commit cfd8ba8)

…am_error / LLM request failed (openclaw#95542) * fix(agents): fallback on upstream provider errors * refactor(agents): clarify provider fallback classification --------- Co-authored-by: Altay <[email protected]>

…am_error / LLM request failed (openclaw#95542) * fix(agents): fallback on upstream provider errors * refactor(agents): clarify provider fallback classification --------- Co-authored-by: Altay <[email protected]> (cherry picked from commit cfd8ba8)

openclaw-barnacle Bot added agents Agent runtime and tooling size: S labels Jun 21, 2026

clawsweeper Bot mentioned this pull request Jun 21, 2026

fix(agents): classify upstream_error errorType as server_error for model fallback (fixes #95519) (AI-assisted) #95524

Closed

1 task

clawsweeper Bot mentioned this pull request Jun 21, 2026

fix(agent-runner): classify transient provider errors as fallback-worthy #95594

Closed

clawsweeper Bot added rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels Jun 21, 2026

clawsweeper Bot mentioned this pull request Jun 22, 2026

fix(cron): treat generic 'LLM request failed' error as transient server_error #94011

Closed

This was referenced Jun 23, 2026

fix: classify upstream provider errors as server_error to enable fallback #96041

Closed

fix(failover): classify upstream_error as server_error to trigger model fallback #96110

Closed

fix: run fallback models on upstream provider errors #96196

Closed

harjothkhara mentioned this pull request Jun 24, 2026

fix(agents): fallback on embedded upstream errors #96260

Closed

altaywtf self-assigned this Jun 29, 2026

mikasa0818 and others added 2 commits June 29, 2026 13:26

fix(agents): fallback on upstream provider errors

e67b78a

refactor(agents): clarify provider fallback classification

f5a70ef

altaywtf force-pushed the feat/issue-95519 branch from bd0d30c to f5a70ef Compare June 29, 2026 10:54

altaywtf merged commit cfd8ba8 into openclaw:main Jun 29, 2026
94 checks passed

github-actions Bot mentioned this pull request Jun 29, 2026

📡 Upstream Digest — 2026-06-29 13:58 UTC curtismercier/openclaw-mods#1173

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix #95519: [Bug]: Fallback should trigger on provider upstream_error / LLM request failed#95542

fix #95519: [Bug]: Fallback should trigger on provider upstream_error / LLM request failed#95542
altaywtf merged 2 commits into
openclaw:mainfrom
mikasa0818:feat/issue-95519

mikasa0818 commented Jun 21, 2026

Uh oh!

clawsweeper Bot commented Jun 21, 2026 •

edited

Loading

Uh oh!

NianJiuZst commented Jun 21, 2026

Uh oh!

wangwllu commented Jun 23, 2026

Uh oh!

harjothkhara commented Jun 24, 2026

Uh oh!

Uh oh!

altaywtf commented Jun 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Uh oh!

Conversation

mikasa0818 commented Jun 21, 2026

Summary

What Problem This Solves

Root Cause

Changes

Real behavior proof

Regression Test Plan

User Impact

Merge risk

Scope, contract, and compatibility

Uh oh!

clawsweeper Bot commented Jun 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

NianJiuZst commented Jun 21, 2026

Uh oh!

wangwllu commented Jun 23, 2026

Uh oh!

harjothkhara commented Jun 24, 2026

Uh oh!

Uh oh!

altaywtf commented Jun 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

clawsweeper Bot commented Jun 21, 2026 •

edited

Loading