diff --git a/litellm-rust/.agents/skills/rust-error/references/boundaries.md b/litellm-rust/.agents/skills/rust-error/references/boundaries.md index b4450d939b3..253cf52a7db 100644 --- a/litellm-rust/.agents/skills/rust-error/references/boundaries.md +++ b/litellm-rust/.agents/skills/rust-error/references/boundaries.md @@ -30,7 +30,7 @@ The two boundaries are not identical. The gateway has variants of its own for bo 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 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. OCR's `Provider` error keeps the whole body for Python's `get_error_class`, bounded by the connection's response size limit instead +Provider bodies are passed through intact for gateway and Python error mapping. `litellm_http::Error::upstream(status, body)` retains the complete body, but its `Display`, `Debug` and `Report` diagnostics show only the first 256 characters and mark the cut. Do not log the raw body or `RustUpstreamError` args. OCR's `Provider` error also keeps the whole body for Python's `get_error_class`, bounded by the connection's response size limit 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 diff --git a/litellm-rust/crates/gateway-inference/src/error.rs b/litellm-rust/crates/gateway-inference/src/error.rs index 525ccee825f..505483b426c 100644 --- a/litellm-rust/crates/gateway-inference/src/error.rs +++ b/litellm-rust/crates/gateway-inference/src/error.rs @@ -271,6 +271,26 @@ mod tests { assert_eq!(error.body(request_id), expected); } + #[rstest] + #[case::ascii("busy ".repeat(80))] + #[case::unicode("é".repeat(300))] + fn long_provider_errors_preserve_the_messages_envelope(#[case] message: String) { + let body = json!({ + "type": "error", + "error": {"type": "overloaded_error", "message": message}, + "request_id": "provider-request", + }); + let error = upstream(529, &body.to_string()); + let report = litellm_error::Report::new(&error).to_string(); + let diagnostic_body: String = body.to_string().chars().take(256).collect(); + assert_eq!(report.matches(&diagnostic_body).count(), 1, "{report}"); + assert!(report.ends_with("... (truncated)"), "{report}"); + assert!(!report.contains(&message), "{report}"); + assert!(!format!("{error:?}").contains(&message)); + assert_eq!(error.status(), StatusCode::from_u16(529).unwrap()); + assert_eq!(error.body(Some("gateway-request")), body); + } + #[rstest] #[case::bedrock_detail( upstream(403, r#"{"detail":{"message":"denied"}}"#), diff --git a/litellm-rust/crates/http/src/error.rs b/litellm-rust/crates/http/src/error.rs index 34668ad5466..29c8b988c0b 100644 --- a/litellm-rust/crates/http/src/error.rs +++ b/litellm-rust/crates/http/src/error.rs @@ -1,4 +1,4 @@ -use std::{io, path::PathBuf, sync::Arc}; +use std::{borrow::Cow, fmt, io, path::PathBuf, sync::Arc}; use tokio::time::error::Elapsed; use tokio_tungstenite::tungstenite; @@ -59,7 +59,7 @@ pub(crate) enum Repr { #[error("upstream request timed out")] Timeout(#[source] reqwest::Error), #[error("upstream request failed with status {status}: {body}")] - Status { status: u16, body: String }, + Status { status: u16, body: UpstreamBody }, #[error("invalid WebSocket URL")] WebSocketUrl(#[source] url::ParseError), @@ -138,7 +138,7 @@ impl Error { pub fn upstream(status: u16, body: String) -> Self { Repr::Status { status, - body: bounded(body), + body: UpstreamBody(body), } .into() } @@ -392,7 +392,7 @@ impl Error { pub fn upstream_response(&self) -> Option<(u16, &str)> { match &*self.0 { - Repr::Status { status, body } => Some((*status, body)), + Repr::Status { status, body } => Some((*status, &body.0)), Repr::Read { .. } | Repr::InvalidPem { .. } | Repr::Client(_) @@ -509,10 +509,24 @@ impl Error { } } -fn bounded(body: String) -> String { +pub(crate) struct UpstreamBody(String); + +impl fmt::Display for UpstreamBody { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt::Display::fmt(&bounded(&self.0), f) + } +} + +impl fmt::Debug for UpstreamBody { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt::Debug::fmt(&bounded(&self.0), f) + } +} + +fn bounded(body: &str) -> Cow<'_, str> { match body.char_indices().nth(UPSTREAM_BODY_MAX_CHARS) { - None => body, - Some((cut, _)) => format!("{}{TRUNCATION_MARKER}", &body[..cut]), + None => Cow::Borrowed(body), + Some((cut, _)) => Cow::Owned(format!("{}{TRUNCATION_MARKER}", &body[..cut])), } } @@ -634,17 +648,30 @@ mod tests { } #[rstest] + #[case::empty(String::new(), String::new())] #[case::short("slow down".to_owned(), "slow down".to_owned())] + #[case::ascii_over_the_limit( + format!("{}private-tail", "a".repeat(UPSTREAM_BODY_MAX_CHARS)), + format!("{}{TRUNCATION_MARKER}", "a".repeat(UPSTREAM_BODY_MAX_CHARS)) + )] #[case::at_the_limit("\u{00e9}".repeat(UPSTREAM_BODY_MAX_CHARS), "\u{00e9}".repeat(UPSTREAM_BODY_MAX_CHARS))] #[case::over_the_limit( - "\u{00e9}".repeat(UPSTREAM_BODY_MAX_CHARS + 1), + format!("{}private-tail", "\u{00e9}".repeat(UPSTREAM_BODY_MAX_CHARS)), format!("{}{TRUNCATION_MARKER}", "\u{00e9}".repeat(UPSTREAM_BODY_MAX_CHARS)) )] - fn upstream_bounds_the_provider_body_by_characters( + fn upstream_preserves_the_body_and_bounds_only_diagnostics( #[case] body: String, #[case] expected: String, ) { - let error = crate::Error::upstream(429, body); - assert_eq!(error.upstream_response(), Some((429, expected.as_str()))); + let error = crate::Error::upstream(429, body.clone()); + let expected_message = format!("upstream request failed with status 429: {expected}"); + assert_eq!(error.to_string(), expected_message); + assert_eq!(Report::new(&error).to_string(), expected_message); + let debug = format!("{error:?}"); + assert!(debug.contains(&format!("{expected:?}")), "{debug}"); + assert!(!debug.contains("private-tail"), "{debug}"); + assert!(!format!("{error:#?}").contains("private-tail")); + assert_eq!(error.upstream_response(), Some((429, body.as_str()))); + assert_eq!(error.status(), Some(429)); } } diff --git a/litellm-rust/crates/python-bridge/AGENTS.md b/litellm-rust/crates/python-bridge/AGENTS.md index c7a20b215dc..d90fe073b20 100644 --- a/litellm-rust/crates/python-bridge/AGENTS.md +++ b/litellm-rust/crates/python-bridge/AGENTS.md @@ -113,8 +113,9 @@ GIL handling to `litellm-host-python`. direct PyO3 conversion before expanding Rust coverage to image-heavy paths. - Render a Rust error for Python only in its mapping function, with `litellm_error::Report`, never `Debug` or a document's contents. An upstream - body reaches Python only as the `RustUpstreamError` args, bounded where - `litellm_http::Error::upstream` builds it. + body reaches Python intact as the `RustUpstreamError` args for provider error + mapping. The 256-character cap applies only to HTTP error diagnostics, not + response bodies. Do not log those args. ## Tests diff --git a/litellm-rust/crates/python-bridge/src/errors.rs b/litellm-rust/crates/python-bridge/src/errors.rs index 7d325266178..f17109ef509 100644 --- a/litellm-rust/crates/python-bridge/src/errors.rs +++ b/litellm-rust/crates/python-bridge/src/errors.rs @@ -179,6 +179,35 @@ mod tests { }); } + #[rstest::rstest] + #[case::ascii("busy ".repeat(80))] + #[case::unicode("é".repeat(300))] + fn long_provider_bodies_reach_python_intact( + #[case] message: String, + #[values(RequestMarker::Unmarked, RequestMarker::Messages)] marker: RequestMarker, + ) { + let envelope = serde_json::json!({ + "type": "error", + "error": {"type": "overloaded_error", "message": message}, + "request_id": "provider-request", + }); + let body = envelope.to_string(); + let error = litellm_http::Error::upstream(529, body.clone()).into(); + Python::initialize(); + Python::attach(|py| { + let upstream = route_failure(py, error, marker, Ok).unwrap(); + assert!(upstream.is_instance_of::(py)); + let ((status, returned_body), headers) = upstream_payload(py, &upstream); + assert_eq!(status, 529); + assert_eq!(returned_body, body); + assert_eq!( + serde_json::from_str::(&returned_body).unwrap(), + envelope + ); + assert!(headers.is_empty()); + }); + } + #[test] fn missing_api_key_stays_a_runtime_error_while_other_auth_failures_are_value_errors() { Python::initialize();