diff --git a/src/commit.rs b/src/commit.rs index efcf603..c939f8b 100644 --- a/src/commit.rs +++ b/src/commit.rs @@ -80,7 +80,11 @@ pub async fn generate(patch: String, remaining_tokens: usize, model: Model, sett bail!("Maximum token count must be greater than zero") } - // Try multi-step approach first + // Try multi-step approach first (see multi_step_integration.rs for details) + // + // Error handling strategy: + // - Authentication errors are propagated immediately using is_openai_auth_error() + // - Other errors trigger fallback to local multi-step or single-step generation let max_length = settings .and_then(|s| s.max_commit_length) .or(config::APP_CONFIG.max_commit_length); @@ -121,7 +125,7 @@ pub async fn generate(patch: String, remaining_tokens: usize, model: Model, sett Ok(message) => return Ok(openai::Response { response: message }), Err(e) => { // Check if it's an API key error - if e.to_string().contains("invalid_api_key") || e.to_string().contains("Incorrect API key") { + if crate::error::is_openai_auth_error(&e) { bail!("Invalid OpenAI API key. Please check your API key configuration."); } log::warn!("Multi-step generation with custom settings failed: {e}"); @@ -149,7 +153,7 @@ pub async fn generate(patch: String, remaining_tokens: usize, model: Model, sett Ok(message) => return Ok(openai::Response { response: message }), Err(e) => { // Check if it's an API key error - if e.to_string().contains("invalid_api_key") || e.to_string().contains("Incorrect API key") { + if crate::error::is_openai_auth_error(&e) { bail!("Invalid OpenAI API key. Please check your API key configuration."); } log::warn!("Multi-step generation failed: {e}"); diff --git a/src/error.rs b/src/error.rs new file mode 100644 index 0000000..dd3005f --- /dev/null +++ b/src/error.rs @@ -0,0 +1,207 @@ +//! Error handling utilities for the git-ai CLI tool. +//! +//! This module provides helpers for detecting and handling specific error types, +//! particularly authentication failures from the OpenAI API. +//! +//! # OpenAI Error Structure +//! +//! According to the official async-openai documentation: +//! - `OpenAIError::ApiError(ApiError)` contains structured error information from OpenAI +//! - `ApiError` has fields: `message`, `type`, `param`, and `code` +//! - Authentication errors have `code` set to `"invalid_api_key"` +//! - `OpenAIError::Reqwest(Error)` contains HTTP-level errors (connection issues, etc.) +//! +//! Reference: https://docs.rs/async-openai/latest/async_openai/error/ + +use anyhow::Error; +use async_openai::error::OpenAIError; + +/// Checks if an error represents an OpenAI API authentication failure. +/// +/// This function detects authentication failures by checking for: +/// 1. **Structured API errors** (preferred): Checks if the error contains an `OpenAIError::ApiError` +/// with `code` field set to `"invalid_api_key"` - this is the official OpenAI error code +/// for authentication failures. +/// 2. **String-based fallback**: As a fallback, checks for authentication-related keywords in +/// the error message for cases where the error has been wrapped or converted to a string. +/// +/// This approach is based on the official OpenAI API error codes documentation and the +/// async-openai Rust library structure. +/// +/// # Arguments +/// +/// * `error` - The error to check +/// +/// # Returns +/// +/// `true` if the error appears to be an authentication failure, `false` otherwise +/// +/// # Examples +/// +/// ``` +/// use anyhow::anyhow; +/// use ai::error::is_openai_auth_error; +/// +/// let error = anyhow!("invalid_api_key: Incorrect API key provided"); +/// assert!(is_openai_auth_error(&error)); +/// ``` +pub fn is_openai_auth_error(error: &Error) -> bool { + // First, try to downcast to OpenAIError for accurate detection + if let Some(openai_err) = error.downcast_ref::() { + match openai_err { + // Official OpenAI API error with structured error code + OpenAIError::ApiError(api_err) => { + // Check for the official invalid_api_key error code + if api_err.code.as_deref() == Some("invalid_api_key") { + return true; + } + // Also check for authentication-related types + if let Some(err_type) = &api_err.r#type { + if err_type.contains("authentication") || err_type.contains("invalid_request_error") { + // For invalid_request_error, check if the message mentions API key + if err_type == "invalid_request_error" && api_err.message.to_lowercase().contains("api key") { + return true; + } + } + } + } + // HTTP-level errors (connection failures, malformed requests, etc.) + OpenAIError::Reqwest(_) => { + // Reqwest errors for auth issues typically manifest as connection errors + // when the API key format is completely invalid (e.g., "dl://BA7...") + let msg = error.to_string().to_lowercase(); + if msg.contains("error sending request") || msg.contains("connection") { + return true; + } + } + _ => {} + } + } + + // Fallback: String-based detection for wrapped errors + let msg = error.to_string().to_lowercase(); + + // OpenAI-specific API key errors (from API responses) + msg.contains("invalid_api_key") || + msg.contains("incorrect api key") || + msg.contains("openai api authentication failed") || + + // Generic auth failures (scoped to avoid false positives) + (msg.contains("authentication") && msg.contains("openai")) || + (msg.contains("unauthorized") && msg.contains("openai")) || + + // HTTP errors that typically indicate auth issues with OpenAI + // This pattern catches connection issues when the API key is malformed + (msg.contains("http error") && msg.contains("error sending request")) +} + +#[cfg(test)] +mod tests { + use anyhow::anyhow; + use async_openai::error::{ApiError, OpenAIError}; + + use super::*; + + // Tests for structured OpenAIError detection (preferred method) + + #[test] + fn test_detects_structured_invalid_api_key() { + let api_error = ApiError { + message: "Incorrect API key provided: dl://BA7...".to_string(), + r#type: Some("invalid_request_error".to_string()), + param: None, + code: Some("invalid_api_key".to_string()) + }; + let openai_error = OpenAIError::ApiError(api_error); + let error: anyhow::Error = openai_error.into(); + assert!(is_openai_auth_error(&error)); + } + + #[test] + fn test_detects_invalid_request_with_api_key_message() { + let api_error = ApiError { + message: "You must provide a valid API key".to_string(), + r#type: Some("invalid_request_error".to_string()), + param: None, + code: None + }; + let openai_error = OpenAIError::ApiError(api_error); + let error: anyhow::Error = openai_error.into(); + assert!(is_openai_auth_error(&error)); + } + + #[test] + fn test_detects_reqwest_error_sending_request() { + // Simulate a wrapped reqwest error by using anyhow + // In production, malformed API keys cause "error sending request" from reqwest + let error = anyhow!("http error: error sending request"); + assert!(is_openai_auth_error(&error)); + } + + #[test] + fn test_ignores_structured_non_auth_error() { + let api_error = ApiError { + message: "Model not found".to_string(), + r#type: Some("invalid_request_error".to_string()), + param: Some("model".to_string()), + code: Some("model_not_found".to_string()) + }; + let openai_error = OpenAIError::ApiError(api_error); + let error: anyhow::Error = openai_error.into(); + assert!(!is_openai_auth_error(&error)); + } + + // Tests for string-based fallback detection (for wrapped errors) + + #[test] + fn test_detects_invalid_api_key_string() { + let error = anyhow!("invalid_api_key: Incorrect API key provided"); + assert!(is_openai_auth_error(&error)); + } + + #[test] + fn test_detects_incorrect_api_key_string() { + let error = anyhow!("Incorrect API key provided: sk-xxxxx"); + assert!(is_openai_auth_error(&error)); + } + + #[test] + fn test_detects_openai_auth_failed_string() { + let error = anyhow!("OpenAI API authentication failed: http error"); + assert!(is_openai_auth_error(&error)); + } + + #[test] + fn test_detects_http_error_sending_request() { + let error = anyhow!("http error: error sending request"); + assert!(is_openai_auth_error(&error)); + } + + #[test] + fn test_detects_openai_specific_auth() { + let error = anyhow!("OpenAI authentication failed"); + assert!(is_openai_auth_error(&error)); + } + + #[test] + fn test_ignores_generic_auth_errors() { + // Should not match generic auth errors without OpenAI context + let error = anyhow!("Database authentication timeout"); + assert!(!is_openai_auth_error(&error)); + + let error = anyhow!("OAuth2 unauthorized redirect"); + assert!(!is_openai_auth_error(&error)); + } + + #[test] + fn test_ignores_unrelated_errors() { + let error = anyhow!("File not found"); + assert!(!is_openai_auth_error(&error)); + } + + #[test] + fn test_ignores_non_auth_openai_errors() { + let error = anyhow!("OpenAI rate limit exceeded"); + assert!(!is_openai_auth_error(&error)); + } +} diff --git a/src/lib.rs b/src/lib.rs index 7081bf5..8a6deda 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,17 +1,18 @@ pub mod commit; pub mod config; -pub mod hook; -pub mod style; -pub mod model; +pub mod debug_output; +pub mod error; pub mod filesystem; -pub mod openai; -pub mod profiling; pub mod function_calling; +pub mod generation; +pub mod hook; +pub mod model; pub mod multi_step_analysis; pub mod multi_step_integration; +pub mod openai; +pub mod profiling; pub mod simple_multi_step; -pub mod debug_output; -pub mod generation; +pub mod style; // Re-exports pub use profiling::Profile; diff --git a/src/multi_step_integration.rs b/src/multi_step_integration.rs index b544aff..2b67c0f 100644 --- a/src/multi_step_integration.rs +++ b/src/multi_step_integration.rs @@ -20,6 +20,22 @@ pub struct ParsedFile { } /// Main entry point for multi-step commit message generation +/// +/// This function uses a sophisticated divide-and-conquer approach: +/// 1. Parse the diff into individual files +/// 2. Analyze each file concurrently using the OpenAI API +/// 3. Calculate impact scores for each file +/// 4. Generate multiple commit message candidates +/// 5. Select the best message based on impact scores +/// +/// # Error Handling +/// +/// Authentication failures are detected early and propagated immediately to provide +/// clear feedback to users. Uses [`crate::error::is_openai_auth_error`] to identify +/// API key issues and other authentication problems. +/// +/// Other errors (non-auth) are logged as warnings and processing continues with +/// remaining files to maximize the chance of generating a useful commit message. pub async fn generate_commit_message_multi_step( client: &Client, model: &str, diff_content: &str, max_length: Option ) -> Result { @@ -89,10 +105,12 @@ pub async fn generate_commit_message_multi_step( file_analyses.push((file, analysis)); } Err(e) => { - // Check if it's an API key error - if so, propagate it immediately - let error_str = e.to_string(); - if error_str.contains("invalid_api_key") || error_str.contains("Incorrect API key") || error_str.contains("Invalid API key") { - return Err(e); + // Check if it's an API key or authentication error - if so, propagate it immediately + if crate::error::is_openai_auth_error(&e) { + return Err(anyhow::anyhow!( + "OpenAI API authentication failed: {}. Please check your API key configuration.", + e + )); } log::warn!("Failed to analyze file {}: {}", file.path, e); // Continue with other files even if one fails diff --git a/src/openai.rs b/src/openai.rs index 8125e83..04d18fb 100644 --- a/src/openai.rs +++ b/src/openai.rs @@ -198,6 +198,16 @@ fn truncate_to_fit(text: &str, max_tokens: usize, model: &Model) -> Result Result { profile!("OpenAI API call with custom config"); @@ -209,7 +219,7 @@ pub async fn call_with_config(request: Request, config: OpenAIConfig) -> Result< Ok(message) => return Ok(Response { response: message }), Err(e) => { // Check if it's an API key error and propagate it - if e.to_string().contains("invalid_api_key") || e.to_string().contains("Incorrect API key") { + if crate::error::is_openai_auth_error(&e) { return Err(e); } log::warn!("Multi-step approach failed, falling back to single-step: {e}"); diff --git a/tests/api_key_error_test.rs b/tests/api_key_error_test.rs new file mode 100644 index 0000000..7e5b262 --- /dev/null +++ b/tests/api_key_error_test.rs @@ -0,0 +1,45 @@ +use ai::commit; +use ai::config::AppConfig; +use ai::model::Model; + +#[tokio::test] +async fn test_invalid_api_key_propagates_error() { + // Initialize logging to capture warnings + let _ = env_logger::builder().is_test(true).try_init(); + + // Ensure no API key is available from environment to force early validation failure + let original_key = std::env::var("OPENAI_API_KEY").ok(); + std::env::remove_var("OPENAI_API_KEY"); + + // Create settings with an invalid API key that fails early validation (no network calls) + let settings = AppConfig { + openai_api_key: Some("".to_string()), // Empty string triggers early validation failure + model: Some("gpt-4o-mini".to_string()), + max_tokens: Some(1024), + max_commit_length: Some(72), + timeout: Some(30) + }; + + let example_diff = "diff --git a/test.txt b/test.txt\n+Hello World".to_string(); + + // This should fail with an API key error, not log a warning and continue + let result = commit::generate(example_diff, 1024, Model::GPT41Mini, Some(&settings)).await; + + // Restore original environment variable if it existed + if let Some(key) = original_key { + std::env::set_var("OPENAI_API_KEY", key); + } + + // Verify the behavior - it should return an error, not continue with other files + assert!(result.is_err(), "Expected API key error to be propagated as error, not warning"); + + let error_message = result.unwrap_err().to_string(); + println!("Actual error message: '{}'", error_message); + + // The error should indicate that the API key is not configured (early validation without network calls) + assert!( + error_message.contains("OpenAI API key not configured") || error_message.contains("Invalid OpenAI API key"), + "Expected error message to indicate API key configuration issue, got: {}", + error_message + ); +}