diff --git a/crates/goose-provider-types/src/errors.rs b/crates/goose-provider-types/src/errors.rs index cbe038447..ed441738b 100644 --- a/crates/goose-provider-types/src/errors.rs +++ b/crates/goose-provider-types/src/errors.rs @@ -96,6 +96,31 @@ fn is_network_error(err: &reqwest::Error) -> bool { err.is_connect() || err.is_timeout() || (err.status().is_none() && err.is_request()) } +fn sanitized_reqwest_url(error: &reqwest::Error) -> Option { + let mut url = error.url()?.clone(); + let _ = url.set_password(None); + let _ = url.set_username(""); + url.set_query(None); + url.set_fragment(None); + Some(url.to_string()) +} + +fn reqwest_error_category(error: &reqwest::Error) -> &'static str { + if error.is_builder() { + "Request builder error" + } else if error.is_redirect() { + "Redirect error" + } else if error.is_status() { + "HTTP status error" + } else if error.is_body() { + "Request body error" + } else if error.is_decode() { + "Response decode error" + } else { + "Request error" + } +} + fn provider_error_from_reqwest(error: &reqwest::Error) -> ProviderError { if is_network_error(error) { let msg = if error.is_timeout() { @@ -121,14 +146,19 @@ fn provider_error_from_reqwest(error: &reqwest::Error) -> ProviderError { return ProviderError::NetworkError(msg); } - let mut details = vec![]; + let mut details = Vec::new(); if let Some(status) = error.status() { details.push(format!("status: {}", status)); } + if let Some(url) = sanitized_reqwest_url(error) { + details.push(format!("url: {url}")); + } + + let category = reqwest_error_category(error); let msg = if details.is_empty() { - error.to_string() + category.to_string() } else { - format!("{} ({})", error, details.join(", ")) + format!("{category} ({})", details.join(", ")) }; ProviderError::RequestFailed(msg) } @@ -172,6 +202,68 @@ impl From for ProviderError { } } +#[cfg(test)] +mod tests { + use super::*; + use std::io::{Read, Write}; + use std::net::TcpListener; + + #[tokio::test] + async fn direct_reqwest_error_redacts_unsupported_scheme_url() { + let error = reqwest::Client::new() + .get("ftp://url-user:url-password@provider.invalid/chat?api_key=query-secret") + .send() + .await + .unwrap_err(); + + assert!(error.to_string().contains("query-secret")); + assert!(!is_network_error(&error)); + + let message = ProviderError::from(error).to_string(); + assert!(!message.contains("query-secret")); + assert!(!message.contains("url-user")); + assert!(!message.contains("url-password")); + assert!(message.contains("ftp://provider.invalid/chat")); + } + + #[tokio::test] + async fn anyhow_wrapped_status_error_redacts_url() { + let listener = TcpListener::bind("127.0.0.1:0").unwrap(); + let address = listener.local_addr().unwrap(); + let server = std::thread::spawn(move || { + let (mut stream, _) = listener.accept().unwrap(); + let mut request = [0; 1024]; + assert!(stream.read(&mut request).unwrap() > 0); + stream + .write_all( + b"HTTP/1.1 401 Unauthorized\r\nContent-Length: 0\r\nConnection: close\r\n\r\n", + ) + .unwrap(); + }); + + let error = reqwest::Client::new() + .get(format!( + "http://url-user:url-password@{address}/chat?api_key=query-secret" + )) + .send() + .await + .unwrap() + .error_for_status() + .unwrap_err(); + server.join().unwrap(); + + assert!(error.to_string().contains("query-secret")); + assert_eq!(error.status(), Some(StatusCode::UNAUTHORIZED)); + + let message = ProviderError::from(anyhow::Error::new(error)).to_string(); + assert!(!message.contains("query-secret")); + assert!(!message.contains("url-user")); + assert!(!message.contains("url-password")); + assert!(message.contains("status: 401 Unauthorized")); + assert!(message.contains(&format!("http://{address}/chat"))); + } +} + #[derive(Debug)] pub enum GoogleErrorCode { BadRequest = 400,