Skip to content
Merged
33 changes: 22 additions & 11 deletions crates/goose/src/providers/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,17 @@ fn check_context_length_exceeded(text: &str) -> bool {
.any(|phrase| text_lower.contains(phrase))
}

fn format_server_error_message(status_code: u16, payload: Option<&Value>) -> String {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't status_code by of ype StatusCode here?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

eh, then I need to add .as_u16() inside the function. but guess that's negligible. will fix...

if let Some(p) = payload {
format!("HTTP {}: {}", status_code, p.to_string())
Comment thread
angiejones marked this conversation as resolved.
Outdated
} else {
format!(
"HTTP {}: No response body received from server",
status_code
)
}
}

pub fn map_http_error_to_provider_error(
status: StatusCode,
payload: Option<Value>,
Expand All @@ -79,7 +90,7 @@ pub fn map_http_error_to_provider_error(
"Authentication failed. Please ensure your API keys are valid and have the required permissions. \
Status: {}{}",
status,
payload.as_ref().map(|p| format!(". Response: {:?}", p)).unwrap_or_default()
payload.as_ref().map(|p| format!(". Response: {}", p.to_string())).unwrap_or_default()
Comment thread
angiejones marked this conversation as resolved.
Outdated
);
ProviderError::Authentication(message)
}
Expand Down Expand Up @@ -116,7 +127,10 @@ pub fn map_http_error_to_provider_error(
details: format!("{:?}", payload),
retry_delay: None,
},
_ if status.is_server_error() => ProviderError::ServerError(format!("{:?}", payload)),
_ if status.is_server_error() => ProviderError::ServerError(format_server_error_message(
status.as_u16(),
payload.as_ref(),
)),
_ => ProviderError::RequestFailed(format!("Request failed with status: {}", status)),
};

Expand Down Expand Up @@ -295,12 +309,9 @@ pub async fn handle_response_google_compat(response: Response) -> Result<Value,
retry_delay,
})
}
_ if final_status.is_server_error() => {
Err(ProviderError::ServerError(format!("{:?}", payload)))
}
StatusCode::INTERNAL_SERVER_ERROR | StatusCode::SERVICE_UNAVAILABLE => {
Err(ProviderError::ServerError(format!("{:?}", payload)))
}
_ if final_status.is_server_error() => Err(ProviderError::ServerError(
format_server_error_message(final_status.as_u16(), payload.as_ref()),
)),
_ => {
tracing::debug!(
"{}", format!("Provider request failed with status: {}. Payload: {:?}", final_status, payload)
Expand Down Expand Up @@ -1064,7 +1075,7 @@ mod tests {
StatusCode::UNAUTHORIZED,
Some(json!({"error": "auth failed"})),
ProviderError::Authentication(
"Authentication failed. Please ensure your API keys are valid and have the required permissions. Status: 401 Unauthorized. Response: Object {\"error\": String(\"auth failed\")}".to_string(),
"Authentication failed. Please ensure your API keys are valid and have the required permissions. Status: 401 Unauthorized. Response: {\"error\":\"auth failed\"}".to_string(),
),
),
// UNAUTHORIZED/FORBIDDEN - without payload
Expand Down Expand Up @@ -1112,13 +1123,13 @@ mod tests {
(
StatusCode::INTERNAL_SERVER_ERROR,
None,
ProviderError::ServerError("None".to_string()),
ProviderError::ServerError("HTTP 500: No response body received from server".to_string()),
Comment thread
angiejones marked this conversation as resolved.
Outdated
),
// is_server_error() with payload
(
StatusCode::BAD_GATEWAY,
Some(json!({"error": "upstream error"})),
ProviderError::ServerError("Some(Object {\"error\": String(\"upstream error\")})".to_string()),
ProviderError::ServerError("HTTP 502: {\"error\":\"upstream error\"}".to_string()),
Comment thread
angiejones marked this conversation as resolved.
Outdated
),
// Default - any other status code
(
Expand Down
Loading