Repository navigation
refactor(rust): move failure classification and request ownership into core, drop the unreachable bridge routes - #41745
Conversation
…idge routes The Python side binds anthropic_messages, completion and their async twins, names the native module never exported, and the catalog only routes OCR and transcription to Rust. The flat-keyword messages and chat_completions functions in the bridge were therefore reachable only from the wheel smoke test, and if those routes return to Rust they return in the machine-driven shape, not this one. The wheel harness now exercises transcription for the sync, async, concurrency and SIGINT cases, and the route contract tests cover the same validation ordering through transcription's arguments. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…imeout rule MessagesRequest, ChatCompletionsRequest and AudioTranscriptionRequest borrowed their strings, so every async caller had to keep an owned twin alive across the await and destructure it back into borrows. The bridge's RouteOptions and per-route execute glue existed only for that. The request types now own their fields, and the rule that a non-positive or non-finite timeout means no timeout lives in litellm_core::timeout instead of the bridge. The transcription route now builds the core request inline from its parameters, and the websocket connection uses the same timeout rule. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…e typed pair of native exceptions Every route error in core now answers phase(): rejected before any provider I/O (with a named rejection), an upstream status with body and headers, or a failure after dispatch. The match is exhaustive per enum, so adding a variant forces the decision where the variant lives. The umbrella litellm_core::Error existed only so the bridge could re-derive this classification variant by variant; it is gone, along with is_request, is_response and the bridge's copies of the MissingApiKey rule. The bridge maps every route through the same function: file errors keep their Python types, other rejections raise RustBridgeDeclined with (message, rejection), and anything at or after the provider raises RustUpstreamError with (status, body, headers), status 0 when no response arrived. Subclassing a native exception with typed fields needs Python 3.12 under abi3, so the payload travels in args and Python decodes it once, in litellm.rust_bridge.failures. The OCR route host no longer sets status_code, message, headers or a format flag on exceptions and Python no longer reads them back; it builds the mapper's input from the decoded failure instead. Transcription now raises the same declined and upstream exceptions the Python runtime already handled for it, instead of ValueError and RuntimeError that fell through to the caller. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… importing the package The OCR route host looked up litellm.rust_bridge.ocr.route_host and litellm.rust_bridge.timeouts by module path at call time, a back-edge from the extension into package internals that unit tests had to stub through sys.modules. Python now passes an OcrHostHelpers value (response, map_failure, timeout_to_seconds) as the hook's fourth argument, the host keeps that handle for the call and traverses it for GC, and the bridge imports nothing from the package. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
| body, | ||
| headers: &[], | ||
| }, | ||
| Self::Network(_) => Phase::AfterProvider, |
| | Self::MissingRedirectLocation | ||
| | Self::InvalidRedirect |
There was a problem hiding this comment.
Redirect failures misclassified These errors occur before provider dispatch, but this classification maps them as connection failures instead of request failures
Knowledge Base Used: Model request execution
| pub struct AudioTranscriptionRequest { | ||
| pub model: String, | ||
| pub audio: Value, | ||
| pub api_key: Option<&'a str>, | ||
| pub api_base: Option<&'a str>, | ||
| pub custom_llm_provider: Option<&'a str>, | ||
| pub api_key: Option<String>, | ||
| pub api_base: Option<String>, | ||
| pub custom_llm_provider: Option<String>, | ||
| pub extra_headers: Option<Map<String, Value>>, | ||
| pub optional_params: Map<String, Value>, | ||
| pub timeout: Option<Duration>, |
There was a problem hiding this comment.
Public Rust API breaks Owned request fields and the removed Error export break callers, violating the requirement for user-controlled compatibility flags
Rule Used: What: avoid backwards-incompatible changes without user-controlled flags Why: This breaks current behaviour for users using existing functionality Example of BAD: this PR (#22164) introduced run_post_custom... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Devin Review found 1 potential issue.
3 flags not posted on this PR by your GitHub settings — view them in Devin Review. (Configure)
| } | ||
| } |
There was a problem hiding this comment.
🟡 Connection failures block safe fallback
When connection setup fails, several handlers create Network, which reports AfterProvider. Hosts then treat an unsent request as potentially billed and suppress safe fallback.
Learn more
Phase::BeforeProvider means no provider I/O occurred, so another implementation can safely run the request. Reqwest reports failed DNS, TCP, and TLS connection setup through errors where is_connect() is true. The audio handler, messages handler, and OCR transport adapter convert those errors directly to Network. The new phase() implementation therefore loses dispatch certainty even though the Connect variant exists for this case.
Example: A Bedrock hostname fails DNS resolution. The request never leaves the process, but the handler creates Network, and phase() returns AfterProvider instead of BeforeProvider(Unreachable).
Recommended fix: Route pre-dispatch reqwest failures through Error::from_reqwest_before_dispatch in every HTTP handler. Keep response-body read failures and timeouts as Network, because those requests can already have reached the provider.
Was this helpful? React with 👍 or 👎 to provide feedback.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
|
||
| def _mapper_input(error: Exception, failure: failures.Declined | failures.Upstream | None) -> Exception: | ||
| match failure: | ||
| case failures.Upstream(status=0, body=body): |
|
|
||
| def _mapper_input(error: Exception, failure: failures.Declined | failures.Upstream | None) -> Exception: | ||
| match failure: | ||
| case failures.Upstream(status=0, body=body): |
| return request.kwargs | ||
|
|
||
|
|
||
| def _mapper_input(error: Exception, failure: failures.Declined | failures.Upstream | None) -> Exception: |
|
Folded into #41733 as one PR: its four commits are cherry-picked there, so this stacked branch is no longer needed. |
… a legacy Logging adapter Extracted from BerriAI#41733 without the router loop, the cache machine layer, streaming, or the error, timeout and route-pruning work that moved to BerriAI#41745 litellm-callbacks holds the contract a native call and its host share: Machine, HostOp, CallEvent, the in-process run loop, and Passthrough, which is built only by comparing the caller's inputs with the body the route sends, so a route can never mark a key it rewrote. litellm-host-python (formerly python-interop) owns the CPython driver and the Execution handle, and litellm-callbacks-legacy is the @client wrapper as the native call sees it: function_setup, the deployment hooks, pre_call and post_call, the success and failure fan-out and the deferred proxy release. OCR is the one route on it, and the old core and bridge lifecycles are gone The passthrough rule is the structural fix for the bug BerriAI#41719 patched in core and BerriAI#41716 reworks: an inlined remote document no longer counts as the caller's value, so the legacy adapter never hands the caller's URL back into the body. core/tests/ocr/passthrough.rs pins it for every route and document source, including that unchanged values stay passthrough, and callbacks-legacy/tests/payload.rs pins the adapter side with a real pre_call callback Python OCR integration tests that only exercised core behavior now live as Rust tests, so tests/test_litellm_rust keeps the cases that need the full Python stack
TLDR
Problem this solves:
How it solves it:
phase(): rejected, upstream, or after dispatch(message, rejection)and(status, body, headers)in argslitellm.rust_bridge.failuresStacked on #41733; the base is that PR's branch so the diff here is only these four commits
User Flow
Before: a developer whose app transcribes audio through Bedrock on the Rust route gets a bare
RuntimeErrorwhen the provider throttles them, with no status to branch onlitellm.transcription(model="bedrock/mistral.voxtral-mini-3b-2507", file=audio)with the Rust route enabled429with a throttling bodyRuntimeError: upstream request failed with status 429: {...}, nostatus_code, and no litellm exception type to catchAfter: the same throttle comes back as the litellm exception every other route raises
litellm.transcription(model="bedrock/mistral.voxtral-mini-3b-2507", file=audio)with the Rust route enabled429with a throttling bodylitellm.APIErrorwithstatus_code == 429and the provider body in the messageOCR callers see no change: the same public exceptions come back from the same provider answers, which the second proof case shows
Relevant issues
Affected release
Linear ticket
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
uv run pytest tests/test_litellm/<your_test_file>.py -v. Leave the suites (make test-unit-*,make test-unit) to CI: it finishes in ~15 minutes where a laptop takes an hour or more@greptileaito re-request a review after pushing changes)Delays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
Screenshots / Proof of Fix
Shared setup: the native extension is built from the commit under test (
cargo build --release -p litellm-python-bridge --features extension-module, copied tolitellm/rust_bridge/_native.abi3.so).AZURE_AI_API_KEYandAZURE_AI_API_BASEcome from.env. The OCR script is the end user's own code from #41733:No provider returns a throttle on demand, so the transcription case calls the native route directly against the wheel harness's recording server, which answers
429for thex-test-outcome: 429header. It uses the harness fromtests/test_litellm/rust_bridge/native_route_wheel_test.py:Before (e3d46f0)
OCR against Azure AI keeps its public exception
LITELLM_RUST=1 python sync_ocr_with_custom_logger.py azure_ai/mistral-document-ai-2512RuntimeErroris the pre-existing sync-path bug listed under Caveats in refactor(rust): route-neutral callback contract, legacy Logging confined to callbacks-legacy #41733Transcription on a provider 429
python transcription_429.py litellm/rust_bridge/_native.abi3.soRuntimeErrorwith the status inside the message stringAfter (dd7b1ba)
OCR against Azure AI keeps its public exception
LITELLM_RUST=1 python sync_ocr_with_custom_logger.py azure_ai/mistral-document-ai-2512Transcription on a provider 429
python transcription_429.py litellm/rust_bridge/_native.abi3.solitellm.APIError(status_code=429)forlitellm.transcriptioncallersType
🧹 Refactoring
Caveats (if any)
Medium
RustBridgeDeclinedandRustUpstreamErrorinstead ofValueErrorandRuntimeError. The Python runtime already handled both types for this route, solitellm.transcriptioncallers get litellm exceptions where they previously got bare onesunsupportedstill map toAPIConnectionError. Letting them fall back to the Python OCR path is a one-line change inmap_failure, left out on purpose so this PR changes no public OCR outcomeLow
argswith the layout documented on each class and decoded once inlitellm.rust_bridge.failuresRuntimeErroris the pre-existing bug noted thereFinal Attestation
🤖 Generated with Claude Code