Tool-path rate limits: retry at the bridge, then fail the run fast - #492
Open
rejojer wants to merge 1 commit into
Open
Tool-path rate limits: retry at the bridge, then fail the run fast#492rejojer wants to merge 1 commit into
rejojer wants to merge 1 commit 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
| def log_message(self, *args): | ||
| pass | ||
|
|
||
| def do_POST(self): |
| assert self.model_calls == 1, "a second model turn ran" | ||
| return FakeTurn() | ||
|
|
||
| def generate_tool_call_response(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 |
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.
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/502/503 and connection failures, three attempts, 0/2/4 s apart or asRetry-Aftersays. Read timeouts are never replayed (240 s each, and the server may have acted). ARetry-Afterpast a minute is a quota, not a blip: the backoff runs instead of sleeping it out. Exhausted, the last response falls through to the existing>= 400branch, sostatus_codesurvives._bridge_invokerre-raises 429/5xx 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 (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.Verification
Retry-Afterhandling against a local HTTP stub, exhausted-retry status, read-timeout non-replay, quota-lengthRetry-After, invoker re-raise matrix, openai-agents escape, run-error unwrap, providerstatus_code, end-to-end fail-fast on the chat and Messages doors.generate_tool_call_response.PageIndexAPIError(status_code=429)onchat()default, streamed, andprotocol="messages", after one tool call.https://claude.ai/code/session_014S88dcSz7jykegAWyWZk8E