diff --git a/crates/libsy-llm-client/src/client.rs b/crates/libsy-llm-client/src/client.rs index 9f47828aa..234d3c8c6 100644 --- a/crates/libsy-llm-client/src/client.rs +++ b/crates/libsy-llm-client/src/client.rs @@ -136,11 +136,8 @@ impl TranslatingLlmClient { backend.validate_extra_headers(&config.model_name)?; } } - let build_client = |builder: reqwest::ClientBuilder| { - builder.build().map_err(|error| LlmClientError::Transport { - source: Box::new(error), - }) - }; + let build_client = + |builder: reqwest::ClientBuilder| builder.build().map_err(convert_reqwest_error); let client = build_client(reqwest::Client::builder())?; // A redirect could move provider-specific headers to another origin. // Forwarded credentials are sent only to the configured URL. @@ -464,17 +461,9 @@ impl TranslatingLlmClient { // Adapt the reqwest body stream to plain bytes; the SSE-decode itself is // transport-agnostic and lives in `switchyard-translation`. let bytes = http_response.bytes_stream().map(|chunk| { - chunk.map(|bytes| bytes.to_vec()).map_err(|error| { - if error.is_timeout() { - LlmClientError::Timeout { - source: Box::new(error), - } - } else { - LlmClientError::Transport { - source: Box::new(error), - } - } - }) + chunk + .map(|bytes| bytes.to_vec()) + .map_err(convert_reqwest_error) }); let mut chunks = decode_stream(bytes, wire_format)?; // Providers reject an over-ceiling streaming request with an in-band @@ -704,9 +693,25 @@ fn record_gen_ai_request(url: &str, model: &str, streaming: bool) { } } +// The upstream a request was sent to, without anything that can carry a credential. +fn redacted_endpoint(url: &reqwest::Url) -> String { + let mut url = url.clone(); + url.set_query(None); + url.set_fragment(None); + let _ = url.set_username(""); + let _ = url.set_password(None); + url.to_string() +} + fn convert_reqwest_error(error: reqwest::Error) -> LlmClientError { // Reqwest labels truncated or otherwise unreadable response bodies as decode // errors, so distinguish them from serde JSON failures at the call site. + // The url reaches callers, and a provider key can ride in it as a query + // parameter, so keep it to the log and drop it from the error. + if let Some(url) = error.url() { + tracing::warn!(upstream = %redacted_endpoint(url), "upstream request failed"); + } + let error = error.without_url(); if error.is_timeout() { LlmClientError::Timeout { source: Box::new(error), @@ -915,6 +920,22 @@ fn is_reserved_header(name: &str) -> bool { #[cfg(test)] mod tests { + + #[tokio::test] + async fn transport_errors_drop_the_upstream_url() { + let error = reqwest::Client::new() + .post("http://127.0.0.1:1/v1?key=CANARY") + .send() + .await + .expect_err("closed port"); + assert!(!convert_reqwest_error(error).to_string().contains("CANARY")); + } + + #[test] + fn redacted_endpoint_keeps_only_the_address() { + let url = reqwest::Url::parse("https://user:pw@host/v1/chat?key=CANARY#frag").unwrap(); + assert_eq!(redacted_endpoint(&url), "https://host/v1/chat"); + } use std::collections::BTreeMap; use std::error::Error; use std::io::{Read, Write};