mirror of
https://github.com/BerriAI/litellm.git
synced 2026-11-05 07:37:50 +00:00
fix(rust): preserve upstream bodies and bound diagnostics only
This commit is contained in:
parent
015f21dfd9
commit
f55942f7e5
5 changed files with 91 additions and 14 deletions
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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"}}"#),
|
||||
|
|
|
|||
|
|
@ -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));
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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::<RustUpstreamError>(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::<serde_json::Value>(&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();
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue