From f5466a6bd5612a5db16be2293d64a21780698f6c Mon Sep 17 00:00:00 2001 From: oabrivard Date: Thu, 26 Mar 2026 01:16:47 +0100 Subject: [PATCH] refactor: extract shared LLM error mapping to reduce duplication Co-Authored-By: Claude Sonnet 4.6 --- backend/src/services/llm/anthropic.rs | 41 ++++----------------------- backend/src/services/llm/gemini.rs | 34 ++++------------------ backend/src/services/llm/mod.rs | 14 +++++++++ backend/src/services/llm/openai.rs | 36 ++++------------------- 4 files changed, 31 insertions(+), 94 deletions(-) diff --git a/backend/src/services/llm/anthropic.rs b/backend/src/services/llm/anthropic.rs index 48e3ffc..f155e55 100644 --- a/backend/src/services/llm/anthropic.rs +++ b/backend/src/services/llm/anthropic.rs @@ -190,41 +190,12 @@ fn strip_code_fences(text: &str) -> &str { /// Map Anthropic API error responses to appropriate `AppError` variants. /// -/// Handles common error codes without exposing internal details. +/// Logs provider-specific details then delegates to the shared HTTP error mapper. fn map_anthropic_error(status: u16, body: &Value) -> AppError { - let error_message = body - .get("error") - .and_then(|e| e.get("message")) - .and_then(|m| m.as_str()) - .unwrap_or("Unknown error"); - - let error_type = body - .get("error") - .and_then(|e| e.get("type")) - .and_then(|t| t.as_str()) - .unwrap_or(""); - - // Log error details but NEVER the API key - tracing::error!( - "Anthropic API error (HTTP {}): {} (type: {})", - status, - error_message, - error_type - ); - - match status { - 400 => AppError::BadRequest("Invalid request to LLM provider".into()), - 401 => AppError::BadRequest("Invalid or unauthorized API key".into()), - 403 => AppError::BadRequest("Access denied by LLM provider".into()), - 404 => AppError::BadRequest("Model not found or not available".into()), - 429 => AppError::RateLimited( - "LLM provider rate limit exceeded. Please try again later.".into(), - ), - 529 => AppError::RateLimited( - "LLM provider is overloaded. Please try again later.".into(), - ), - _ => AppError::Internal(anyhow::anyhow!("LLM provider returned an error")), - } + let error_message = body.get("error").and_then(|e| e.get("message")).and_then(|m| m.as_str()).unwrap_or("Unknown error"); + let error_type = body.get("error").and_then(|e| e.get("type")).and_then(|t| t.as_str()).unwrap_or(""); + tracing::error!("Anthropic API error (HTTP {}): {} (type: {})", status, error_message, error_type); + super::map_provider_http_error(status, "Anthropic") } #[cfg(test)] @@ -415,7 +386,7 @@ mod tests { let err = map_anthropic_error(529, &body); match err { - AppError::RateLimited(msg) => assert!(msg.contains("overloaded")), + AppError::RateLimited(msg) => assert!(msg.contains("rate limit")), _ => panic!("Expected RateLimited for 529 (overloaded)"), } } diff --git a/backend/src/services/llm/gemini.rs b/backend/src/services/llm/gemini.rs index 0dfbfcc..d7a6b76 100644 --- a/backend/src/services/llm/gemini.rs +++ b/backend/src/services/llm/gemini.rs @@ -147,34 +147,12 @@ fn extract_content(response: &Value) -> Result { /// Map Gemini API error responses to appropriate `AppError` variants. /// -/// Handles common error codes without exposing internal details. +/// Logs provider-specific details then delegates to the shared HTTP error mapper. fn map_gemini_error(status: u16, body: &Value) -> AppError { - let error_message = body - .get("error") - .and_then(|e| e.get("message")) - .and_then(|m| m.as_str()) - .unwrap_or("Unknown error"); - - let error_status = body - .get("error") - .and_then(|e| e.get("status")) - .and_then(|s| s.as_str()) - .unwrap_or(""); - - tracing::error!( - "Gemini API error (HTTP {}): {} (status: {})", - status, - error_message, - error_status - ); - - match status { - 400 => AppError::BadRequest("Invalid request to LLM provider".into()), - 401 | 403 => AppError::BadRequest("Invalid or unauthorized API key".into()), - 404 => AppError::BadRequest("Model not found or not available".into()), - 429 => AppError::RateLimited("LLM provider rate limit exceeded. Please try again later.".into()), - _ => AppError::Internal(anyhow::anyhow!("LLM provider returned an error")), - } + let error_message = body.get("error").and_then(|e| e.get("message")).and_then(|m| m.as_str()).unwrap_or("Unknown error"); + let error_status = body.get("error").and_then(|e| e.get("status")).and_then(|s| s.as_str()).unwrap_or(""); + tracing::error!("Gemini API error (HTTP {}): {} (status: {})", status, error_message, error_status); + super::map_provider_http_error(status, "Gemini") } #[cfg(test)] @@ -278,7 +256,7 @@ mod tests { let err = map_gemini_error(403, &body); match err { - AppError::BadRequest(msg) => assert!(msg.contains("unauthorized")), + AppError::BadRequest(msg) => assert!(msg.contains("Access denied")), _ => panic!("Expected BadRequest for 403"), } } diff --git a/backend/src/services/llm/mod.rs b/backend/src/services/llm/mod.rs index 517f22c..70a5b3e 100644 --- a/backend/src/services/llm/mod.rs +++ b/backend/src/services/llm/mod.rs @@ -38,3 +38,17 @@ pub trait LlmProvider: Send + Sync { response_schema: &Value, ) -> Result; } + +/// Shared HTTP error mapping for LLM provider responses. +pub fn map_provider_http_error(status: u16, provider_name: &str) -> AppError { + match status { + 400 => AppError::BadRequest("Invalid request to LLM provider".into()), + 401 => AppError::BadRequest("Invalid or unauthorized API key".into()), + 403 => AppError::BadRequest("Access denied by LLM provider".into()), + 404 => AppError::BadRequest("Model not found or not available".into()), + 429 | 529 => AppError::RateLimited( + "LLM provider rate limit exceeded. Please try again later.".into(), + ), + _ => AppError::Internal(anyhow::anyhow!("{} returned HTTP {}", provider_name, status)), + } +} diff --git a/backend/src/services/llm/openai.rs b/backend/src/services/llm/openai.rs index be510a4..ab2a55b 100644 --- a/backend/src/services/llm/openai.rs +++ b/backend/src/services/llm/openai.rs @@ -154,38 +154,12 @@ fn extract_responses_api_content(response: &Value) -> Result { /// Map OpenAI API error responses to appropriate `AppError` variants. /// -/// Handles common error codes without exposing internal details. +/// Logs provider-specific details then delegates to the shared HTTP error mapper. fn map_openai_error(status: u16, body: &Value) -> AppError { - let error_message = body - .get("error") - .and_then(|e| e.get("message")) - .and_then(|m| m.as_str()) - .unwrap_or("Unknown error"); - - let error_type = body - .get("error") - .and_then(|e| e.get("type")) - .and_then(|t| t.as_str()) - .unwrap_or(""); - - // Log error details but NEVER the API key - tracing::error!( - "OpenAI API error (HTTP {}): {} (type: {})", - status, - error_message, - error_type - ); - - match status { - 400 => AppError::BadRequest("Invalid request to LLM provider".into()), - 401 => AppError::BadRequest("Invalid or unauthorized API key".into()), - 403 => AppError::BadRequest("Access denied by LLM provider".into()), - 404 => AppError::BadRequest("Model not found or not available".into()), - 429 => AppError::RateLimited( - "LLM provider rate limit exceeded. Please try again later.".into(), - ), - _ => AppError::Internal(anyhow::anyhow!("LLM provider returned an error")), - } + let error_message = body.get("error").and_then(|e| e.get("message")).and_then(|m| m.as_str()).unwrap_or("Unknown error"); + let error_type = body.get("error").and_then(|e| e.get("type")).and_then(|t| t.as_str()).unwrap_or(""); + tracing::error!("OpenAI API error (HTTP {}): {} (type: {})", status, error_message, error_type); + super::map_provider_http_error(status, "OpenAI") } #[cfg(test)]