mirror of
https://github.com/block/goose.git
synced 2026-07-03 14:15:10 +02:00
Include request URL in provider error messages (#9232)
Signed-off-by: Douwe Osinga <douwe@squareup.com> Co-authored-by: Douwe Osinga <douwe@squareup.com>
This commit is contained in:
@@ -208,6 +208,7 @@ impl AnthropicProvider {
|
||||
return Err(map_http_error_to_provider_error(
|
||||
response.status,
|
||||
response.payload,
|
||||
"v1/models",
|
||||
));
|
||||
}
|
||||
|
||||
|
||||
@@ -19,7 +19,7 @@ use super::formats::databricks::create_request;
|
||||
use super::formats::openai_responses::create_responses_request;
|
||||
use super::oauth;
|
||||
use super::openai_compatible::{
|
||||
handle_response_openai_compat, handle_status, map_http_error_to_provider_error,
|
||||
handle_response_openai_compat, handle_status, map_http_error_to_provider_error, sanitize_url,
|
||||
stream_openai_compat, stream_responses_compat,
|
||||
};
|
||||
use super::retry::ProviderRetry;
|
||||
@@ -775,10 +775,11 @@ impl Provider for DatabricksProvider {
|
||||
.await?;
|
||||
if !resp.status().is_success() {
|
||||
let status = resp.status();
|
||||
let url = sanitize_url(resp.url().as_str());
|
||||
let error_text = resp.text().await.unwrap_or_default();
|
||||
|
||||
let json_payload = serde_json::from_str::<Value>(&error_text).ok();
|
||||
return Err(map_http_error_to_provider_error(status, json_payload));
|
||||
return Err(map_http_error_to_provider_error(status, json_payload, &url));
|
||||
}
|
||||
Ok(resp)
|
||||
})
|
||||
@@ -794,9 +795,14 @@ impl Provider for DatabricksProvider {
|
||||
.await?;
|
||||
if !resp.status().is_success() {
|
||||
let status = resp.status();
|
||||
let url = sanitize_url(resp.url().as_str());
|
||||
let error_text = resp.text().await.unwrap_or_default();
|
||||
let json_payload = serde_json::from_str::<Value>(&error_text).ok();
|
||||
return Err(map_http_error_to_provider_error(status, json_payload));
|
||||
return Err(map_http_error_to_provider_error(
|
||||
status,
|
||||
json_payload,
|
||||
&url,
|
||||
));
|
||||
}
|
||||
Ok(resp)
|
||||
})
|
||||
|
||||
@@ -27,7 +27,7 @@ use crate::providers::formats::gcpvertexai::{
|
||||
DEFAULT_MODEL, KNOWN_MODELS,
|
||||
};
|
||||
use crate::providers::gcpauth::GcpAuth;
|
||||
use crate::providers::openai_compatible::map_http_error_to_provider_error;
|
||||
use crate::providers::openai_compatible::{map_http_error_to_provider_error, sanitize_url};
|
||||
use crate::providers::retry::RetryConfig;
|
||||
use crate::providers::utils::RequestLog;
|
||||
use crate::session_context::SESSION_ID_HEADER;
|
||||
@@ -359,9 +359,10 @@ impl GcpVertexAIProvider {
|
||||
"Authentication failed with status: {status}"
|
||||
)));
|
||||
} else {
|
||||
let url = sanitize_url(response.url().as_str());
|
||||
let response_text = response.text().await.unwrap_or_default();
|
||||
let payload = serde_json::from_str::<Value>(&response_text).ok();
|
||||
return Err(map_http_error_to_provider_error(status, payload));
|
||||
return Err(map_http_error_to_provider_error(status, payload, &url));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
use super::api_client::{ApiClient, AuthMethod};
|
||||
use super::base::MessageStream;
|
||||
use super::errors::ProviderError;
|
||||
use super::openai_compatible::{handle_status, map_http_error_to_provider_error};
|
||||
use super::openai_compatible::{handle_status, map_http_error_to_provider_error, sanitize_url};
|
||||
use super::retry::ProviderRetry;
|
||||
use super::utils::RequestLog;
|
||||
use crate::conversation::message::Message;
|
||||
@@ -177,9 +177,10 @@ impl Provider for GoogleProvider {
|
||||
.await?;
|
||||
let status = response.status();
|
||||
if !status.is_success() {
|
||||
let url = sanitize_url(response.url().as_str());
|
||||
let body = response.text().await.unwrap_or_default();
|
||||
let payload = serde_json::from_str::<serde_json::Value>(&body).ok();
|
||||
return Err(map_http_error_to_provider_error(status, payload));
|
||||
return Err(map_http_error_to_provider_error(status, payload, &url));
|
||||
}
|
||||
|
||||
let json: serde_json::Value = response.json().await?;
|
||||
|
||||
@@ -13,6 +13,23 @@ use serde_json::Value;
|
||||
|
||||
use super::errors::ProviderError;
|
||||
|
||||
/// Strip credentials and sensitive query parameters from a URL for safe
|
||||
/// inclusion in error messages and logs. Drops userinfo (`user:pass@`) and
|
||||
/// all query parameters (which may contain API keys like `?key=...`).
|
||||
/// Returns the original string unchanged if it doesn't parse as a URL
|
||||
/// (e.g. a bare path like "v1/models").
|
||||
pub fn sanitize_url(raw: &str) -> String {
|
||||
let Ok(mut url) = url::Url::parse(raw) else {
|
||||
return raw.to_string();
|
||||
};
|
||||
if !url.username().is_empty() || url.password().is_some() {
|
||||
let _ = url.set_username("");
|
||||
let _ = url.set_password(None);
|
||||
}
|
||||
url.set_query(None);
|
||||
url.to_string()
|
||||
}
|
||||
|
||||
/// Hard cap on retry delays we'll honor from remote responses. A malformed
|
||||
/// 429 with `retry_after_seconds: 1e30` (or a far-future HTTP-date) should
|
||||
/// degrade to "no retry hint" rather than freeze the agent or panic when
|
||||
@@ -115,6 +132,7 @@ fn check_context_length_exceeded(text: &str) -> bool {
|
||||
pub fn map_http_error_to_provider_error(
|
||||
status: StatusCode,
|
||||
payload: Option<Value>,
|
||||
url: &str,
|
||||
) -> ProviderError {
|
||||
let extract_message = || -> String {
|
||||
payload
|
||||
@@ -132,13 +150,14 @@ pub fn map_http_error_to_provider_error(
|
||||
let error = match status {
|
||||
StatusCode::OK => unreachable!("Should not call this function with OK status"),
|
||||
StatusCode::UNAUTHORIZED | StatusCode::FORBIDDEN => ProviderError::Authentication(format!(
|
||||
"Authentication failed. Status: {}. Response: {}",
|
||||
"Authentication failed for {url}. Status: {}. Response: {}",
|
||||
status,
|
||||
extract_message()
|
||||
)),
|
||||
StatusCode::NOT_FOUND => {
|
||||
ProviderError::RequestFailed(format!("Resource not found (404): {}", extract_message()))
|
||||
}
|
||||
StatusCode::NOT_FOUND => ProviderError::RequestFailed(format!(
|
||||
"Resource not found (404) at {url}: {}",
|
||||
extract_message()
|
||||
)),
|
||||
StatusCode::PAYMENT_REQUIRED => ProviderError::CreditsExhausted {
|
||||
details: extract_message(),
|
||||
top_up_url: None,
|
||||
@@ -156,11 +175,13 @@ pub fn map_http_error_to_provider_error(
|
||||
details: extract_message(),
|
||||
retry_delay: None,
|
||||
},
|
||||
_ if status.is_server_error() => {
|
||||
ProviderError::ServerError(format!("Server error ({}): {}", status, extract_message()))
|
||||
}
|
||||
_ if status.is_server_error() => ProviderError::ServerError(format!(
|
||||
"Server error ({}) at {url}: {}",
|
||||
status,
|
||||
extract_message()
|
||||
)),
|
||||
_ => ProviderError::RequestFailed(format!(
|
||||
"Request failed with status {}: {}",
|
||||
"Request failed with status {} at {url}: {}",
|
||||
status,
|
||||
extract_message()
|
||||
)),
|
||||
@@ -181,10 +202,11 @@ pub fn map_http_error_to_provider_error(
|
||||
pub async fn handle_status(response: Response) -> Result<Response, ProviderError> {
|
||||
let status = response.status();
|
||||
if !status.is_success() {
|
||||
let url = sanitize_url(response.url().as_str());
|
||||
let headers = response.headers().clone();
|
||||
let body = response.text().await.unwrap_or_default();
|
||||
let payload = serde_json::from_str::<Value>(&body).ok();
|
||||
let mut err = map_http_error_to_provider_error(status, payload.clone());
|
||||
let mut err = map_http_error_to_provider_error(status, payload.clone(), &url);
|
||||
if let ProviderError::RateLimitExceeded { details, .. } = &err {
|
||||
err = ProviderError::RateLimitExceeded {
|
||||
details: details.clone(),
|
||||
|
||||
@@ -133,7 +133,9 @@ impl Provider for OpenAiCompatibleProvider {
|
||||
|
||||
// Re-exported from the dedicated `http_status` module — these helpers are
|
||||
// format-agnostic and used across all provider families.
|
||||
pub use super::http_status::{handle_response, handle_status, map_http_error_to_provider_error};
|
||||
pub use super::http_status::{
|
||||
handle_response, handle_status, map_http_error_to_provider_error, sanitize_url,
|
||||
};
|
||||
|
||||
// Legacy alias kept for callers that haven't migrated their import path yet.
|
||||
pub use super::http_status::handle_response as handle_response_openai_compat;
|
||||
@@ -244,7 +246,7 @@ mod tests {
|
||||
payload: Option<Value>,
|
||||
expected_variant: &str,
|
||||
) {
|
||||
let err = map_http_error_to_provider_error(status, payload);
|
||||
let err = map_http_error_to_provider_error(status, payload, "http://test/endpoint");
|
||||
let actual = err.telemetry_type();
|
||||
let expected_telemetry = match expected_variant {
|
||||
"CreditsExhausted" => "credits_exhausted",
|
||||
|
||||
@@ -9,7 +9,7 @@ use super::base::{
|
||||
};
|
||||
use super::errors::ProviderError;
|
||||
use super::formats::snowflake::{create_request, get_usage, response_to_message};
|
||||
use super::openai_compatible::map_http_error_to_provider_error;
|
||||
use super::openai_compatible::{map_http_error_to_provider_error, sanitize_url};
|
||||
use super::retry::ProviderRetry;
|
||||
use super::utils::{get_model, ImageFormat, RequestLog};
|
||||
use crate::config::ConfigError;
|
||||
@@ -123,6 +123,7 @@ impl SnowflakeProvider {
|
||||
.await?;
|
||||
|
||||
let status = response.status();
|
||||
let url = sanitize_url(response.url().as_str());
|
||||
let payload_text: String = response.text().await.ok().unwrap_or_default();
|
||||
|
||||
if status.is_success() {
|
||||
@@ -292,7 +293,7 @@ impl SnowflakeProvider {
|
||||
Ok(answer_payload)
|
||||
} else {
|
||||
let error_json = serde_json::from_str::<Value>(&payload_text).ok();
|
||||
Err(map_http_error_to_provider_error(status, error_json))
|
||||
Err(map_http_error_to_provider_error(status, error_json, &url))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -76,7 +76,7 @@ impl TetrateProvider {
|
||||
}
|
||||
}
|
||||
|
||||
fn error_from_tetrate_error_payload(payload: Value) -> ProviderError {
|
||||
fn error_from_tetrate_error_payload(payload: Value, url: &str) -> ProviderError {
|
||||
let code = payload
|
||||
.get("error")
|
||||
.and_then(|e| e.get("code"))
|
||||
@@ -84,7 +84,7 @@ impl TetrateProvider {
|
||||
.unwrap_or(500) as u16;
|
||||
let status = reqwest::StatusCode::from_u16(code)
|
||||
.unwrap_or(reqwest::StatusCode::INTERNAL_SERVER_ERROR);
|
||||
Self::enrich_credits_error(map_http_error_to_provider_error(status, Some(payload)))
|
||||
Self::enrich_credits_error(map_http_error_to_provider_error(status, Some(payload), url))
|
||||
}
|
||||
}
|
||||
|
||||
@@ -173,7 +173,10 @@ impl Provider for TetrateProvider {
|
||||
.await
|
||||
.map_err(Self::enrich_credits_error)?;
|
||||
if body.get("error").is_some() {
|
||||
return Err(Self::error_from_tetrate_error_payload(body));
|
||||
return Err(Self::error_from_tetrate_error_payload(
|
||||
body,
|
||||
"v1/chat/completions",
|
||||
));
|
||||
}
|
||||
|
||||
return Err(ProviderError::ExecutionError(
|
||||
@@ -203,7 +206,7 @@ impl Provider for TetrateProvider {
|
||||
|
||||
// Tetrate can return errors in 200 OK responses, so check explicitly
|
||||
if json.get("error").is_some() {
|
||||
return Err(Self::error_from_tetrate_error_payload(json));
|
||||
return Err(Self::error_from_tetrate_error_payload(json, "v1/models"));
|
||||
}
|
||||
|
||||
let arr = json.get("data").and_then(|v| v.as_array()).ok_or_else(|| {
|
||||
@@ -257,7 +260,7 @@ mod tests {
|
||||
"message": "Insufficient credits"
|
||||
}
|
||||
});
|
||||
match TetrateProvider::error_from_tetrate_error_payload(payload) {
|
||||
match TetrateProvider::error_from_tetrate_error_payload(payload, "test") {
|
||||
ProviderError::CreditsExhausted {
|
||||
details,
|
||||
top_up_url,
|
||||
@@ -277,7 +280,7 @@ mod tests {
|
||||
"message": "Invalid API key"
|
||||
}
|
||||
});
|
||||
match TetrateProvider::error_from_tetrate_error_payload(payload) {
|
||||
match TetrateProvider::error_from_tetrate_error_payload(payload, "test") {
|
||||
ProviderError::Authentication(msg) => {
|
||||
assert!(msg.contains("Invalid API key"));
|
||||
}
|
||||
|
||||
@@ -149,13 +149,14 @@ fn parse_google_retry_delay(payload: &Value) -> Option<Duration> {
|
||||
/// - `Err(ProviderError)`: Describes the failure reason.
|
||||
pub async fn handle_response_google_compat(response: Response) -> Result<Value, ProviderError> {
|
||||
let status = response.status();
|
||||
let url = super::http_status::sanitize_url(response.url().as_str());
|
||||
let payload: Option<Value> = response.json().await.ok();
|
||||
let final_status = get_google_final_status(status, payload.as_ref());
|
||||
|
||||
match final_status {
|
||||
StatusCode::OK => payload.ok_or_else( || ProviderError::RequestFailed("Response body is not valid JSON".to_string()) ),
|
||||
StatusCode::UNAUTHORIZED | StatusCode::FORBIDDEN => {
|
||||
Err(ProviderError::Authentication(format!("Authentication failed. Please ensure your API keys are valid and have the required permissions. \
|
||||
Err(ProviderError::Authentication(format!("Authentication failed for {url}. Please ensure your API keys are valid and have the required permissions. \
|
||||
Status: {}. Response: {:?}", final_status, payload )))
|
||||
}
|
||||
StatusCode::BAD_REQUEST | StatusCode::NOT_FOUND => {
|
||||
@@ -172,7 +173,7 @@ pub async fn handle_response_google_compat(response: Response) -> Result<Value,
|
||||
tracing::debug!(
|
||||
"{}", format!("Provider request failed with status: {}. Payload: {:?}", final_status, payload)
|
||||
);
|
||||
Err(ProviderError::RequestFailed(format!("Request failed with status: {}. Message: {}", final_status, error_msg)))
|
||||
Err(ProviderError::RequestFailed(format!("Request failed with status {} at {url}. Message: {}", final_status, error_msg)))
|
||||
}
|
||||
StatusCode::TOO_MANY_REQUESTS => {
|
||||
let retry_delay = payload.as_ref().and_then(parse_google_retry_delay);
|
||||
@@ -182,13 +183,13 @@ pub async fn handle_response_google_compat(response: Response) -> Result<Value,
|
||||
})
|
||||
}
|
||||
_ if final_status.is_server_error() => Err(ProviderError::ServerError(
|
||||
format_server_error_message(final_status, payload.as_ref()),
|
||||
format!("Server error ({}) at {url}: {}", final_status, format_server_error_message(final_status, payload.as_ref())),
|
||||
)),
|
||||
_ => {
|
||||
tracing::debug!(
|
||||
"{}", format!("Provider request failed with status: {}. Payload: {:?}", final_status, payload)
|
||||
);
|
||||
Err(ProviderError::RequestFailed(format!("Request failed with status: {}", final_status)))
|
||||
Err(ProviderError::RequestFailed(format!("Request failed with status {} at {url}", final_status)))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user