fix: don't route invalid_request_error 400s to fallback, skip persist regardless of size - #2175
Closed
XVVH wants to merge 1 commit into
Closed
fix: don't route invalid_request_error 400s to fallback, skip persist regardless of size#2175XVVH wants to merge 1 commit into
XVVH wants to merge 1 commit into
Conversation
When Anthropic returns a 400 invalid_request_error (e.g. orphaned tool_result blocks, unknown fields, malformed message structure), the current code calls _try_activate_fallback() before aborting. This produces two bad outcomes: 1. The fallback provider (e.g. OpenRouter) forwards the same malformed request to Anthropic's backend and returns the same 400, but now attributed to "Provider returned error / provider_name: Azure". The root cause is hidden and the error is misleading. 2. The failed user message is persisted to the session file if the session is below the size thresholds in NousResearch#1630 (50k tokens / 80 messages). On the next gateway restart the session reloads and the first new message immediately triggers the same 400, creating an unrecoverable loop that requires manual session file deletion. Fix: - Detect invalid_request_error by checking error.type in the response body (alongside the existing error.message check for generic 400s). - Skip _try_activate_fallback() for invalid_request_error: these are structural rejections that will fail identically on every provider. - Skip session persistence for any invalid_request_error 400 regardless of session size, extending the existing NousResearch#1630 behaviour to cover mid-sized corrupt sessions (the reported case had ~46 messages / ~29k tokens, below both thresholds). This complements PR NousResearch#2172 (strip orphaned tool_result blocks in convert_messages_to_anthropic) which prevents the bad message from being sent in the first place. This PR provides defence-in-depth: even if a structural 400 occurs for any other reason, it fails fast with a clear error and does not grow the session.
Contributor
|
Thanks for the thorough analysis. We're going a different direction here — instead of skipping fallback for invalid_request_error, we'd rather:
The persist-skip idea has merit but is better handled by preventing broken messages from entering the session in the first place. Appreciate the contribution! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
This PR addresses two related issues discovered while investigating the
invalid_request_error400 failures that have been reported with native Anthropic sessions (#2172 fixes the root cause — orphanedtool_resultblocks). This PR provides defence-in-depth for that class of error and any other structural 400 that may arise.Complements #2172 — both should land together.
Problem 1:
invalid_request_error400s silently route to fallback, masking the root causeWhen Anthropic returns a
400 invalid_request_error(malformed message structure, orphanedtool_resultblocks, unknown fields, etc.), the current code calls_try_activate_fallback()before aborting.This produces two bad outcomes:
The fallback provider (e.g. OpenRouter) forwards the same malformed request to Anthropic's backend and returns the same 400 — but now attributed to
"Provider returned error / provider_name: Azure". The real cause is hidden behind a confusing cross-provider error message.The user (or maintainer) sees an Azure/OpenRouter error and has no indication the problem originated in the request structure, making it very difficult to diagnose.
invalid_request_erroris a structural rejection — it will fail identically on every provider. There is no point routing it to a fallback.Fix: Detect
invalid_request_errorby checkingerror.typein the response body (alongside the existingerror.messagecheck for generic 400s). Skip_try_activate_fallback()for this error type and abort immediately with the original error intact.Problem 2: Mid-sized corrupt sessions persist and reproduce the failure on every restart
The existing
#1630guard skips session persistence when a 400 occurs and the session is large (>50k tokens or >80 messages). This prevents the corrupt session from growing on large conversations.However, the reported session had ~46 messages and ~29k tokens — below both thresholds. Result: the failed user message was persisted. On the next gateway restart, the session reloaded and the first new message immediately triggered the same 400, creating an unrecoverable loop requiring manual session file deletion.
The size thresholds were a reasonable heuristic for context-overflow 400s, but
invalid_request_errorindicates a structural message problem that will reproduce at any session size.Fix: Skip session persistence for any
invalid_request_error400 regardless of session size, in addition to the existing large-session guard. Error message updated to remove the "large" qualifier.Changes
run_agent.py:_err_typefrom the error body alongside the existing_err_message_is_invalid_requestflag for400 + type == "invalid_request_error"_try_activate_fallback()behindnot _is_invalid_request_is_invalid_requestto the session-persistence skip conditionTest plan
invalid_request_error) from primary provider: aborts immediately, does not activate fallback, does not persist session"Error"): existing behaviour preserved — fallback activated if availableRelated
tool_resultblocks inconvert_messages_to_anthropic()(root cause fix)