diff --git a/litellm-rust/.agents/skills/rust-error/SKILL.md b/litellm-rust/.agents/skills/rust-error/SKILL.md index a44bf3577b4..dd8add582ba 100644 --- a/litellm-rust/.agents/skills/rust-error/SKILL.md +++ b/litellm-rust/.agents/skills/rust-error/SKILL.md @@ -8,20 +8,21 @@ description: Define, wrap, convert, classify, report or test Rust error types in `crates/http/src/error.rs` is the reference implementation. Read the reference for the part you are touching before editing, each one carries the reasoning and worked examples behind its rules - [shape](references/shape.md) - - One top-level `Error` per crate in `src/error.rs`. A public enum by default, an opaque struct over a private `Repr` enum when the variants must stay private + - One top-level `Error` per crate in `src/error.rs`. A public enum by default, an opaque struct over a private `Repr` enum when the variants must stay private. No `#[non_exhaustive]` - A variant is a failure mode a caller handles differently. Provider names and guidance are data, never variants - [messages and causes](references/messages-and-causes.md) - - Every message is written once, in `#[error(...)]`. Call sites pass typed facts, never a string - - Store the cause itself, never its string, and render it once - - Strip `reqwest::Error` URLs where they enter our types + - Every message is written once, in `#[error(...)]`, and describes its own layer only. Call sites pass typed facts, never a string + - The cause is the `source()`, never part of the message. Keep it typed, never stringify or drop it + - `reqwest::Error` enters our types only through `litellm_http::Error`, which strips its URL + - Anything shown to someone else is rendered with `litellm_error::Report`, never `to_string()` - [conversions](references/conversions.md) - One `From` per source and target pair, only when the conversion needs no context. It classifies and sanitizes once - Nest a lower crate's error in one variant. Translate when the meaning changes, never mirror its variants - [classification](references/classification.md) - - Callers ask predicates named after their decision, they do not match nested variants + - Callers ask predicates named after their decision, they do not match nested variants. Predicates match exhaustively - Classification states facts. Retry and fallback policy lives with the caller that knows idempotency - [boundaries](references/boundaries.md) - - The gateway and the Python bridge are the only places an error becomes text for someone else, and both ask the same predicates in the same order + - An error becomes text for someone else only in one mapping function per error type, in the gateway crates and the Python bridge. The inference gateway and the bridge ask the same predicates in the same order - [gateway](references/gateway.md) - One `Error` with `IntoResponse` per gateway crate. Every failure flows through the same `From` chain, is classified once in `status()` and rendered by surface and framing - [pyo3](references/pyo3.md) @@ -31,4 +32,4 @@ description: Define, wrap, convert, classify, report or test Rust error types in - [recipes](references/recipes.md) - Common tasks walked through end to end -The workspace uses thiserror 2. The [derive docs](https://docs.rs/thiserror/latest/thiserror/) are the authority on macro behavior +The workspace uses thiserror 2 for every error type, and no library crate uses `anyhow` or `Box` in a public signature. The [derive docs](https://docs.rs/thiserror/latest/thiserror/) are the authority on macro behavior, and the [`std::error::Error` docs](https://doc.rust-lang.org/std/error/trait.Error.html) on what `Display` and `source()` mean diff --git a/litellm-rust/.agents/skills/rust-error/references/boundaries.md b/litellm-rust/.agents/skills/rust-error/references/boundaries.md index 38be6046761..660a10f1c25 100644 --- a/litellm-rust/.agents/skills/rust-error/references/boundaries.md +++ b/litellm-rust/.agents/skills/rust-error/references/boundaries.md @@ -1,13 +1,18 @@ # Boundaries -An error becomes text for someone else in exactly two places, and both are explicit mappings. Nothing else formats an error for a client +A boundary is where an error becomes something another program reads: an HTTP response, a Python exception, a stored or logged message. Each boundary has one mapping function per error type it receives, and nothing outside those functions formats an error for someone else -| Boundary | File | Reference | +| Boundary | Mapping | Reference | |---|---|---| -| Gateway HTTP response | `crates/gateway-inference/src/error.rs` | [gateway](gateway.md) | -| Python bridge | `crates/python-bridge/src/errors.rs` and `crates/python-bridge/src/routes/ocr/errors.rs` | [pyo3](pyo3.md) | +| Inference HTTP response | `Error::status()` and its renderers in `crates/gateway-inference/src/error.rs` | [gateway](gateway.md) | +| Other gateway surfaces | the `IntoResponse` of each `crates/gateway-*` crate's `Error` | [gateway](gateway.md) | +| Python bridge, inference and OCR routes | `route_error_to_pyerr` in `crates/python-bridge/src/errors.rs` and `to_pyerr` in `crates/python-bridge/src/routes/ocr/errors.rs` | [pyo3](pyo3.md) | +| Python bridge, everything else | one `From for PyErr` or `*_to_pyerr` per crate error, all in `crates/python-bridge` | [pyo3](pyo3.md) | +| Python host runtime | `From for PyErr` in `crates/host-python/src/error.rs` | [pyo3](pyo3.md) | -Both ask a nested error the same questions in the same order +## Asking the nested error + +The inference gateway and the Python bridge's route mapping both receive a nested route error, and both ask it the same questions in the same order ```rust if let Some((status, body)) = error.upstream_response() { the provider's status and body pass through } @@ -22,18 +27,12 @@ The two boundaries are not identical. The gateway has variants of its own for bo ## What reaches the client -`to_string()` of our error is what a client sees for anything that is not a provider answer. That is why messages are written for a reader, causes are rendered once and reqwest URLs are stripped on entry +A boundary renders anything that is not a provider answer with `litellm_error::Report`, so the reader sees every layer of the chain once. That is why messages are written for a reader, each layer leaves its cause to `source()`, and reqwest URLs are stripped on entry. A boundary that must not show the chain, such as the bridge's TLS settings error that names the setting but not the file, renders fixed text of its own and says so in its test -Provider bodies are passed through, so bound them. Build `Error::upstream` with `truncate_error_body(&text)` from `litellm_http::request`, which caps the body at 256 characters +Provider bodies are passed through, so they are bounded where they enter our types. `litellm_http::Error::upstream(status, body)` keeps the first 256 characters and marks the cut, so no caller can forget -```rust -if !status.is_success() { - return Err(litellm_http::Error::upstream(status.as_u16(), truncate_error_body(&text)).into()); -} -``` - -Never format `Debug` of an error into a response or a Python exception +Never format `Debug` of an error into a response or a Python exception. A type whose sources may hold secrets derives `veil::Redact`, so its `Debug` is safe to log ## Logging -Propagate with context, and let the terminal handler own the one log or trace event for a failed call. Do not log at every `?`. Diagnostic tracing excludes raw error strings and bodies, see [`rust-tracing`](../../rust-tracing/SKILL.md) +Propagate with context, and let the terminal handler own the one log or trace event for a failed call. Do not log at every `?`, and do not log an error you also return. Diagnostic tracing excludes raw error strings and bodies, see [`rust-tracing`](../../rust-tracing/SKILL.md) diff --git a/litellm-rust/.agents/skills/rust-error/references/classification.md b/litellm-rust/.agents/skills/rust-error/references/classification.md index f6b5e7e6daf..d0f709c9ec6 100644 --- a/litellm-rust/.agents/skills/rust-error/references/classification.md +++ b/litellm-rust/.agents/skills/rust-error/references/classification.md @@ -12,7 +12,7 @@ Downstream crates ask a lower error questions instead of matching its nested var | `upstream_response()` | the provider's own status and body to pass through | | `status()` | an HTTP status to report | | `tls_source()` | which TLS setting to blame in a config error | -| `is_computed_header()` | the caller forwarded a header the signer owns, so the host should take the call | +| `is_computed_header()` | the caller forwarded a header the signer owns, which `litellm_inference::Error` turns into `Unsupported` | Upper types add the questions their callers ask. `litellm_inference::Error` and the OCR error answer `is_missing_credential()` and `is_unsupported()`, which the gateway asks instead of reaching into `Auth` or `Unsupported` @@ -24,19 +24,33 @@ Self::Route(InferenceError::Auth(litellm_auth::Error::MissingApiKey { .. })) => Self::Route(error) if error.is_missing_credential() => StatusCode::UNAUTHORIZED, ``` +A predicate is an exhaustive `match` with no `_` arm, never `matches!`, so a new variant does not compile until it has answered every question on purpose + +```rust +pub fn is_request(&self) -> bool { + match &*self.0 { + Repr::Request(_) | Repr::BlockedUrl | Repr::DownloadTooLarge => true, + Repr::Connect(_) | Repr::Network(_) | Repr::Timeout(_) | Repr::Status { .. } => false, + } +} +``` + Add a predicate when a real caller needs a new distinction, and name it after the fact the caller needs, in the caller's words, not after the variant's spelling. An upper type's predicates call through to the nested error, as in `Self::Http(error) => error.is_request()` and `Self::Auth(error) => error.is_missing_credential()` A public enum may be matched one level down by the type that owns the variant, such as the gateway matching its own `Unsupported` or the OCR bridge matching `FileRead`. Going a second level down, into a variant of a variant, is what to avoid, the gateway used to do that for `MissingApiKey` and `Unsupported` ## Facts, not policy -A predicate reports what happened. The decision to retry, reroute or fall back depends on things the error cannot know, such as whether the request is idempotent, whether its body can be replayed, the deadline and the attempt budget. That decision lives with the caller +A predicate reports what happened. The decision to retry, reroute or fall back depends on things the error cannot know, such as whether the request is idempotent, whether its body can be replayed, the deadline and the attempt budget. That decision lives with the caller. `dispatched()` exists for the router's reroute decision, so every transport failure answers it even before that caller is wired A timeout is the main trap. A request that timed out may already have reached the provider and been billed, so `Timeout` answers `dispatched() == true` and is not safe to retry as if it were a connect failure. Only a failure that provably happened before any byte reached the provider answers `false`: connecting, building or signing the request, and media downloads, which fetch the caller's document before the provider call is made ```rust pub fn dispatched(&self) -> bool { - matches!(*self.0, Repr::Network(_) | Repr::Timeout(_) | Repr::Status { .. } | ..) + match &*self.0 { + Repr::Network(_) | Repr::Timeout(_) | Repr::Status { .. } => true, + Repr::Connect(_) | Repr::RequestBody(_) | Repr::Download(_) | Repr::BlockedUrl => false, + } } ``` diff --git a/litellm-rust/.agents/skills/rust-error/references/conversions.md b/litellm-rust/.agents/skills/rust-error/references/conversions.md index 1bfa3073aae..db46e4e3aa9 100644 --- a/litellm-rust/.agents/skills/rust-error/references/conversions.md +++ b/litellm-rust/.agents/skills/rust-error/references/conversions.md @@ -43,7 +43,7 @@ One `io::Error` can mean "reading the CA bundle failed" or "resolving the media ```rust .map_err(Repr::Resolve)? -.map_err(|cause| Repr::Read { path, tls_source, cause })? +.map_err(|source| Repr::Read { path, tls_source, source })? ``` Do not drop meaningful context just to use `?` @@ -71,9 +71,9 @@ All four inference routes classify a header the signer computes the same way, be |---|---|---| | has nothing to add | nest in one transparent variant and delegate predicates | `Self::Http(error) => error.is_request()` | | changes what the failure means | translate to an upper variant, keep the cause attached when it helps | computed header -> `Unsupported` | -| knows something the lower one does not | a variant with the extra fields plus the cause | `Read { path, tls_source, cause }` | +| knows something the lower one does not | a variant with the extra fields plus the cause as its source | `Read { path, tls_source, source }` | -Never re-declare a lower crate's variants in the upper type with an upper-layer name and a 1:1 mapping function. OCR once did that for every media download failure, `MediaError::BlockedUrl => Error::BlockedDocumentUrl` and so on, which doubled every change and let the two lists drift. `litellm_inference::Error`'s `From` still maps six same-named variants one to one. It predates this rule and is the shape to migrate away from, not to copy +Never re-declare a lower crate's variants in the upper type with an upper-layer name and a 1:1 mapping function. OCR once did that for every media download failure, `MediaError::BlockedUrl => Error::BlockedDocumentUrl` and so on, which doubled every change and let the two lists drift. ## Third-party and host faults @@ -86,3 +86,7 @@ Converting a fault from a crate we do not own into a string variant is still fla // keeps the cause typed .map_err(|error| Error::InvalidRequest(ErrorDetail::invalid("header", error))) ``` + +## Through a foreign error type + +A trait we implement for another crate sometimes forces its error type on us, such as `io::Error` from a body stream or `sqlx::Error` from a driver. Wrap ours as that type's source (`io::Error::other(error)`, `azure_core::Error::new(kind, error)`) and recover it with `downcast_ref` on the other side, never `with_message(error.to_string())` or `from_msg`. The error must come back out as the same value it went in as, so one failure classifies the same way on every path diff --git a/litellm-rust/.agents/skills/rust-error/references/gateway.md b/litellm-rust/.agents/skills/rust-error/references/gateway.md index 8e460ea746b..9386159e3b3 100644 --- a/litellm-rust/.agents/skills/rust-error/references/gateway.md +++ b/litellm-rust/.agents/skills/rust-error/references/gateway.md @@ -31,9 +31,14 @@ Do not reach into a nested enum's variants from the gateway. When a new nested f ## Render by surface and framing -Rendering is separate from classification. The surface picks the envelope, which is why `gateway_inference::Error` has both `body(request_id)` for the Anthropic shape and `openai_response()` for the OpenAI shape, and the moment picks the framing: a JSON body before the headers went out, an `event: error` SSE frame from `sse_frame()` after. Both framings render the same `body()`, so a streaming failure and a JSON failure of the same error agree on everything but framing +Rendering is separate from classification. The surface picks the envelope and the moment picks the framing, a JSON body before the headers went out and an `event: error` SSE frame after. Both framings of one surface render the same envelope, so a streaming failure and a JSON failure of the same error agree on everything but framing -Two rules hold for every surface. The error `type` string is derived from the status, so a new failure never names its own type. A provider body already in the surface's shape passes through untouched and anything else has its message extracted, with the body bounded by `truncate_error_body` when the error was built, see [boundaries](boundaries.md) +| Surface | JSON | SSE | +|---|---|---| +| Anthropic, `/v1/messages` | `body(request_id)` | `sse_frame()` | +| OpenAI, chat, responses and the rest | `openai_response()` | `openai_sse_frame()` | + +Two rules hold for both. The error `type` string is derived from the status by `error_type()`, so a new failure never names its own type, and the message of anything that is not a provider answer is the `litellm_error::Report` of the error. The Anthropic envelope passes a provider body already in its shape through untouched and extracts the message from anything else. The OpenAI envelope always wraps, because that is what the Python proxy returns there ## Every way in uses the same Error @@ -41,14 +46,10 @@ A request can fail before the handler runs, in the handler, or after the headers - An extractor sets `type Rejection = Error`, so a bad body is the surface's envelope rather than axum's plain text - A path the gateway knows but does not serve returns `Error::Unsupported(path)` from one handler, so the client gets a 501 in the surface's envelope rather than a bare 404 -- A stream that fails after its headers went out hands `Error::from(error).sse_frame()` to `litellm_host_http::Sse::new`. `litellm_host_http::Error` separates `Call(E)`, the route's own error, from `Protocol`, a host bug, and the `From` maps the second to `Internal` +- A stream that fails after its headers went out hands the surface's SSE frame of `Error::from(error)` to `litellm_host_http::Sse::new`. `litellm_host_http::Error` separates `Call(E)`, the route's own error, from `Protocol`, a host bug, and the `From` maps the second to `Internal` The axum hooks behind each line are in [`rust-gateway`](../../rust-gateway/SKILL.md) -## Known gaps - -`InvalidBody(String)` and `Internal(String)` flatten their cause into a string, against [messages and causes](messages-and-causes.md). A new variant carries the typed cause - ## Testing -`status()` and `body()` are pure, so unit tests call them on errors built the way production builds them, one `#[case]` per fault, as `status_follows_who_is_at_fault` and `body_follows_the_anthropic_exception_mapping` in `crates/gateway-inference/src/error.rs` do. Assert the status and the fields the surface promises, not the whole body. Driving a whole router belongs to the gateway crate's own tests, see [`rust-gateway`](../../rust-gateway/references/testing.md) +`status()` and `body()` are pure, so unit tests call them on errors built the way production builds them, one `#[case]` per fault, as `status_follows_who_is_at_fault` and `body_follows_the_anthropic_exception_mapping` in `crates/gateway-inference/src/error.rs` do. Assert the status and the fields the surface promises, not the whole body, except where passing the body through untouched is the contract. Driving a whole router belongs to the gateway crate's own tests, see [`rust-gateway`](../../rust-gateway/references/testing.md) diff --git a/litellm-rust/.agents/skills/rust-error/references/messages-and-causes.md b/litellm-rust/.agents/skills/rust-error/references/messages-and-causes.md index 2a422c26ebb..81a237219bf 100644 --- a/litellm-rust/.agents/skills/rust-error/references/messages-and-causes.md +++ b/litellm-rust/.agents/skills/rust-error/references/messages-and-causes.md @@ -9,7 +9,6 @@ Every message template is written once, in the variant's `#[error(...)]`. A call Error::Network("WebSocket handshake returned a wrong Sec-WebSocket-Accept".into()) Error::Request(format!("bad header {name}")) Error::Signature(error.to_string()) -Error::AwsSignableRequest(format!("{name} is not visible ASCII")) // the call site names what happened, the variant owns the words Repr::Handshake(HandshakeFault::Accept).into() @@ -17,84 +16,95 @@ Error::signature(error) Error::NonAsciiHeader { name } ``` -When a check fails for a distinct reason, that reason is a variant or a field, never a string inside a generic variant +When a check fails for a distinct reason, that reason is a variant or a field, never a string inside a generic variant. A `Message(String)` variant, or a `From` / `From<&str>` into an error, is the hole every stringly call site goes through, so neither exists Format arguments can call functions and reach into fields, so a hand-written `Display` is rarely needed ```rust -#[error("could not read {}: {cause}", path.display())] +#[error("could not read {}", path.display())] #[error("invalid model: {provider} has no model {model:?} - use one of: {}", supported.join(", "))] ``` -Keep messages short and lowercase unless Python parity pins the exact text. Do not end them with a period, since a reporter may append `: {cause}` +Keep messages short and lowercase, with no trailing period, unless Python parity pins the exact text. A reporter appends `: {cause}` after them. Never format `Debug` (`{:?}`) of an error, or of a type with no `Display`, into a message + +## The cause is the source, never the message + +This is the rule from the [`std::error::Error` docs](https://doc.rust-lang.org/std/error/trait.Error.html): an error's `Display` describes that layer only, and the error underneath is returned by `source()`. thiserror wires `source()` from a field marked `#[source]`, a `#[from]` field, or a field named `source` + +```rust +// prints the cause twice under any reporter +#[error("could not read {}: {source}", path.display())] +Read { path: PathBuf, source: io::Error }, +#[error("invalid JSON: {0}")] +Json(#[from] serde_json::Error), + +// this layer's words, the cause is the source +#[error("could not read {}", path.display())] +Read { path: PathBuf, source: io::Error }, +#[error("invalid JSON")] +Json(#[from] serde_json::Error), +``` + +| Field | thiserror does | Use for | +|---|---|---| +| `source: E` or `#[source] cause: E` | exposes it as `source()` | a cause with context fields next to it | +| `#[from] E` | implies `#[source]` and adds `From` | context-free wrapping, see [conversions](conversions.md) | +| `#[error(transparent)]` | forwards both `Display` and `source()` to the inner value | nesting another of our errors that adds no text | +| no attribute, any other name | nothing, the field is plain data | facts the message renders, such as a path or a status | + +`#[error(transparent)]` is an invisible layer, not an extra chain node, so a transparent wrapper's `source()` is the inner value's `source()` + +A field that holds an error but is neither rendered nor a source hides that error from every reader. Every error-typed field is a source ## Keep causes typed -Store the lower-level error itself, never its string +Store the lower-level error itself, never its string, and never drop it with `map_err(|_| ...)` -| Flattening loses | Because | +| Flattening or dropping loses | Because | |---|---| -| `source()` | a `String` has no cause | +| the chain | a `String` has no `source()` | | downcasting | the type is gone | | redaction | `reqwest::Error`'s `Display` includes the request URL, which can carry credentials in the path or query | -If the cause is not `Clone` and the error must be, wrap the cause in `Arc` or make the crate's error opaque. Do not stringify to get `Clone` +If the cause is not `Clone` and the error must be, hold it as `litellm_error::Source`, a cloneable `Arc` that is transparent to `Display`, `source()` and `downcast_ref`, or make the crate's error opaque. Do not stringify or drop a cause to get `Clone` or `PartialEq` ```rust // loses the type and leaks the URL .map_err(|error| Error::Network(error.to_string())) +// loses everything +.map_err(|_| Error::Transport) // keeps the cause, the From impl strips the URL .map_err(litellm_http::Error::from) ``` +A cause that must not be shown at all, such as a secret manager's response, is still a source. The type derives [`veil::Redact`](https://docs.rs/veil) with `#[redact]` on the field, so `Debug` hides it, and the boundary decides how much of the chain to show, see [boundaries](boundaries.md) + ## Strip reqwest URLs on entry -Call `without_url()` where a `reqwest::Error` enters our type, which in practice is `From for litellm_http::Error`. Once it is inside, every later `Display`, `Debug` or chain walk is safe from that URL +Every `reqwest::Error` enters our types through `From for litellm_http::Error`, which calls `without_url()`. A crate that talks HTTP holds `litellm_http::Error`, never `reqwest::Error`, so every later `Display`, `Debug` or chain walk is safe from that URL `without_url()` only removes the URL reqwest attached. It does not scrub headers, response bodies, provider messages, file paths or anything a variant stores next to it. Those need their own policy, see [boundaries](boundaries.md) -## Render each cause once +## Rendering -Either the variant's message includes the cause or `source()` returns it, never both, or a reporter that walks the chain prints it twice. This is the rule from the [`std::error::Error` docs](https://doc.rust-lang.org/std/error/trait.Error.html) - -Our boundaries (the gateway error body and the Python bridge) render with `to_string()` and never walk the chain. So in this workspace a leaf variant puts its cause in its message and stores it in a field named `cause`, with no `#[source]`. thiserror treats a field named `source` as the source even without the attribute, so the name matters. `litellm_llms::ErrorDetail`, the auth, inference and OCR errors all follow this - -| Field | thiserror does | Use for | -|---|---|---| -| `cause: io::Error` rendered in `#[error]` | nothing, `source()` stays `None` | leaf foreign causes, the default here | -| `source: io::Error` or `#[source]` | exposes it as `source()` | causes the message does not mention | -| `#[from] io::Error` | implies `#[source]` and adds `From` | context-free wrapping, message must not mention it | -| `#[error(transparent)]` | forwards both `Display` and `source()` to the inner value | wrapping another of our errors that adds no text | +Anything that shows an error to someone else renders the whole chain with `litellm_error::Report`, which writes each layer once joined by `: `, the single-line form of the unstable [`std::error::Report`](https://doc.rust-lang.org/std/error/struct.Report.html). `to_string()` of an error is only its top layer and is never what a client, a Python exception or a log line shows ```rust -// prints the reqwest error twice under a chain-walking reporter -#[error("request failed: {source}")] -Request { source: reqwest::Error }, - -// leaf, rendered once in the message -#[error("could not read {}: {cause}", path.display())] -Read { path: PathBuf, cause: io::Error }, - -// nested, adds no text of its own -#[error(transparent)] -Http(litellm_http::Error), +let message = litellm_error::Report::new(&error).to_string(); +// "could not reach the provider: error sending request: client error (Connect): tcp connect error: Connection refused" ``` -`#[error(transparent)]` is an invisible layer, not an extra chain node. A transparent wrapper's `source()` is the inner value's `source()`, so a transparent wrapper over a leaf has no source at all - -`reqwest::Error` keeps the useful part of its message in its own source chain (the hyper and io errors), so the private `Chain` helper in `crates/http/src/error.rs` renders `reqwest::Error` plus its sources into one message. Every reqwest error enters our types in that crate, so that is the only place it is needed - ## #[from] limits `#[from]` only works on a variant whose only fields are the source and an optional backtrace. A variant that needs context such as a path, a phase or a `TlsSource` takes its cause through an explicit `map_err` or a named constructor ```rust -std::fs::read(&path).map_err(|cause| Repr::Read { +std::fs::read(&path).map_err(|source| Repr::Read { path: path.clone(), tls_source: TlsSource::CaBundle, - cause, + source, })?; ``` -`#[backtrace]` and automatic `provide()` need a nightly compiler and are not used here. `std::backtrace::Backtrace` itself is stable, but do not add a field of that type to a derived error without checking how thiserror treats it +`#[backtrace]` and automatic `provide()` need a nightly compiler and are not used here diff --git a/litellm-rust/.agents/skills/rust-error/references/pyo3.md b/litellm-rust/.agents/skills/rust-error/references/pyo3.md index 797bceae68e..b830332d5a7 100644 --- a/litellm-rust/.agents/skills/rust-error/references/pyo3.md +++ b/litellm-rust/.agents/skills/rust-error/references/pyo3.md @@ -1,56 +1,49 @@ # Python bridge -`crates/python-bridge` turns a Rust error into a Python exception in one function per route family: `errors::route_error_to_pyerr` for the inference routes and `routes/ocr/errors.rs::to_pyerr` for OCR. A `#[pyfunction]` returns `Err(PyErr)` and pyo3 raises it, so those two functions are the whole boundary. No route builds a `PyErr` from its own error inline +`crates/python-bridge` turns a Rust error into a Python exception through one mapping function per error type. A `#[pyfunction]` returns `Err(PyErr)` and pyo3 raises it, so those functions are the whole boundary + +| Error | Mapping | +|---|---| +| `litellm_inference::Error` from a route | `errors::route_error_to_pyerr` | +| the OCR error | `routes/ocr/errors.rs::to_pyerr` | +| token counter, cache, spend, secrets, argument projection | one `From for PyErr` or `*_to_pyerr` next to the code that registers the functions | + +A route host never builds a `PyErr` from a route error itself. It calls the mapping, and the mapping is the only place that knows the exception classes and their attributes ## Exception classes | Class | Meaning | Payload | |---|---|---| -| `RustUpstreamError` | the provider call was issued and failed | `args == (status, body)`, `status` is 0 when there was no HTTP response, plus a `headers` attribute of `(name, value)` pairs | +| `RustUpstreamError` | the provider call was issued and the provider answered with a failure | `args == (status, body)` and a `headers` attribute of `(name, value)` pairs, always set | | `RustBridgeDeclined` | the route stopped before calling the provider, so the host may run its own path | `args == (message,)` | -| `ValueError` | the caller's input is at fault | the message | -| `RuntimeError` | the environment, the wire or the provider's answer is at fault | the message | +| a standard class from `pyo3::exceptions` | the caller's input (`ValueError`, `TypeError`), a missing file (`FileNotFoundError`), anything else (`RuntimeError`) | the rendered chain | -The two native classes come from `create_exception!` in `crates/python-bridge/src/errors.rs` and are exported with `#[pymodule_export]`, which is how the [pyo3 guide](https://github.com/PyO3/pyo3/blob/main/guide/src/exception.md) makes an exception importable. Their docstrings are the contract Python reads +The two native classes come from `create_exception!` in `crates/python-bridge/src/errors.rs` and are exported with `#[pymodule_export]`, which is how the [pyo3 guide](https://github.com/PyO3/pyo3/blob/main/guide/src/exception.md) makes an exception importable. Their docstrings are the contract Python reads. `errors::upstream_error(py, status, body, headers)` is the one constructor of `RustUpstreamError`, so the payload cannot differ between routes -Python decodes them once. `litellm/rust_bridge/failures.py` validates `args` and `headers` with a `TypeAdapter` and rebuilds the provider's `httpx.Response`, and `litellm/rust_bridge/runtime.py` catches `RustBridgeDeclined`. Nothing else in Python inspects a native exception, so a new fact goes into `args` or that one `headers` attribute, and the decoder learns to read it in the same change +Python decodes a `RustUpstreamError` in `litellm/rust_bridge/failures.py`, which validates `args` and `headers` with a `TypeAdapter` and rebuilds the provider's `httpx.Response`. `litellm/rust_bridge/runtime.py` catches `RustBridgeDeclined` and reads its reason. `_raise_upstream` in `runtime.py` still reads `args` a second time by hand for the `attempt` path, which is the one decode to fold into `failures.py`. A new fact goes into `args` or `headers`, and the decoder learns to read it in the same change + +Two routes also flag a request fault with a boolean attribute, read by their Python route module: `ocr_request_format_error` in `host/routes/ocr.py` and `messages_request_error` in `host/routes/messages.py`. OCR additionally sets `status_code` and `message` for Python parity. Do not add another attribute, nothing on the Python side types them ## Why args, not a typed subclass The bridge builds with pyo3's `abi3-py310` feature. Under abi3, `#[pyclass(extends = PyException)]` is only allowed on Python 3.12 and newer, so a class with typed fields is not available. Even where it is allowed, pyo3 passes an empty tuple to the native base's `tp_new`, so `.args` and `str()` would need manual overrides. The payload therefore travels in `args`, which a `create_exception!` class fills from `new_err` -```rust -RustUpstreamError::new_err((status, body.to_owned())) -``` - -OCR still sets `status_code`, `message` and `ocr_request_format_error` on the exception with `setattr` for Python parity. Do not add more attributes that way, nothing on the Python side types them - ## Mapping order -```rust -pub(crate) fn route_error_to_pyerr(error: Error) -> PyErr { - match error.upstream_response() { - Some((status, body)) => RustUpstreamError::new_err((status, body.to_owned())), - None => by_fault(error.is_request(), error.to_string()), - } -} -``` - -The provider's answer passes through first. Everything else is split by `is_request()` into `ValueError` or `RuntimeError`, with `to_string()` as the message. A missing API key is a `RuntimeError` because `is_request()` answers false for it, and `missing_api_key_stays_a_runtime_error_while_other_auth_failures_are_value_errors` pins that +The provider's answer passes through first as `RustUpstreamError`. Everything else is split by `is_request()` into `ValueError` or `RuntimeError`, with the message rendered by `litellm_error::Report`. A missing API key is a `RuntimeError` because `is_request()` answers false for it, and `missing_api_key_stays_a_runtime_error_while_other_auth_failures_are_value_errors` pins that OCR puts Python parity arms in front of that order, each matching a top-level variant of its own public enum | Arm | Exception | |---|---| -| `Provider { status, body, headers }` | `RustUpstreamError` with the headers attached | -| `Http` with a `status()` | `RustUpstreamError`, which also carries a media download status, a known gap | +| the provider answered | `RustUpstreamError` with the provider's headers | +| `RequestFormat` | `ValueError` flagged with `ocr_request_format_error` | | `FileRead` whose cause is `NotFound` | `FileNotFoundError` naming the path | | other `FileRead` | `OSError` with the io message | -| `RequestFormat` | `ValueError` flagged with `ocr_request_format_error` | -OCR also treats every auth failure as the caller's, so a missing key is a `ValueError` there. That is the one place the two bridges disagree +OCR also treats every auth failure as the caller's, so a missing `litellm_auth` key is a `ValueError` there, while the inference routes raise `RuntimeError` -Use the standard class whose Python meaning matches, from `pyo3::exceptions`. `PyFileNotFoundError` for a missing file is right, a `RuntimeError` whose text says the file is missing is not. `import_exception!` exists for an exception defined in Python, none is used today because `failures.py` maps to LiteLLM's public exceptions through `litellm.exception_type` +Use the standard class whose Python meaning matches. `PyFileNotFoundError` for a missing file is right, a `RuntimeError` whose text says the file is missing is not. `import_exception!` exists for an exception defined in Python, none is used today because `failures.py` maps to LiteLLM's public exceptions through `litellm.exception_type` ## Declining @@ -59,9 +52,10 @@ Use the standard class whose Python meaning matches, from `pyo3::exceptions`. `P ## Rules - Never format `Debug` of an error into a `PyErr` -- Never build a `PyErr` at a call site from a string. Return the crate error and let the mapping function classify it -- A `panic!` or `unwrap` inside a `#[pyfunction]` reaches Python as `pyo3_runtime.PanicException`, which is not an `Exception` subclass. Model the failure as a value instead -- When the mapping itself calls Python, as the OCR `setattr` does, a failure there is Python's own exception. Return it as is rather than wrapping it in a Rust message +- Never build a `PyErr` from a string at a call site. A failed argument check is a variant of a typed error, as `ProjectionError` in `coercion.rs` is, mapped once +- A Python exception raised inside the call stays that exception. Return it as is, never wrap it in a Rust message or stringify it into a new `ValueError` +- A `panic!`, `unwrap` or `expect` inside a `#[pyfunction]` or `#[getter]` reaches Python as `pyo3_runtime.PanicException`, which is not an `Exception` subclass. Model the failure as a value instead +- When the mapping itself calls Python, as `setattr` does, propagate its failure with `?` rather than discarding it with `.ok()` ## Testing @@ -70,7 +64,7 @@ Build the Rust error the way production builds it, map it, and assert the contra ```rust Python::initialize(); Python::attach(|py| { - let failure = route_error_to_pyerr(litellm_http::Error::upstream(429, "slow down".into()).into()); + let failure = route_error_to_pyerr(py, litellm_http::Error::upstream(429, "slow down".into()).into()); assert!(failure.is_instance_of::(py)); let args: (u16, String) = failure.value(py).getattr("args").unwrap().extract().unwrap(); assert_eq!(args, (429, "slow down".into())); diff --git a/litellm-rust/.agents/skills/rust-error/references/recipes.md b/litellm-rust/.agents/skills/rust-error/references/recipes.md index c5bddca3f4e..80ba1512f77 100644 --- a/litellm-rust/.agents/skills/rust-error/references/recipes.md +++ b/litellm-rust/.agents/skills/rust-error/references/recipes.md @@ -6,38 +6,37 @@ Say media downloads must reject a `Content-Type` outside an allow-list 1. Add a `Repr` variant with typed facts and the message, `#[error("media type {0:?} is not allowed")] DisallowedMediaType(String)` 2. Return it where the check fails, `return Err(Repr::DisallowedMediaType(content_type.to_owned()).into())` -3. Answer every predicate on purpose, the way its neighbor `DownloadTooLarge` does. A media download happens before the provider call, so it is not in `dispatched()`. The caller chose the URL, so it is in `is_request()` +3. Add an arm to every predicate's exhaustive `match`, the compiler lists them, the way its neighbor `DownloadTooLarge` answers. A media download happens before the provider call, so it is not in `dispatched()`. The caller chose the URL, so it is in `is_request()` 4. Leave both boundaries alone. With those answers the gateway returns 400 and Python raises `ValueError` 5. Test it with a mock server answering `Content-Type: application/x-msdownload` and assert `error.is_request()` -## A provider-specific failure with a parity message +## A provider-specific failure -Python raises `Missing REDUCTO_API_KEY - set it in the environment or pass api_key to litellm.ocr()/litellm.aocr()` and a test pins the text. The OCR error carries this today as `MissingReductoApiKey`, which is the shape to avoid +A second OCR provider needs a missing-key error whose text names its environment variable and how to set it -1. Look for an existing provider-neutral variant whose template produces the exact text. `litellm_auth::Error::MissingApiKey { provider, environment_variable }` renders `Missing {provider} API Key - Set ...`, which does not match -2. Find what varies between providers with the same failure. Here it is the environment variable and the guidance after the dash -3. Add one neutral variant with those as fields, `#[error("Missing {environment_variable} - {guidance}")] MissingCredential { environment_variable: &'static str, guidance: &'static str }`, and pass Reducto's values from the Reducto transformation -4. The next provider with the same failure passes its own values and adds no variant. `is_missing_credential()` lists `MissingCredential { .. }` once and the boundaries need no change +1. Look for an existing provider-neutral variant whose template fits. `litellm_auth::Error::MissingApiKey { provider, environment_variable }` renders `Missing {provider} API Key - Set ...` +2. If none fits, find what varies between providers with the same failure, here the environment variable and the guidance after the dash, and add one neutral variant with those as fields, `#[error("Missing {environment_variable} - {guidance}")] MissingCredential { environment_variable: &'static str, guidance: &'static str }` +3. Pass the provider's values from its transformation. The next provider passes its own and adds no variant, and `is_missing_credential()` lists `MissingCredential { .. }` once ## Wrapping a new lower crate in litellm_inference Say routes start calling `litellm_cache` and its errors must surface -1. Add one variant, `#[error(transparent)] Cache(litellm_cache::Error)`, with `#[from]` if no context is needed -2. Extend `is_request()` and `upstream_response()` with an arm that calls through to the cache error's own predicates, adding a predicate there if it has none +1. Add one variant, `#[error(transparent)] Cache(#[from] litellm_cache::Error)` +2. Extend every predicate's `match` with an arm that calls through to the cache error's own predicates, adding a predicate there if it has none 3. Do not copy the cache error's variants into `litellm_inference::Error` ## A foreign error that needs context Reading a file fails with `io::Error`, and the message must name the file -1. A variant with the context and the cause, `Read { path: PathBuf, cause: io::Error }` with `#[error("could not read {}: {cause}", path.display())]` -2. Build it with `map_err(|cause| Repr::Read { path: path.clone(), cause })`, not a `From` -3. If the error must be `Clone`, the crate's error is opaque or the field is `Arc` +1. A variant with the context and the cause, `Read { path: PathBuf, source: io::Error }` with `#[error("could not read {}", path.display())]` +2. Build it with `map_err(|source| Repr::Read { path: path.clone(), source })`, not a `From` +3. If the error must be `Clone`, the crate's error is opaque or the field is `litellm_error::Source` ## Migrating a stringly error -Call sites look like `Error::Network(error.to_string())` or `Error::Request(format!(...))` +Call sites look like `Error::Network(error.to_string())`, `Error::Request(format!(...))` or `.map_err(|_| Error::Unavailable)` 1. List every call site of the string variant and group them by what actually failed 2. Each group becomes a variant holding the typed cause or facts. Groups no caller tells apart share one @@ -45,6 +44,15 @@ Call sites look like `Error::Network(error.to_string())` or `Error::Request(form 4. Delete the string variant. If it cannot be deleted yet, nothing new may construct it 5. Re-run the boundary tests, since messages a client sees may change +## Moving a crate to source chains + +A variant reads `#[error("invalid JSON: {0}")] Json(#[from] serde_json::Error)` or stores `cause` and renders `{cause}` + +1. Drop the cause from the template, `#[error("invalid JSON")]` +2. Make the cause the source, `#[from]`, `#[source]` or a field named `source` +3. Find what renders this error for someone else and make it use `litellm_error::Report` +4. Update tests that asserted the cause in `to_string()` to assert it in the report + ## Making a public enum opaque Downstream crates match nested variants, or a variant needs a non-`Clone` cause diff --git a/litellm-rust/.agents/skills/rust-error/references/shape.md b/litellm-rust/.agents/skills/rust-error/references/shape.md index 75da8f1b8cf..e8467ff57d2 100644 --- a/litellm-rust/.agents/skills/rust-error/references/shape.md +++ b/litellm-rust/.agents/skills/rust-error/references/shape.md @@ -4,6 +4,8 @@ A crate's errors live in `src/error.rs` and are re-exported from `lib.rs` as `Error`. A caller of any public function in the crate should only ever see `Result<_, crate::Error>`, so one `?` and one set of predicates cover the whole crate +This deliberately departs from the advice to put an error type next to each unit of fallibility ([Sabrina Jewson, "Modular Errors"](https://sabrinajewson.org/blog/errors)). Our callers do not branch on which function failed, they ask the predicates in [classification](classification.md), and the boundaries map one type per crate. Splitting by function would multiply those predicates and mappings without giving any caller a decision it lacks. Layering, the other half of that advice, is kept: each crate nests the one below it, see [conversions](conversions.md) + That one type takes one of two forms | Form | Use when | Example | @@ -30,8 +32,8 @@ pub struct Error(Arc); #[derive(Debug, thiserror::Error)] pub(crate) enum Repr { - #[error("could not reach the provider: {}", Chain(.0))] - Connect(reqwest::Error), + #[error("could not reach the provider")] + Connect(#[source] reqwest::Error), #[error("upstream request failed with status {status}: {body}")] Status { status: u16, body: String }, } @@ -43,6 +45,8 @@ In the opaque form other crates cannot name a variant, so they cannot build an e `Arc` makes cloning a refcount bump, so `Repr` never has to be `Clone`. `Box` is enough when the goal is only a small `Result`. `Arc` is for when callers clone +Every crate here is internal to the workspace, so do not mark errors or variants `#[non_exhaustive]`. It only forces a wildcard arm on the other workspace crates and turns off the exhaustiveness check that makes a new variant visit every `match`. When the variants must be free to change, use the opaque form + ## What else may sit next to Error These are not a second top-level error and need no folding in @@ -95,3 +99,5 @@ Split into a separate type only when different functions fail in disjoint ways a Name a split type after what went wrong, not after the function that returns it. `ConnectError` reads better than `OpenSessionError`, and a unit struct is fine for a single failure mode A split type still converts into the crate's `Error` when a caller further up needs one `Result` type + +A utility crate whose modules are unrelated, such as `core-utils`, has no crate-wide failure, so each module that can fail owns one error next to its code instead of a `src/error.rs`. The rest of this skill applies to each of them diff --git a/litellm-rust/.agents/skills/rust-error/references/testing.md b/litellm-rust/.agents/skills/rust-error/references/testing.md index 71d24987e2f..b6b4da72d6c 100644 --- a/litellm-rust/.agents/skills/rust-error/references/testing.md +++ b/litellm-rust/.agents/skills/rust-error/references/testing.md @@ -20,7 +20,7 @@ In order of preference 2. A real cause, such as `tungstenite::Error::ConnectionClosed` or a `serde_json::from_str("{")` error, passed through our `From` 3. A public constructor, such as `Error::upstream(500, "boom".into())` -Inside the owning crate a test may match the private representation through a `#[cfg(test)] fn repr(&self) -> &Repr` accessor, as `crates/http/src/media.rs` does. A public `ErrorDetail` can be destructured anywhere, and its `cause` derefs to `dyn Error` for a `downcast_ref` +Inside the owning crate a test may match the private representation through the `#[cfg(test)] fn repr(&self) -> &Repr` accessor in `crates/http/src/error.rs`, as the `media.rs` and `tls.rs` tests do. Anywhere else, walk `source()` and `downcast_ref` to the cause the test expects ## What to cover @@ -28,8 +28,10 @@ Inside the owning crate a test may match the private representation through a `# |---|---| | classification | one `#[case]` per input class, asserting the predicate | | redaction | build from a URL holding `api_key=secret`, assert the message lacks `secret` and the path | -| cause kept | destructure the variant and downcast the `cause` field, or assert the message contains the root cause's text | -| rendered once | the cause's text appears once in `to_string()` and `source()` is `None`, since the message already carries it | +| cause kept | walk `source()` and `downcast_ref` to the expected type | +| rendered once | the cause's text appears exactly once in `litellm_error::Report::new(&error).to_string()` and not in `to_string()` | | boundary mapping | the gateway status and body, or the Python exception type and its `status` attribute | -Assert exact message text only where it is a contract, such as a message Python parity pins. Everywhere else assert that the message contains the facts it must carry +Assert exact message text only where it is a contract, such as a message Python parity pins. Everywhere else assert that the message contains the facts it must carry. Matching a fieldless variant with `matches!` is fine, full `assert_eq!` on an error that holds a cause is not, since causes compare by pointer + +A test that returns `Result` reports only `Debug` of the error on failure. Prefer `expect` with a short reason, or `unwrap_err` followed by assertions on the predicates diff --git a/litellm-rust/.agents/skills/rust-gateway/SKILL.md b/litellm-rust/.agents/skills/rust-gateway/SKILL.md index d176eeb778d..66b93ee128d 100644 --- a/litellm-rust/.agents/skills/rust-gateway/SKILL.md +++ b/litellm-rust/.agents/skills/rust-gateway/SKILL.md @@ -5,7 +5,7 @@ description: Add or change an HTTP route, extractor, response envelope, stream o # Rust gateway -`crates/gateway` mounts endpoint routers and serves the listener. `crates/gateway-inference` owns the inference endpoints, `gateway-mcp`, `gateway-ui`, `gateway-management` and `gateway-auth` own theirs. Each crate's `AGENTS.md` says what belongs in it. Read the reference for the part you are touching +`crates/gateway` mounts endpoint routers and serves the listener. `crates/gateway-inference`, `gateway-mcp` and `gateway-ui` own their endpoints, `gateway-auth` is the inbound authentication middleware and `gateway-management` holds management logic that has no router yet. A crate's `AGENTS.md`, where it has one, says what belongs in it. Read the reference for the part you are touching - [surfaces](references/surfaces.md) - Each API surface mirrors the envelope the Python proxy returns there, so the shapes differ on purpose diff --git a/litellm-rust/.agents/skills/rust-gateway/references/requests.md b/litellm-rust/.agents/skills/rust-gateway/references/requests.md index c4799c40487..dd05f65c2bc 100644 --- a/litellm-rust/.agents/skills/rust-gateway/references/requests.md +++ b/litellm-rust/.agents/skills/rust-gateway/references/requests.md @@ -2,7 +2,7 @@ ## Extractors reject with the crate's Error -A custom extractor sets `type Rejection = Error`, so a bad body gets the surface's envelope rather than axum's plain-text rejection. `JsonObject` and `InferenceBody` in `crates/gateway-inference/src/request.rs` are the pattern +A custom extractor sets `type Rejection = Error`, so a bad body gets the surface's envelope rather than axum's plain-text rejection. Never take axum's own `Json`, `Query` or `Path` in a handler, their rejection is plain text. The rejection is the variant's source, never its string. `JsonObject` and `InferenceBody` in `crates/gateway-inference/src/request.rs` are the pattern ```rust impl FromRequest for JsonObject { @@ -13,7 +13,7 @@ impl FromRequest for JsonObject { if error.status() == StatusCode::PAYLOAD_TOO_LARGE { Error::BodyTooLarge } else { - Error::InvalidBody(error.to_string()) + Error::InvalidBody(error) } })?; object(&body).map(Self) @@ -25,10 +25,10 @@ When the handler needs something from the request in the error body, such as the ## Paths -Every alias of a route is registered together in `crates/gateway-inference/src/lib.rs`, the bare path, its `/v1` form and the deployment-prefixed form, otherwise one alias silently behaves differently from the others. A path the gateway knows but does not serve, such as `/v1/completions`, is routed to `request::unsupported`, which returns `Error::Unsupported(path)` so the client gets a 501 in the surface's envelope rather than a bare 404 +Every alias the Python proxy registers for a route is registered together in `crates/gateway-inference/src/lib.rs`, such as the bare path, its `/v1` form and the `/engines/{model}` and `/openai/deployments/{model}` forms, otherwise one alias silently behaves differently from the others. A path the gateway knows but does not serve, such as `/v1/completions`, is routed to `request::unsupported`, which returns `Error::Unsupported(path)` so the client gets a 501 in the surface's envelope rather than a bare 404 `crates/gateway` merges the mounted routers and adds the shared inbound middleware as `route_layer`s, so authentication runs only for matched routes. Endpoint paths and parsing stay in the mounted crate, see the two crates' `AGENTS.md` ## Streaming -Once the response headers are sent the status cannot change. A streaming handler builds `litellm_host_http::Sse::new(Json, |error| Bytes::from(Error::from(error).sse_frame()))`, where the closure renders a mid-stream failure as an `event: error` frame through the same `body()` a JSON failure would use. `litellm_host_http::Error` separates `Call(E)`, the route's own error, from `Protocol`, a host bug, and the crate's `From` maps the second to `Internal` +Once the response headers are sent the status cannot change. A streaming handler builds `litellm_host_http::Sse::new(Json, |error| Bytes::from(Error::from(error).sse_frame()))`, where the closure renders a mid-stream failure as an `event: error` frame through the same envelope a JSON failure on that surface uses, `sse_frame()` for Anthropic and `openai_sse_frame()` for OpenAI. `litellm_host_http::Error` separates `Call(E)`, the route's own error, from `Protocol`, a host bug, and the crate's `From` maps the second to `Internal` diff --git a/litellm-rust/.agents/skills/rust-gateway/references/surfaces.md b/litellm-rust/.agents/skills/rust-gateway/references/surfaces.md index 9a4a4d0aa1b..a0b408abde6 100644 --- a/litellm-rust/.agents/skills/rust-gateway/references/surfaces.md +++ b/litellm-rust/.agents/skills/rust-gateway/references/surfaces.md @@ -4,13 +4,13 @@ Each surface mirrors what the Python proxy returns there, so the shapes differ o | Surface | Envelope | Where | |---|---|---| -| `/v1/messages` | Anthropic, `{"type": "error", "error": {"type", "message"}, "request_id"}` | `Error::body()` in `crates/gateway-inference/src/error.rs` | -| OpenAI-shaped routes | `{"error": {"message", "type", "param", "code"}}` | `Error::openai_response()` in the same file | +| `/v1/messages` | Anthropic, `{"type": "error", "error": {"type", "message"}, "request_id"}` | `Error::body()` and `sse_frame()` in `crates/gateway-inference/src/error.rs` | +| OpenAI-shaped routes | `{"error": {"message", "type", "param", "code"}}` | `Error::openai_response()` and `openai_sse_frame()` in the same file | | MCP | `{"detail": message}` | `crates/gateway-mcp/src/error.rs` | -| UI | status plus a message | `crates/gateway-ui/src/error.rs` | +| UI | `{"error": {"message"}}`, plus `Retry-After` on 429 | `crates/gateway-ui/src/error.rs` | | auth rejections | plain text | `crates/gateway-auth/src/error.rs` | -Two rules hold on every surface. The error `type` string is derived from the status by `error_type()`, so a new failure never names its own type. A provider body already in the surface's shape passes through untouched, anything else has its message extracted and re-enveloped, and the body was bounded by `truncate_error_body` when the error was built +Every surface takes its status from one `status()` on its crate's `Error` and renders the message of anything that is not a provider answer with `litellm_error::Report`. In `gateway-inference` the error `type` string is derived from the status by `error_type()`, so a new failure never names its own type. The Anthropic envelope passes a provider body already in its shape through untouched and extracts the message from anything else, and the OpenAI envelope always wraps, as the Python proxy does. A provider body is bounded by `litellm_http::Error::upstream` when the error is built `/v1/messages` adds the request id to the envelope when the provider did not set one, which is why its handler takes the body as `Result` and renders the rejection itself, see [requests](requests.md) diff --git a/litellm-rust/Cargo.lock b/litellm-rust/Cargo.lock index d2b1354901c..461bb212db7 100644 --- a/litellm-rust/Cargo.lock +++ b/litellm-rust/Cargo.lock @@ -3712,6 +3712,14 @@ dependencies = [ "tokio", ] +[[package]] +name = "litellm-error" +version = "0.1.0" +dependencies = [ + "rstest", + "thiserror 2.0.19", +] + [[package]] name = "litellm-framer" version = "0.1.0" diff --git a/litellm-rust/Cargo.toml b/litellm-rust/Cargo.toml index 3cb0f9cf06a..449d2313feb 100644 --- a/litellm-rust/Cargo.toml +++ b/litellm-rust/Cargo.toml @@ -10,6 +10,7 @@ repository = "https://github.com/BerriAI/litellm" [workspace.dependencies] litellm-config = { path = "crates/config" } +litellm-error = { path = "crates/error" } litellm-router = { path = "crates/router" } litellm-router-types = { path = "crates/router-types" } litellm-tracing = { path = "crates/tracing" } diff --git a/litellm-rust/crates/error/Cargo.toml b/litellm-rust/crates/error/Cargo.toml new file mode 100644 index 00000000000..fa2d26a1b99 --- /dev/null +++ b/litellm-rust/crates/error/Cargo.toml @@ -0,0 +1,13 @@ +[package] +name = "litellm-error" +version = "0.1.0" +edition.workspace = true +license.workspace = true +repository.workspace = true +description = "Shared error rendering and type-erased causes for litellm-rust boundaries" + +[dependencies] +thiserror.workspace = true + +[dev-dependencies] +rstest.workspace = true diff --git a/litellm-rust/crates/error/src/lib.rs b/litellm-rust/crates/error/src/lib.rs new file mode 100644 index 00000000000..ce16ed3846a --- /dev/null +++ b/litellm-rust/crates/error/src/lib.rs @@ -0,0 +1,106 @@ +use std::{error::Error, fmt, ops::Deref, sync::Arc}; + +pub struct Report<'a>(&'a (dyn Error + 'static)); + +impl<'a> Report<'a> { + pub fn new(error: &'a (dyn Error + 'static)) -> Self { + Self(error) + } +} + +impl fmt::Display for Report<'_> { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(formatter, "{}", self.0)?; + std::iter::successors(self.0.source(), |&cause| cause.source()) + .try_for_each(|cause| write!(formatter, ": {cause}")) + } +} + +#[derive(Clone, Debug, thiserror::Error)] +#[error(transparent)] +pub struct Source(Arc); + +impl Source { + pub fn new(error: impl Error + Send + Sync + 'static) -> Self { + Self(Arc::new(error)) + } +} + +impl Deref for Source { + type Target = dyn Error + Send + Sync; + + fn deref(&self) -> &Self::Target { + self.0.as_ref() + } +} + +impl PartialEq for Source { + fn eq(&self, other: &Self) -> bool { + Arc::ptr_eq(&self.0, &other.0) + } +} + +impl Eq for Source {} + +#[cfg(test)] +mod tests { + use std::io; + + use rstest::rstest; + + use super::{Report, Source}; + + #[derive(Debug, thiserror::Error)] + enum Leaf { + #[error("could not read the config")] + Read(#[source] io::Error), + #[error("could not parse the config")] + Parse(#[source] Source), + #[error("config is empty")] + Empty, + } + + #[derive(Debug, thiserror::Error)] + enum Outer { + #[error("startup failed")] + Startup(#[from] Leaf), + #[error(transparent)] + Nested(Leaf), + } + + fn denied() -> io::Error { + io::Error::new(io::ErrorKind::PermissionDenied, "permission denied") + } + + #[rstest] + #[case::leaf(Outer::Nested(Leaf::Empty), "config is empty")] + #[case::context_over_a_cause( + Outer::Startup(Leaf::Read(denied())), + "startup failed: could not read the config: permission denied" + )] + #[case::transparent_adds_no_text( + Outer::Nested(Leaf::Read(denied())), + "could not read the config: permission denied" + )] + #[case::erased_cause_keeps_its_chain( + Outer::Nested(Leaf::Parse(Source::new(Leaf::Read(denied())))), + "could not parse the config: could not read the config: permission denied" + )] + fn report_renders_every_layer_once(#[case] error: Outer, #[case] expected: &str) { + assert_eq!(Report::new(&error).to_string(), expected); + } + + #[test] + fn a_source_still_downcasts_to_the_original_cause() { + let source = Source::new(denied()); + let cause = source.downcast_ref::().expect("io cause"); + assert_eq!(cause.kind(), io::ErrorKind::PermissionDenied); + } + + #[test] + fn a_clone_shares_its_cause() { + let source = Source::new(denied()); + assert_eq!(source.clone(), source); + assert_ne!(Source::new(denied()), source); + } +}