Skip to content

Commit f23061a

Browse files
oleanderoleander
authored
Propagate OpenAI API key errors immediately and add test script (#53)
1 parent 7d2bc95 commit f23061a

File tree

5 files changed

+143
-12
lines changed

5 files changed

+143
-12
lines changed

‎Cargo.lock‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/bin/hook.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ use std::str::FromStr;
4646
use std::time::Duration;
4747
use std::path::PathBuf;
4848

49+
#[cfg(debug_assertions)]
4950
use colored::Colorize;
5051
use structopt::StructOpt;
5152
use indicatif::{ProgressBar, ProgressStyle};

‎src/commit.rs‎

Lines changed: 123 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -73,22 +73,56 @@ pub async fn generate(patch: String, remaining_tokens: usize, model: Model, sett
7373
.and_then(|s| s.max_commit_length)
7474
.or(config::APP.max_commit_length);
7575

76+
// Check if we have a valid API key configuration
77+
let has_valid_api_key = if let Some(custom_settings) = settings {
78+
custom_settings
79+
.openai_api_key
80+
.as_ref()
81+
.map(|key| !key.is_empty() && key != "<PLACE HOLDER FOR YOUR API KEY>")
82+
.unwrap_or(false)
83+
} else {
84+
// Check environment variable or config
85+
config::APP
86+
.openai_api_key
87+
.as_ref()
88+
.map(|key| !key.is_empty() && key != "<PLACE HOLDER FOR YOUR API KEY>")
89+
.unwrap_or(false)
90+
|| std::env::var("OPENAI_API_KEY")
91+
.map(|key| !key.is_empty())
92+
.unwrap_or(false)
93+
};
94+
95+
if !has_valid_api_key {
96+
bail!("OpenAI API key not configured. Please set your API key using:\n git-ai config set openai-api-key <your-key>\nor set the OPENAI_API_KEY environment variable.");
97+
}
98+
7699
// Use custom settings if provided
77100
if let Some(custom_settings) = settings {
78101
if let Some(api_key) = &custom_settings.openai_api_key {
79102
if !api_key.is_empty() && api_key != "<PLACE HOLDER FOR YOUR API KEY>" {
80-
let config = openai::create_openai_config(custom_settings)?;
81-
let client = Client::with_config(config);
82-
let model_str = model.to_string();
103+
match openai::create_openai_config(custom_settings) {
104+
Ok(config) => {
105+
let client = Client::with_config(config);
106+
let model_str = model.to_string();
83107

84-
match generate_commit_message_multi_step(&client, &model_str, &patch, max_length).await {
85-
Ok(message) => return Ok(openai::Response { response: message }),
86-
Err(e) => {
87-
log::warn!("Multi-step generation with custom settings failed: {e}");
88-
if let Some(session) = debug_output::debug_session() {
89-
session.set_multi_step_error(e.to_string());
108+
match generate_commit_message_multi_step(&client, &model_str, &patch, max_length).await {
109+
Ok(message) => return Ok(openai::Response { response: message }),
110+
Err(e) => {
111+
// Check if it's an API key error
112+
if e.to_string().contains("invalid_api_key") || e.to_string().contains("Incorrect API key") {
113+
bail!("Invalid OpenAI API key. Please check your API key configuration.");
114+
}
115+
log::warn!("Multi-step generation with custom settings failed: {e}");
116+
if let Some(session) = debug_output::debug_session() {
117+
session.set_multi_step_error(e.to_string());
118+
}
119+
}
90120
}
91121
}
122+
Err(e) => {
123+
// If config creation fails due to API key, propagate the error
124+
return Err(e);
125+
}
92126
}
93127
}
94128
}
@@ -102,6 +136,10 @@ pub async fn generate(patch: String, remaining_tokens: usize, model: Model, sett
102136
match generate_commit_message_multi_step(&client, &model_str, &patch, max_length).await {
103137
Ok(message) => return Ok(openai::Response { response: message }),
104138
Err(e) => {
139+
// Check if it's an API key error
140+
if e.to_string().contains("invalid_api_key") || e.to_string().contains("Incorrect API key") {
141+
bail!("Invalid OpenAI API key. Please check your API key configuration.");
142+
}
105143
log::warn!("Multi-step generation failed: {e}");
106144
if let Some(session) = debug_output::debug_session() {
107145
session.set_multi_step_error(e.to_string());
@@ -132,8 +170,10 @@ pub async fn generate(patch: String, remaining_tokens: usize, model: Model, sett
132170
match settings {
133171
Some(custom_settings) => {
134172
// Create a client with custom settings
135-
let config = openai::create_openai_config(custom_settings)?;
136-
openai::call_with_config(request, config).await
173+
match openai::create_openai_config(custom_settings) {
174+
Ok(config) => openai::call_with_config(request, config).await,
175+
Err(e) => Err(e)
176+
}
137177
}
138178
None => {
139179
// Use the default global config
@@ -158,3 +198,75 @@ pub fn get_instruction_token_count(model: &Model) -> Result<usize> {
158198
let template = get_instruction_template()?;
159199
model.count_tokens(&template)
160200
}
201+
202+
#[cfg(test)]
203+
mod tests {
204+
use super::*;
205+
206+
#[tokio::test]
207+
async fn test_missing_api_key_error() {
208+
// Create settings with no API key
209+
let settings = Settings {
210+
openai_api_key: None,
211+
model: Some("gpt-4o-mini".to_string()),
212+
max_tokens: Some(1024),
213+
max_commit_length: Some(72),
214+
timeout: Some(30)
215+
};
216+
217+
// Temporarily clear the environment variable
218+
let original_key = std::env::var("OPENAI_API_KEY").ok();
219+
std::env::remove_var("OPENAI_API_KEY");
220+
221+
// Test that generate returns an error for missing API key
222+
let result = generate(
223+
"diff --git a/test.txt b/test.txt\n+Hello World".to_string(),
224+
1024,
225+
Model::GPT4oMini,
226+
Some(&settings)
227+
)
228+
.await;
229+
230+
// Restore original environment variable if it existed
231+
if let Some(key) = original_key {
232+
std::env::set_var("OPENAI_API_KEY", key);
233+
}
234+
235+
assert!(result.is_err());
236+
let error_message = result.unwrap_err().to_string();
237+
assert!(
238+
error_message.contains("OpenAI API key not configured"),
239+
"Expected error message about missing API key, got: {}",
240+
error_message
241+
);
242+
}
243+
244+
#[tokio::test]
245+
async fn test_invalid_api_key_error() {
246+
// Create settings with invalid API key
247+
let settings = Settings {
248+
openai_api_key: Some("<PLACE HOLDER FOR YOUR API KEY>".to_string()),
249+
model: Some("gpt-4o-mini".to_string()),
250+
max_tokens: Some(1024),
251+
max_commit_length: Some(72),
252+
timeout: Some(30)
253+
};
254+
255+
// Test that generate returns an error for invalid API key
256+
let result = generate(
257+
"diff --git a/test.txt b/test.txt\n+Hello World".to_string(),
258+
1024,
259+
Model::GPT4oMini,
260+
Some(&settings)
261+
)
262+
.await;
263+
264+
assert!(result.is_err());
265+
let error_message = result.unwrap_err().to_string();
266+
assert!(
267+
error_message.contains("OpenAI API key not configured"),
268+
"Expected error message about invalid API key, got: {}",
269+
error_message
270+
);
271+
}
272+
}

‎src/multi_step_integration.rs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,11 @@ pub async fn generate_commit_message_multi_step(
8989
file_analyses.push((file, analysis));
9090
}
9191
Err(e) => {
92+
// Check if it's an API key error - if so, propagate it immediately
93+
let error_str = e.to_string();
94+
if error_str.contains("invalid_api_key") || error_str.contains("Incorrect API key") || error_str.contains("Invalid API key") {
95+
return Err(e);
96+
}
9297
log::warn!("Failed to analyze file {}: {}", file.path, e);
9398
// Continue with other files even if one fails
9499
}

‎src/openai.rs‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,10 @@ pub async fn call_with_config(request: Request, config: OpenAIConfig) -> Result<
208208
match generate_commit_message_multi_step(&client, &model, &request.prompt, config::APP.max_commit_length).await {
209209
Ok(message) => return Ok(Response { response: message }),
210210
Err(e) => {
211+
// Check if it's an API key error and propagate it
212+
if e.to_string().contains("invalid_api_key") || e.to_string().contains("Incorrect API key") {
213+
return Err(e);
214+
}
211215
log::warn!("Multi-step approach failed, falling back to single-step: {e}");
212216
}
213217
}
@@ -335,6 +339,15 @@ pub async fn call_with_config(request: Request, config: OpenAIConfig) -> Result<
335339
last_error = Some(e);
336340
log::warn!("OpenAI API attempt {attempt} failed");
337341

342+
// Check if it's an API key error - fail immediately without retrying
343+
if let OpenAIError::ApiError(ref api_err) = &last_error.as_ref().unwrap() {
344+
if api_err.code.as_deref() == Some("invalid_api_key") {
345+
let error_msg = format!("Invalid OpenAI API key: {}", api_err.message);
346+
log::error!("{error_msg}");
347+
return Err(anyhow!(error_msg));
348+
}
349+
}
350+
338351
if attempt < MAX_ATTEMPTS {
339352
let delay = Duration::from_millis(500 * attempt as u64);
340353
log::debug!("Retrying after {delay:?}");

0 commit comments

Comments
 (0)