From 762a353c043003dbd8a60459c6f8ed2c4c4ea311 Mon Sep 17 00:00:00 2001 From: angiejones Date: Sun, 2 Nov 2025 13:41:53 -0600 Subject: [PATCH 01/11] fix: improve server error messages to include HTTP status code Fixes #5528 Previously, when a server error (5xx) occurred without a response body, the error message would display 'Server error: None', which was not helpful for debugging. This change improves error messages by: - Including the HTTP status code (e.g., 'HTTP 500') - Providing a clear message when no response body is received - Maintaining detailed error info when a response body is present Example error messages: - Before: 'Server error: None' - After: 'HTTP 500: No response body received from server' This affects all OpenAI-compatible providers (OpenAI, Claude, Google, Ollama, Azure, Databricks, GitHub Copilot, etc.) and Google-compatible endpoints. --- crates/goose/src/providers/utils.rs | 30 ++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/crates/goose/src/providers/utils.rs b/crates/goose/src/providers/utils.rs index 777750415531..ff346685d833 100644 --- a/crates/goose/src/providers/utils.rs +++ b/crates/goose/src/providers/utils.rs @@ -116,7 +116,17 @@ 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() => { + let msg = if let Some(p) = &payload { + format!("HTTP {}: {:?}", status.as_u16(), p) + } else { + format!( + "HTTP {}: No response body received from server", + status.as_u16() + ) + }; + ProviderError::ServerError(msg) + } _ => ProviderError::RequestFailed(format!("Request failed with status: {}", status)), }; @@ -296,10 +306,20 @@ pub async fn handle_response_google_compat(response: Response) -> Result { - Err(ProviderError::ServerError(format!("{:?}", payload))) + let msg = if let Some(p) = &payload { + format!("HTTP {}: {:?}", final_status.as_u16(), p) + } else { + format!("HTTP {}: No response body received from server", final_status.as_u16()) + }; + Err(ProviderError::ServerError(msg)) } StatusCode::INTERNAL_SERVER_ERROR | StatusCode::SERVICE_UNAVAILABLE => { - Err(ProviderError::ServerError(format!("{:?}", payload))) + let msg = if let Some(p) = &payload { + format!("HTTP {}: {:?}", final_status.as_u16(), p) + } else { + format!("HTTP {}: No response body received from server", final_status.as_u16()) + }; + Err(ProviderError::ServerError(msg)) } _ => { tracing::debug!( @@ -1112,13 +1132,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()), ), // 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: Object {\"error\": String(\"upstream error\")}".to_string()), ), // Default - any other status code ( From fa045d0b890670bd08b5334131e7758482053045 Mon Sep 17 00:00:00 2001 From: angiejones Date: Sun, 2 Nov 2025 13:59:39 -0600 Subject: [PATCH 02/11] removing unreachable code --- crates/goose/src/providers/utils.rs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/crates/goose/src/providers/utils.rs b/crates/goose/src/providers/utils.rs index ff346685d833..5824e3722058 100644 --- a/crates/goose/src/providers/utils.rs +++ b/crates/goose/src/providers/utils.rs @@ -313,14 +313,6 @@ pub async fn handle_response_google_compat(response: Response) -> Result { - let msg = if let Some(p) = &payload { - format!("HTTP {}: {:?}", final_status.as_u16(), p) - } else { - format!("HTTP {}: No response body received from server", final_status.as_u16()) - }; - Err(ProviderError::ServerError(msg)) - } _ => { tracing::debug!( "{}", format!("Provider request failed with status: {}. Payload: {:?}", final_status, payload) From 3cb5037e29eff667fab47520e661148db88b825b Mon Sep 17 00:00:00 2001 From: Angie Jones Date: Sun, 2 Nov 2025 14:07:48 -0600 Subject: [PATCH 03/11] Apply suggestion from @Copilot Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- crates/goose/src/providers/utils.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/goose/src/providers/utils.rs b/crates/goose/src/providers/utils.rs index 5824e3722058..4c3b3b547ad1 100644 --- a/crates/goose/src/providers/utils.rs +++ b/crates/goose/src/providers/utils.rs @@ -309,7 +309,7 @@ pub async fn handle_response_google_compat(response: Response) -> Result Date: Sun, 2 Nov 2025 14:14:18 -0600 Subject: [PATCH 04/11] Update crates/goose/src/providers/utils.rs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- crates/goose/src/providers/utils.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/goose/src/providers/utils.rs b/crates/goose/src/providers/utils.rs index 4c3b3b547ad1..5824e3722058 100644 --- a/crates/goose/src/providers/utils.rs +++ b/crates/goose/src/providers/utils.rs @@ -309,7 +309,7 @@ pub async fn handle_response_google_compat(response: Response) -> Result Date: Sun, 2 Nov 2025 14:30:55 -0600 Subject: [PATCH 05/11] adding function to eliminate duplicate code --- crates/goose/src/providers/utils.rs | 37 ++++++++++++++--------------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/crates/goose/src/providers/utils.rs b/crates/goose/src/providers/utils.rs index 5824e3722058..2d907742a87b 100644 --- a/crates/goose/src/providers/utils.rs +++ b/crates/goose/src/providers/utils.rs @@ -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 { + if let Some(p) = payload { + format!("HTTP {}: {:?}", status_code, p) + } else { + format!( + "HTTP {}: No response body received from server", + status_code + ) + } +} + pub fn map_http_error_to_provider_error( status: StatusCode, payload: Option, @@ -116,17 +127,10 @@ pub fn map_http_error_to_provider_error( details: format!("{:?}", payload), retry_delay: None, }, - _ if status.is_server_error() => { - let msg = if let Some(p) = &payload { - format!("HTTP {}: {:?}", status.as_u16(), p) - } else { - format!( - "HTTP {}: No response body received from server", - status.as_u16() - ) - }; - ProviderError::ServerError(msg) - } + _ 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)), }; @@ -305,14 +309,9 @@ pub async fn handle_response_google_compat(response: Response) -> Result { - let msg = if let Some(p) = &payload { - format!("HTTP {}: {:?}", final_status.as_u16(), p) - } else { - format!("HTTP {}: No response body received from server", final_status.as_u16()) - }; - Err(ProviderError::ServerError(msg)) - } + _ 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) From 356b2ad1c2041cc1de9d33e1237d6b8d166a76ae Mon Sep 17 00:00:00 2001 From: angiejones Date: Sun, 2 Nov 2025 15:06:44 -0600 Subject: [PATCH 06/11] to_string > Object --- crates/goose/src/providers/utils.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/goose/src/providers/utils.rs b/crates/goose/src/providers/utils.rs index 2d907742a87b..f06d42e8254c 100644 --- a/crates/goose/src/providers/utils.rs +++ b/crates/goose/src/providers/utils.rs @@ -70,7 +70,7 @@ fn check_context_length_exceeded(text: &str) -> bool { fn format_server_error_message(status_code: u16, payload: Option<&Value>) -> String { if let Some(p) = payload { - format!("HTTP {}: {:?}", status_code, p) + format!("HTTP {}: {}", status_code, p.to_string()) } else { format!( "HTTP {}: No response body received from server", @@ -90,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() ); ProviderError::Authentication(message) } @@ -1075,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 @@ -1129,7 +1129,7 @@ mod tests { ( StatusCode::BAD_GATEWAY, Some(json!({"error": "upstream error"})), - ProviderError::ServerError("HTTP 502: Object {\"error\": String(\"upstream error\")}".to_string()), + ProviderError::ServerError("HTTP 502: {\"error\":\"upstream error\"}".to_string()), ), // Default - any other status code ( From 54ed5e042e07aa47aff574e374dd44091cd7d290 Mon Sep 17 00:00:00 2001 From: angiejones Date: Sun, 2 Nov 2025 15:20:20 -0600 Subject: [PATCH 07/11] copilot pmo --- crates/goose/src/providers/utils.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/goose/src/providers/utils.rs b/crates/goose/src/providers/utils.rs index f06d42e8254c..950b0a404c7a 100644 --- a/crates/goose/src/providers/utils.rs +++ b/crates/goose/src/providers/utils.rs @@ -70,7 +70,7 @@ fn check_context_length_exceeded(text: &str) -> bool { fn format_server_error_message(status_code: u16, payload: Option<&Value>) -> String { if let Some(p) = payload { - format!("HTTP {}: {}", status_code, p.to_string()) + format!("HTTP {}: {}", status_code, p) } else { format!( "HTTP {}: No response body received from server", @@ -90,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.to_string())).unwrap_or_default() + payload.as_ref().map(|p| format!(". Response: {}", p)).unwrap_or_default() ); ProviderError::Authentication(message) } From 43d6394cd001ab05e3647d877559ef276892b352 Mon Sep 17 00:00:00 2001 From: angiejones Date: Sun, 2 Nov 2025 16:08:15 -0600 Subject: [PATCH 08/11] fix: handle null payload in server error messages to prevent 'Server error: None' When providers return 500 errors with null response bodies, the error formatting was displaying 'Server error: null' which appeared as 'Server error: None' to users. This was confusing and unhelpful. This fix treats Value::Null the same as None, displaying the more informative message 'HTTP 500: No response body received from server' instead. Fixes #5528 --- crates/goose/src/providers/utils.rs | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/crates/goose/src/providers/utils.rs b/crates/goose/src/providers/utils.rs index 950b0a404c7a..e0b34b64d5f6 100644 --- a/crates/goose/src/providers/utils.rs +++ b/crates/goose/src/providers/utils.rs @@ -69,13 +69,12 @@ fn check_context_length_exceeded(text: &str) -> bool { } fn format_server_error_message(status_code: u16, payload: Option<&Value>) -> String { - if let Some(p) = payload { - format!("HTTP {}: {}", status_code, p) - } else { - format!( + match payload { + Some(Value::Null) | None => format!( "HTTP {}: No response body received from server", status_code - ) + ), + Some(p) => format!("HTTP {}: {}", status_code, p), } } @@ -1125,6 +1124,12 @@ mod tests { None, ProviderError::ServerError("HTTP 500: No response body received from server".to_string()), ), + // is_server_error() with null payload (the "Server error: None" bug) + ( + StatusCode::INTERNAL_SERVER_ERROR, + Some(Value::Null), + ProviderError::ServerError("HTTP 500: No response body received from server".to_string()), + ), // is_server_error() with payload ( StatusCode::BAD_GATEWAY, From 12f6fe2ea5d8030e0cc19913f5eb9dc97cc1bab3 Mon Sep 17 00:00:00 2001 From: angiejones Date: Tue, 4 Nov 2025 11:10:53 -0600 Subject: [PATCH 09/11] refactor: use format_server_error_message helper in test expectations Use the format_server_error_message helper function in test cases instead of hardcoding error message strings. This improves maintainability by ensuring error message format changes only need to be updated in one place. Addresses code review feedback from DOsinga. --- crates/goose/src/providers/utils.rs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/crates/goose/src/providers/utils.rs b/crates/goose/src/providers/utils.rs index e0b34b64d5f6..c39d7433a1e0 100644 --- a/crates/goose/src/providers/utils.rs +++ b/crates/goose/src/providers/utils.rs @@ -1122,19 +1122,28 @@ mod tests { ( StatusCode::INTERNAL_SERVER_ERROR, None, - ProviderError::ServerError("HTTP 500: No response body received from server".to_string()), + ProviderError::ServerError(format_server_error_message( + StatusCode::INTERNAL_SERVER_ERROR.as_u16(), + None, + )), ), // is_server_error() with null payload (the "Server error: None" bug) ( StatusCode::INTERNAL_SERVER_ERROR, Some(Value::Null), - ProviderError::ServerError("HTTP 500: No response body received from server".to_string()), + ProviderError::ServerError(format_server_error_message( + StatusCode::INTERNAL_SERVER_ERROR.as_u16(), + Some(&Value::Null), + )), ), // is_server_error() with payload ( StatusCode::BAD_GATEWAY, Some(json!({"error": "upstream error"})), - ProviderError::ServerError("HTTP 502: {\"error\":\"upstream error\"}".to_string()), + ProviderError::ServerError(format_server_error_message( + StatusCode::BAD_GATEWAY.as_u16(), + Some(&json!({"error": "upstream error"})), + )), ), // Default - any other status code ( From 539073bb997e4462a47315ec446ebfdb002ee224 Mon Sep 17 00:00:00 2001 From: angiejones Date: Tue, 4 Nov 2025 11:19:24 -0600 Subject: [PATCH 10/11] refactor: change format_server_error_message to accept StatusCode instead of u16 - Changed parameter type from u16 to StatusCode for better type safety - Updated all call sites to pass StatusCode directly - Updated test expectations to use the helper function instead of hardcoded strings - Improves maintainability by keeping error format in one place Addresses code review feedback from DOsinga on PR #5532. --- crates/goose/src/providers/utils.rs | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/crates/goose/src/providers/utils.rs b/crates/goose/src/providers/utils.rs index c39d7433a1e0..0779dcab016c 100644 --- a/crates/goose/src/providers/utils.rs +++ b/crates/goose/src/providers/utils.rs @@ -68,13 +68,13 @@ 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 { +fn format_server_error_message(status_code: StatusCode, payload: Option<&Value>) -> String { match payload { Some(Value::Null) | None => format!( "HTTP {}: No response body received from server", - status_code + status_code.as_u16() ), - Some(p) => format!("HTTP {}: {}", status_code, p), + Some(p) => format!("HTTP {}: {}", status_code.as_u16(), p), } } @@ -126,10 +126,9 @@ pub fn map_http_error_to_provider_error( details: format!("{:?}", payload), retry_delay: None, }, - _ if status.is_server_error() => ProviderError::ServerError(format_server_error_message( - status.as_u16(), - payload.as_ref(), - )), + _ if status.is_server_error() => { + ProviderError::ServerError(format_server_error_message(status, payload.as_ref())) + } _ => ProviderError::RequestFailed(format!("Request failed with status: {}", status)), }; @@ -309,7 +308,7 @@ pub async fn handle_response_google_compat(response: Response) -> Result Err(ProviderError::ServerError( - format_server_error_message(final_status.as_u16(), payload.as_ref()), + format_server_error_message(final_status, payload.as_ref()), )), _ => { tracing::debug!( @@ -1123,7 +1122,7 @@ mod tests { StatusCode::INTERNAL_SERVER_ERROR, None, ProviderError::ServerError(format_server_error_message( - StatusCode::INTERNAL_SERVER_ERROR.as_u16(), + StatusCode::INTERNAL_SERVER_ERROR, None, )), ), @@ -1132,7 +1131,7 @@ mod tests { StatusCode::INTERNAL_SERVER_ERROR, Some(Value::Null), ProviderError::ServerError(format_server_error_message( - StatusCode::INTERNAL_SERVER_ERROR.as_u16(), + StatusCode::INTERNAL_SERVER_ERROR, Some(&Value::Null), )), ), @@ -1141,7 +1140,7 @@ mod tests { StatusCode::BAD_GATEWAY, Some(json!({"error": "upstream error"})), ProviderError::ServerError(format_server_error_message( - StatusCode::BAD_GATEWAY.as_u16(), + StatusCode::BAD_GATEWAY, Some(&json!({"error": "upstream error"})), )), ), From 2103b218a06c531d6006d4c2873f958c717809b1 Mon Sep 17 00:00:00 2001 From: angiejones Date: Tue, 4 Nov 2025 11:25:09 -0600 Subject: [PATCH 11/11] removing self-explanatory comments --- crates/goose/src/providers/utils.rs | 9 --------- 1 file changed, 9 deletions(-) diff --git a/crates/goose/src/providers/utils.rs b/crates/goose/src/providers/utils.rs index 0779dcab016c..c227b4ddf1c7 100644 --- a/crates/goose/src/providers/utils.rs +++ b/crates/goose/src/providers/utils.rs @@ -1068,7 +1068,6 @@ mod tests { #[test] fn test_map_http_error_to_provider_error() { let test_cases = vec![ - // UNAUTHORIZED/FORBIDDEN - with payload ( StatusCode::UNAUTHORIZED, Some(json!({"error": "auth failed"})), @@ -1076,7 +1075,6 @@ mod tests { "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 ( StatusCode::FORBIDDEN, None, @@ -1084,7 +1082,6 @@ mod tests { "Authentication failed. Please ensure your API keys are valid and have the required permissions. Status: 403 Forbidden".to_string(), ), ), - // BAD_REQUEST - with context_length_exceeded detection ( StatusCode::BAD_REQUEST, Some(json!({"error": {"message": "context_length_exceeded"}})), @@ -1092,7 +1089,6 @@ mod tests { "{\"error\":{\"message\":\"context_length_exceeded\"}}".to_string(), ), ), - // BAD_REQUEST - with error.message extraction ( StatusCode::BAD_REQUEST, Some(json!({"error": {"message": "Custom error"}})), @@ -1100,7 +1096,6 @@ mod tests { "Request failed with status: 400 Bad Request. Message: Custom error".to_string(), ), ), - // BAD_REQUEST - without payload ( StatusCode::BAD_REQUEST, None, @@ -1108,7 +1103,6 @@ mod tests { "Request failed with status: 400 Bad Request".to_string(), ), ), - // TOO_MANY_REQUESTS ( StatusCode::TOO_MANY_REQUESTS, Some(json!({"retry_after": 60})), @@ -1117,7 +1111,6 @@ mod tests { retry_delay: None, }, ), - // is_server_error() without payload ( StatusCode::INTERNAL_SERVER_ERROR, None, @@ -1126,7 +1119,6 @@ mod tests { None, )), ), - // is_server_error() with null payload (the "Server error: None" bug) ( StatusCode::INTERNAL_SERVER_ERROR, Some(Value::Null), @@ -1135,7 +1127,6 @@ mod tests { Some(&Value::Null), )), ), - // is_server_error() with payload ( StatusCode::BAD_GATEWAY, Some(json!({"error": "upstream error"})),