Tool-path rate limits: retry at the bridge, then fail the run fast - #492
Open
rejojer wants to merge 3 commits into
Open
Tool-path rate limits: retry at the bridge, then fail the run fast#492rejojer wants to merge 3 commits into
rejojer wants to merge 3 commits into
Conversation
A PageIndex cloud 429 (or 5xx) on a tool call used to reach the model as an INTERNAL_ERROR envelope saying "try again": the model re-called once with no wait, then wrote the failure into its answer, and chat() returned normally with no status anywhere. The same 429 before the loop (the doc_id targeting lookup) already propagated raw. - McpBridge mounts a urllib3 Retry: 429/502/503 and connection failures, three attempts, 0/2/4 s apart or as Retry-After says; read timeouts are never replayed (240 s each, and the server may have acted); a Retry-After past a minute is a quota, not a blip, so the backoff runs instead of sleeping it out. Exhausted, the last response falls through to the existing >= 400 branch, so the status_code survives. - _bridge_invoker re-raises 429/5xx alongside 401/403. The frameworks turn a raised tool exception back into model-visible text, so each chat() door gets its own escape: the in-process MCPServer's failure_error_function lets a PageIndex-caused failure propagate and _translate_run_error unwraps it from the framework's wrapper (which also un-flattens the 401 case); the Messages lane runs each turn's tools through the runner's public generate_tool_call_response() and raises before the next model call. - _model_backend_error keeps the provider's status_code. Claude Agent SDK tools cannot fail fast: the SDK MCP server converts handler exceptions into JSON-RPC errors for Claude Code by design. Claude-Session: https://claude.ai/code/session_014S88dcSz7jykegAWyWZk8E
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| def log_message(self, *args): | ||
| pass | ||
|
|
||
| def do_POST(self): |
| def test_bridge_read_timeout_is_not_retried(mcp_stub, monkeypatch): | ||
| """A read timeout is a full wait the server may have acted on: | ||
| surfaced once, never replayed.""" | ||
| import pageindex.mcp_bridge as mcp_bridge |
| escapes the run (a model-side slip staying model-visible is covered end | ||
| to end in test_local_chat).""" | ||
| pytest.importorskip("agents") | ||
| import pageindex.mcp_bridge as mcp_bridge |
The bridge retry is now a plain urllib3 Retry: 429 and every 5xx retried three times at the fixed 0/2/4 s backoff, Retry-After ignored. That drops the _Retry subclass, whose get_retry_after raised InvalidHeader on a non-integer header (turning a 429 into "could not reach the server"), honoured a 60 s Retry-After three times over, and let a 413 carrying Retry-After replay. 500 and 504 join the forcelist so the invoker's "what survived the bridge's retries" holds for every status it re-raises. Retry is imported from requests.adapters, the declared dependency. The invoker re-raises transport failures too: once the bridge's own connection retries fail, the model cannot reach the server either, and the envelope only sent it round the retry loop. The handshake error blames the API key only on 401/403: a rate-limited handshake is now a run-terminating error and was telling users to rotate a working key. Docstrings on agent_tools()/build_agent_tools and the Anthropic adapter state the real raise set: 401/403, post-retry 429/5xx, unreachable server. Claude-Session: https://claude.ai/code/session_013xk3xt9KgHNTjsYmFLKxbu
|
|
||
| def test_handshake_failure_blames_the_key_only_on_auth_statuses(monkeypatch): | ||
| """A rate-limited or failing handshake is not a key problem.""" | ||
| import types |
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
1 participant
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.
A PageIndex cloud 429 (or 5xx) on a tool call used to reach the model as an
INTERNAL_ERRORenvelope saying "try again": the model re-called once with no wait, then wrote the failure into its answer, andchat()returned normally with no status anywhere. The same 429 before the loop (the doc_id targeting lookup) already propagated raw.Changes
McpBridgemounts a urllib3Retry: 429, 500/502/503/504 and connection failures, three attempts, 0/2/4 s apart.Retry-Afteris ignored: a long one is a quota, not a blip, and parsing it was the one way a 429 could turn into "could not reach the server" (urllib3 raises on a non-integer value). Read timeouts are never replayed (240 s each, and the server may have acted). Exhausted, the last response falls through to the existing>= 400branch, sostatus_codesurvives._bridge_invokerre-raises 429/5xx and an unreachable server (a transport failure that outlived the connection retries) alongside 401/403. The frameworks turn a raised tool exception back into model-visible text, so eachchat()door gets its own escape:MCPServerpasses afailure_error_functionthat lets a PageIndex-caused failure propagate, and_translate_run_errorunwraps it from the framework's wrapper (this also un-flattens the mid-session 401 case).generate_tool_call_response(), and a recorded failure raises before the next model call._model_backend_errorkeeps the provider'sstatus_code.Claude Agent SDK tools cannot fail fast: the SDK MCP server converts handler exceptions into JSON-RPC errors for Claude Code by design.
Behaviour change on the public tool surfaces
as_openai_tools()users running their ownRunner.runnow get an exception for a post-retry 429/5xx or an unreachable server (the 401/403 re-raise always intended this; the framework absorbed it).as_anthropic_tools()users' runners absorb it into anis_errorresult and log a traceback. The plain functions fromagent_tools()raisePageIndexAPIErrorfor the same set, where their docstring used to promise only 401/403; the docstrings now state the real raise set.Verification
Retry-Afterheader present and ignored), exhausted-retry status for 429 and 504, read-timeout non-replay, an unreachable server re-raised through the invoker, the handshake wording per status; plus the invoker re-raise matrix, openai-agents escape, run-error unwrap, providerstatus_code, end-to-end fail-fast on the chat and Messages doors, and a regression guard for model-side slips.generate_tool_call_response.PageIndexAPIError(status_code=429)onchat()default, streamed, andprotocol="messages", after one tool call.https://claude.ai/code/session_014S88dcSz7jykegAWyWZk8E
https://claude.ai/code/session_013xk3xt9KgHNTjsYmFLKxbu