Skip to content

Commit 61cf6c7

Browse files
committed
🛡️ Improve error handling and add defensive programming
Add robust error handling throughout the codebase to prevent crashes This change implements more resilient error handling patterns across the app: - Replace unwrap() calls with unwrap_or_else() to provide fallback values - Add file extension handling with case-insensitive comparisons - Implement file analyzer improvements with robust file type detection - Enhance JSON parsing with proper error handling and recovery - Add debug logging for parsing failures - Add parking_lot::Mutex for more reliable mutex handling - Improve error handling in regex operations - Make file matching more robust in file analyzers module These changes significantly improve the reliability and user experience by preventing crashes and providing graceful error recovery.
1 parent 46fbe7b commit 61cf6c7

File tree

15 files changed

+238
-87
lines changed

15 files changed

+238
-87
lines changed

Cargo.lock

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

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ tokio-retry = "0.3.0"
4444
log = "0.4.22"
4545
futures = "0.3.30"
4646
schemars = "0.8.21"
47+
parking_lot = "0.12.1"
4748

4849
[dev-dependencies]
4950
dotenv = "0.15.0"

src/changes/change_analyzer.rs

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,52 @@ impl ChangeAnalyzer {
108108
_ => ChangeType::Modified,
109109
};
110110

111+
let file_path = new_file.path().map_or_else(
112+
|| {
113+
old_file
114+
.path()
115+
.map(|p| p.to_string_lossy().into_owned())
116+
.unwrap_or_default()
117+
},
118+
|p| p.to_string_lossy().into_owned(),
119+
);
120+
121+
// Perform file-specific analysis based on file type
122+
let mut analysis = Vec::new();
123+
124+
// Determine file type and add relevant analysis
125+
if let Some(extension) = std::path::Path::new(&file_path).extension() {
126+
if let Some(ext_str) = extension.to_str() {
127+
match ext_str.to_lowercase().as_str() {
128+
"rs" => analysis.push("Rust source code changes".to_string()),
129+
"js" | "ts" => {
130+
analysis.push("JavaScript/TypeScript changes".to_string());
131+
}
132+
"py" => analysis.push("Python code changes".to_string()),
133+
"java" => analysis.push("Java code changes".to_string()),
134+
"c" | "cpp" | "h" => analysis.push("C/C++ code changes".to_string()),
135+
"md" => analysis.push("Documentation changes".to_string()),
136+
"json" | "yml" | "yaml" | "toml" => {
137+
analysis.push("Configuration changes".to_string());
138+
}
139+
_ => {}
140+
}
141+
}
142+
}
143+
144+
// Add analysis based on change type
145+
match change_type {
146+
ChangeType::Added => analysis.push("New file added".to_string()),
147+
ChangeType::Deleted => analysis.push("File removed".to_string()),
148+
ChangeType::Modified => {
149+
if file_path.contains("test") || file_path.contains("spec") {
150+
analysis.push("Test modifications".to_string());
151+
} else if file_path.contains("README") || file_path.contains("docs/") {
152+
analysis.push("Documentation updates".to_string());
153+
}
154+
}
155+
}
156+
111157
let file_change = FileChange {
112158
old_path: old_file
113159
.path()
@@ -118,7 +164,7 @@ impl ChangeAnalyzer {
118164
.map(|p| p.to_string_lossy().into_owned())
119165
.unwrap_or_default(),
120166
change_type,
121-
analysis: vec![], // TODO: Implement file-specific analysis if needed
167+
analysis,
122168
};
123169

124170
file_changes.push(file_change);

src/changes/models.rs

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
use crate::log_debug;
12
use schemars::JsonSchema;
23
use serde::{Deserialize, Serialize};
34
use std::collections::HashMap;
@@ -117,16 +118,47 @@ pub struct SectionItem {
117118

118119
impl From<String> for ChangelogResponse {
119120
/// Converts a JSON string to a `ChangelogResponse`
120-
#[allow(clippy::unwrap_used)] // todo: handle unwrap maybe use try_from instead
121121
fn from(value: String) -> Self {
122-
serde_json::from_str(&value).unwrap()
122+
serde_json::from_str(&value).unwrap_or_else(|e| {
123+
log_debug!("Failed to parse ChangelogResponse: {}", e);
124+
Self {
125+
version: Some("Error".to_string()),
126+
release_date: Some("Error".to_string()),
127+
sections: HashMap::new(),
128+
breaking_changes: Vec::new(),
129+
metrics: ChangeMetrics {
130+
total_commits: 0,
131+
files_changed: 0,
132+
insertions: 0,
133+
deletions: 0,
134+
total_lines_changed: 0,
135+
},
136+
}
137+
})
123138
}
124139
}
125140

126141
impl From<String> for ReleaseNotesResponse {
127142
/// Converts a JSON string to a `ReleaseNotesResponse`
128-
#[allow(clippy::unwrap_used)] // todo: handle unwrap maybe use try_from instead
129143
fn from(value: String) -> Self {
130-
serde_json::from_str(&value).unwrap()
144+
serde_json::from_str(&value).unwrap_or_else(|e| {
145+
log_debug!("Failed to parse ReleaseNotesResponse: {}", e);
146+
Self {
147+
version: Some("Error".to_string()),
148+
release_date: Some("Error".to_string()),
149+
summary: format!("Error parsing response: {e}"),
150+
highlights: Vec::new(),
151+
sections: Vec::new(),
152+
breaking_changes: Vec::new(),
153+
upgrade_notes: Vec::new(),
154+
metrics: ChangeMetrics {
155+
total_commits: 0,
156+
files_changed: 0,
157+
insertions: 0,
158+
deletions: 0,
159+
total_lines_changed: 0,
160+
},
161+
}
162+
})
131163
}
132164
}

src/changes/prompt.rs

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,17 @@ use super::{
55
use crate::common::{get_combined_instructions, DetailLevel};
66
use crate::config::Config;
77
use crate::gitmoji::get_gitmoji_list;
8+
use crate::log_debug;
89

9-
#[allow(clippy::unwrap_used)] // todo: handle unwrap
1010
pub fn create_changelog_system_prompt(config: &Config) -> String {
1111
let changelog_schema = schemars::schema_for!(ChangelogResponse);
12-
let changelog_schema_str = serde_json::to_string_pretty(&changelog_schema).unwrap();
12+
let changelog_schema_str = match serde_json::to_string_pretty(&changelog_schema) {
13+
Ok(schema) => schema,
14+
Err(e) => {
15+
log_debug!("Failed to serialize changelog schema: {}", e);
16+
"{ \"error\": \"Failed to serialize schema\" }".to_string()
17+
}
18+
};
1319

1420
let mut prompt = String::from(
1521
"You are an AI assistant specialized in generating clear, concise, and informative changelogs for software projects. \
@@ -116,10 +122,15 @@ pub fn create_changelog_system_prompt(config: &Config) -> String {
116122
prompt
117123
}
118124

119-
#[allow(clippy::unwrap_used)] // todo: handle unwrap maybe use try_from instead
120125
pub fn create_release_notes_system_prompt(config: &Config) -> String {
121126
let release_notes_schema = schemars::schema_for!(ReleaseNotesResponse);
122-
let release_notes_schema_str = serde_json::to_string_pretty(&release_notes_schema).unwrap();
127+
let release_notes_schema_str = match serde_json::to_string_pretty(&release_notes_schema) {
128+
Ok(schema) => schema,
129+
Err(e) => {
130+
log_debug!("Failed to serialize release notes schema: {}", e);
131+
"{ \"error\": \"Failed to serialize schema\" }".to_string()
132+
}
133+
};
123134

124135
let mut prompt = String::from(
125136
"You are an AI assistant specialized in generating comprehensive and user-friendly release notes for software projects. \

src/changes/readme_reader.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,15 @@ pub async fn get_readme_summary(
1111
config: &Config,
1212
provider_type: &LLMProviderType,
1313
) -> Result<Option<String>> {
14-
if let Some(readme_content) = git_repo
14+
match git_repo
1515
.get_readme_at_commit(commit_ish)
1616
.context("Failed to get README at specified commit")?
1717
{
18-
let summary = summarize_readme(config, provider_type, &readme_content).await?;
19-
Ok(Some(summary))
20-
} else {
21-
Ok(None)
18+
Some(readme_content) => {
19+
let summary = summarize_readme(config, provider_type, &readme_content).await?;
20+
Ok(Some(summary))
21+
}
22+
_ => Ok(None),
2223
}
2324
}
2425

src/config.rs

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,6 @@ impl Config {
200200
}
201201

202202
impl Default for Config {
203-
#[allow(clippy::unwrap_used)] // todo: handle unwrap
204203
fn default() -> Self {
205204
let mut providers = HashMap::new();
206205
for provider in get_available_providers() {
@@ -210,10 +209,20 @@ impl Default for Config {
210209
);
211210
}
212211

212+
// Default to OpenAI if available, otherwise use the first available provider
213+
let default_provider = if providers.contains_key("OpenAI") {
214+
"OpenAI".to_string()
215+
} else {
216+
providers.keys().next().map_or_else(
217+
|| "OpenAI".to_string(), // Fallback even if no providers (should never happen)
218+
std::clone::Clone::clone,
219+
)
220+
};
221+
213222
Self {
214-
default_provider: get_available_providers().first().unwrap().to_string(),
223+
default_provider,
215224
providers,
216-
use_gitmoji: true,
225+
use_gitmoji: default_gitmoji(),
217226
instructions: String::new(),
218227
instruction_preset: default_instruction_preset(),
219228
temp_instructions: None,

src/context.rs

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
use crate::log_debug;
12
use crate::token_optimizer::TokenOptimizer;
23
use schemars::JsonSchema;
34
use serde::{Deserialize, Serialize};
@@ -40,9 +41,15 @@ pub struct GeneratedMessage {
4041
}
4142

4243
impl From<String> for GeneratedMessage {
43-
#[allow(clippy::unwrap_used)] // todo: handle error maybe replace with try_from
4444
fn from(value: String) -> Self {
45-
serde_json::from_str(&value).unwrap()
45+
serde_json::from_str(&value).unwrap_or_else(|e| {
46+
log_debug!("Failed to parse GeneratedMessage: {}", e);
47+
GeneratedMessage {
48+
emoji: None,
49+
title: "Parsing Error".to_string(),
50+
message: format!("Failed to parse response: {e}"),
51+
}
52+
})
4653
}
4754
}
4855

src/file_analyzers/javascript.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,11 +35,10 @@ impl FileAnalyzer for JavaScriptAnalyzer {
3535
"JavaScript/TypeScript source file"
3636
}
3737

38-
#[allow(clippy::case_sensitive_file_extension_comparisons)] // todo: check if we should compare case-insensitively
3938
fn extract_metadata(&self, file: &str, content: &str) -> ProjectMetadata {
4039
let mut metadata = ProjectMetadata {
4140
language: Some(
42-
if file.ends_with(".ts") {
41+
if file.to_lowercase().ends_with(".ts") {
4342
"TypeScript"
4443
} else {
4544
"JavaScript"

src/file_analyzers/json.rs

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use super::{FileAnalyzer, ProjectMetadata};
22
use crate::context::StagedFile;
3+
use crate::log_debug;
34
use regex::Regex;
45
use std::collections::HashSet;
56

@@ -82,20 +83,31 @@ impl JsonAnalyzer {
8283
}
8384
}
8485

85-
#[allow(clippy::unwrap_used)] // todo: handle unwrap maybe use try_from instead
8686
fn extract_modified_top_level_keys(diff: &str) -> Option<Vec<String>> {
8787
let lines: Vec<&str> = diff.lines().collect();
88-
let re = Regex::new(r#"^[+-]\s*"(\w+)"\s*:"#).expect("Could not compile regex");
88+
let re = match Regex::new(r#"^[+-]\s*"(\w+)"\s*:"#) {
89+
Ok(re) => re,
90+
Err(e) => {
91+
log_debug!("Failed to compile regex: {}", e);
92+
return None;
93+
}
94+
};
8995
let mut keys = HashSet::new();
9096

9197
for (i, line) in lines.iter().enumerate() {
9298
if let Some(cap) = re.captures(line) {
93-
let key = cap.get(1).unwrap().as_str();
94-
let prev_line = if i > 0 { lines[i - 1] } else { "" };
95-
let next_line = lines.get(i + 1).unwrap_or(&"");
96-
97-
if !prev_line.trim().ends_with('{') && !next_line.trim().starts_with('}') {
98-
keys.insert(key.to_string());
99+
if let Some(key_match) = cap.get(1) {
100+
let key = key_match.as_str();
101+
let prev_line = if i > 0 { lines[i - 1] } else { "" };
102+
let next_line = if i + 1 < lines.len() {
103+
lines[i + 1]
104+
} else {
105+
""
106+
};
107+
108+
if !prev_line.trim().ends_with('{') && !next_line.trim().starts_with('}') {
109+
keys.insert(key.to_string());
110+
}
99111
}
100112
}
101113
}

0 commit comments

Comments
 (0)