Skip to content

Commit acf563d

Browse files
committed
feat: improve error handling for mutex locks across various commands and settings
1 parent e9e0bb2 commit acf563d

14 files changed

Lines changed: 198 additions & 106 deletions

src-tauri/Cargo.toml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,8 @@ index-progress-logging = []
6969
index-error-logging = []
7070

7171
# Testing features
72-
full = ["long-tests", "generate-test-data", "benchmarks", "open-file-in-app"]
73-
full-no-generate-test-data = ["long-tests", "benchmarks", "open-file-in-app"]
72+
full = ["long-tests", "generate-test-data", "benchmarks", "open-file-in-app", "sftp-tests"]
73+
full-no-generate-test-data = ["long-tests", "benchmarks", "open-file-in-app", "sftp-tests"]
7474
open-file-in-app = []
7575
generate-test-data = []
7676
long-tests = []

src-tauri/src/commands/file_system_operation_commands.rs

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -178,14 +178,14 @@ pub async fn open_directory(path: String) -> Result<String, String> {
178178

179179
if file_type.is_dir() {
180180
directories.push(models::Directory {
181-
name: entry.file_name().to_str().unwrap().to_string(),
182-
path: path_of_entry.to_str().unwrap().to_string(),
181+
name: entry.file_name().to_str().unwrap_or("[invalid name]").to_string(),
182+
path: path_of_entry.to_str().unwrap_or("[invalid path]").to_string(),
183183
is_symlink: path_of_entry.is_symlink(),
184184
access_rights_as_string: get_access_permission_string(metadata.permissions(), true),
185185
access_rights_as_number: get_access_permission_number(metadata.permissions(), true),
186186
size_in_bytes: 0,
187-
sub_file_count: count_subfiles(path_of_entry.to_str().unwrap()),
188-
sub_dir_count: count_subdirectories(path_of_entry.to_str().unwrap()),
187+
sub_file_count: path_of_entry.to_str().map(count_subfiles).unwrap_or(0),
188+
sub_dir_count: path_of_entry.to_str().map(count_subdirectories).unwrap_or(0),
189189
created: metadata
190190
.created()
191191
.map_or("1970-01-01 00:00:00".to_string(), |time| {
@@ -204,8 +204,8 @@ pub async fn open_directory(path: String) -> Result<String, String> {
204204
});
205205
} else if file_type.is_file() {
206206
files.push(models::File {
207-
name: entry.file_name().to_str().unwrap().to_string(),
208-
path: path_of_entry.to_str().unwrap().to_string(),
207+
name: entry.file_name().to_str().unwrap_or("[invalid name]").to_string(),
208+
path: path_of_entry.to_str().unwrap_or("[invalid path]").to_string(),
209209
is_symlink: path_of_entry.is_symlink(),
210210
access_rights_as_string: get_access_permission_string(
211211
metadata.permissions(),
@@ -475,13 +475,13 @@ fn generate_unique_path(original_path: &str) -> String {
475475
return original_path.to_string();
476476
}
477477

478-
let parent = path.parent().unwrap_or(Path::new(""));
479-
let file_name = path.file_name().unwrap().to_string_lossy();
478+
let parent = path.parent().unwrap_or_else(|| Path::new("."));
479+
let file_name = path.file_name().map(|n| n.to_string_lossy()).unwrap_or_else(|| "[invalid_name]".into());
480480

481481
// Check if it's a file with extension or a directory
482482
if let Some(extension) = path.extension() {
483483
// It's a file with extension
484-
let stem = path.file_stem().unwrap().to_string_lossy();
484+
let stem = path.file_stem().map(|s| s.to_string_lossy()).unwrap_or_else(|| "[invalid_stem]".into());
485485
let ext = extension.to_string_lossy();
486486

487487
for i in 1..=9999 {
@@ -595,8 +595,8 @@ pub async fn copy_file_or_dir(source_path: &str, destination_path: &str) -> Resu
595595
} else if entry_path.is_dir() {
596596
// Recursively copy subdirectory
597597
let sub_size = Box::pin(copy_file_or_dir(
598-
entry_path.to_str().unwrap(),
599-
dest_path.to_str().unwrap(),
598+
entry_path.to_str().unwrap_or("[invalid source path]"),
599+
dest_path.to_str().unwrap_or("[invalid dest path]"),
600600
))
601601
.await?;
602602
total_size += sub_size;

src-tauri/src/commands/hash_commands.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ impl Display for HashError {
5858
async fn get_checksum_method(
5959
state: Arc<Mutex<SettingsState>>,
6060
) -> Result<ChecksumMethod, HashError> {
61-
let settings_state = state.lock().unwrap();
61+
let settings_state = state.lock().map_err(|_| HashError::SettingsLockError)?;
6262
let inner_settings = settings_state
6363
.0
6464
.lock()

src-tauri/src/commands/meta_data_commands.rs

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -29,17 +29,27 @@ use tauri::State;
2929
/// ```
3030
#[tauri::command]
3131
pub fn get_meta_data_as_json(state: State<Arc<Mutex<MetaDataState>>>) -> Result<String, String> {
32-
let meta_data = state.lock().unwrap().refresh_volumes();
32+
let meta_data = state.lock().map_err(|_| {
33+
Error::new(
34+
ErrorCode::InternalError,
35+
"Failed to acquire lock on metadata state".to_string(),
36+
).to_json()
37+
})?.refresh_volumes();
3338

34-
if meta_data.is_err() {
39+
if let Err(e) = meta_data {
3540
return Err(Error::new(
3641
ErrorCode::InternalError,
37-
format!("Error: {}", meta_data.err().unwrap()),
42+
format!("Error refreshing volumes: {}", e),
3843
)
3944
.to_json());
4045
}
4146

42-
let meta_data = state.lock().unwrap().0.clone();
47+
let meta_data = state.lock().map_err(|_| {
48+
Error::new(
49+
ErrorCode::InternalError,
50+
"Failed to acquire lock on metadata state".to_string(),
51+
).to_json()
52+
})?.0.clone();
4353

4454
serde_json::to_string(&meta_data).map_err(|e| {
4555
Error::new(
@@ -52,17 +62,27 @@ pub fn get_meta_data_as_json(state: State<Arc<Mutex<MetaDataState>>>) -> Result<
5262

5363
#[cfg(test)]
5464
pub fn get_meta_data_as_json_impl(state: Arc<Mutex<MetaDataState>>) -> Result<String, String> {
55-
let meta_data = state.lock().unwrap().refresh_volumes();
65+
let meta_data = state.lock().map_err(|_| {
66+
Error::new(
67+
ErrorCode::InternalError,
68+
"Failed to acquire lock on metadata state".to_string(),
69+
).to_json()
70+
})?.refresh_volumes();
5671

57-
if meta_data.is_err() {
72+
if let Err(e) = meta_data {
5873
return Err(Error::new(
5974
ErrorCode::InternalError,
60-
format!("Error: {}", meta_data.err().unwrap()),
75+
format!("Error refreshing volumes: {}", e),
6176
)
6277
.to_json());
6378
}
6479

65-
let meta_data = state.lock().unwrap().0.clone();
80+
let meta_data = state.lock().map_err(|_| {
81+
Error::new(
82+
ErrorCode::InternalError,
83+
"Failed to acquire lock on metadata state".to_string(),
84+
).to_json()
85+
})?.0.clone();
6686

6787
serde_json::to_string(&meta_data).map_err(|e| {
6888
Error::new(

src-tauri/src/commands/search_engine_commands.rs

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ pub fn search_impl(
4646
"Search implementation called with query: {}",
4747
query
4848
);
49-
let engine = state.lock().unwrap();
49+
let engine = state.lock().map_err(|_| "lock poisoned")?;
5050
engine.search(&query)
5151
}
5252

@@ -96,7 +96,7 @@ pub fn search_with_extension_impl(
9696
"Search with extension called: query='{}', extensions={:?}",
9797
query, extensions
9898
);
99-
let engine = state.lock().unwrap();
99+
let engine = state.lock().map_err(|_| "Failed to acquire lock on search engine state")?;
100100
engine.search_by_extension(&query, extensions)
101101
}
102102

@@ -173,7 +173,7 @@ pub fn add_paths_recursive_impl(
173173

174174
log_info!("Starting optimized chunked indexing for path: {} with chunk size: {}", folder, default_chunk_size);
175175

176-
let engine_state = state.lock().unwrap();
176+
let engine_state = state.lock().map_err(|_| "Failed to acquire lock on search engine state")?;
177177
let result = engine_state.start_chunked_indexing(path, default_chunk_size);
178178

179179
match &result {
@@ -212,7 +212,7 @@ pub fn add_path(
212212

213213
pub fn add_path_impl(path: String, state: Arc<Mutex<SearchEngineState>>) -> Result<(), String> {
214214
log_info!("Add path called with: {}", path);
215-
let engine = state.lock().unwrap();
215+
let engine = state.lock().map_err(|_| "Failed to acquire lock on search engine state")?;
216216
engine.add_path(&path)
217217
}
218218

@@ -250,7 +250,7 @@ pub fn remove_paths_recursive_impl(
250250
"Remove paths recursive called with folder: {}",
251251
folder
252252
);
253-
let engine = state.lock().unwrap();
253+
let engine = state.lock().map_err(|_| "Failed to acquire lock on search engine state")?;
254254
engine.remove_paths_recursive(&folder)
255255
}
256256

@@ -282,7 +282,7 @@ pub fn remove_path(
282282

283283
pub fn remove_path_impl(path: String, state: Arc<Mutex<SearchEngineState>>) -> Result<(), String> {
284284
log_info!("Remove path called with: {}", path);
285-
let engine = state.lock().unwrap();
285+
let engine = state.lock().map_err(|_| "Failed to acquire lock on search engine state")?;
286286
engine.remove_path(&path)
287287
}
288288

@@ -313,12 +313,12 @@ pub fn clear_search_engine(
313313
pub fn clear_search_engine_impl(state: Arc<Mutex<SearchEngineState>>) -> Result<(), String> {
314314
log_info!("Clear search engine called");
315315

316-
let state = state.lock().unwrap();
317-
let mut engine = state.engine.write().unwrap();
316+
let state = state.lock().map_err(|_| "Failed to acquire lock on search engine state")?;
317+
let mut engine = state.engine.write().map_err(|_| "Failed to acquire write lock on search engine")?;
318318
engine.clear();
319319

320320
// Update state
321-
let mut data = state.data.lock().unwrap();
321+
let mut data = state.data.lock().map_err(|_| "Failed to acquire lock on search engine data")?;
322322
data.last_updated = chrono::Utc::now().timestamp_millis() as u64;
323323

324324
Ok(())
@@ -376,7 +376,7 @@ pub fn get_search_engine_info_impl(
376376
state: Arc<Mutex<SearchEngineState>>,
377377
) -> Result<SearchEngineInfo, String> {
378378
log_info!("Get search engine info called");
379-
let engine = state.lock().unwrap();
379+
let engine = state.lock().map_err(|_| "Failed to acquire lock on search engine state")?;
380380
Ok(engine.get_search_engine_info())
381381
}
382382

@@ -395,7 +395,7 @@ pub async fn get_indexing_progress(
395395
progress.files_indexed,
396396
progress.files_discovered,
397397
progress.percentage_complete,
398-
progress.current_path.as_ref().map(|p| p.split('/').last().unwrap_or(p)),
398+
progress.current_path.as_ref().and_then(|p| p.split('/').last()).unwrap_or(""),
399399
data.status
400400
);
401401

@@ -756,11 +756,11 @@ pub fn get_suggestions_impl(
756756
return Ok(Vec::new());
757757
}
758758

759-
let search_engine_state = state.lock().unwrap();
759+
let search_engine_state = state.lock().map_err(|_| "Failed to acquire lock on search engine state")?;
760760

761761
// Check if search engine is enabled
762762
{
763-
let data = search_engine_state.data.lock().unwrap();
763+
let data = search_engine_state.data.lock().map_err(|_| "Failed to acquire lock on search engine data")?;
764764
if !data.config.search_engine_enabled {
765765
log_error!("Search engine is disabled in configuration.");
766766
return Err("Search engine is disabled in configuration".to_string());

src-tauri/src/commands/settings_commands.rs

Lines changed: 41 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,23 @@ pub fn get_settings_as_json(state: State<Arc<Mutex<SettingsState>>>) -> String {
2929
}
3030

3131
pub fn get_settings_as_json_impl(state: Arc<Mutex<SettingsState>>) -> String {
32-
let settings_inner = state.lock().unwrap().0.clone();
33-
to_string(&settings_inner).unwrap()
32+
let settings_inner = match state.lock() {
33+
Ok(guard) => guard.0.clone(),
34+
Err(_) => {
35+
return Error::new(
36+
ErrorCode::InternalError,
37+
"Failed to acquire lock on settings state".to_string(),
38+
).to_json();
39+
}
40+
};
41+
42+
match to_string(&settings_inner) {
43+
Ok(json) => json,
44+
Err(_) => Error::new(
45+
ErrorCode::InternalError,
46+
"Failed to serialize settings to JSON".to_string(),
47+
).to_json()
48+
}
3449
}
3550

3651
/// Retrieves the value of a specific setting field.
@@ -68,7 +83,12 @@ pub fn get_setting_field_impl(
6883
state: Arc<Mutex<SettingsState>>,
6984
key: String,
7085
) -> Result<serde_json::Value, String> {
71-
let settings_state = state.lock().unwrap();
86+
let settings_state = state.lock().map_err(|_| {
87+
Error::new(
88+
ErrorCode::InternalError,
89+
"Failed to acquire lock on settings state".to_string(),
90+
).to_json()
91+
})?;
7292
settings_state.get_setting_field(&key).map_err(|e| {
7393
Error::new(
7494
ErrorCode::InternalError,
@@ -116,7 +136,12 @@ pub fn update_settings_field_impl(
116136
key: String,
117137
value: serde_json::Value,
118138
) -> Result<String, String> {
119-
let settings_state = state.lock().unwrap();
139+
let settings_state = state.lock().map_err(|_| {
140+
Error::new(
141+
ErrorCode::InternalError,
142+
"Failed to acquire lock on settings state".to_string(),
143+
).to_json()
144+
})?;
120145
settings_state
121146
.update_setting_field(&key, value)
122147
.and_then(|updated| {
@@ -170,7 +195,12 @@ pub fn update_multiple_settings_impl(
170195
state: Arc<Mutex<SettingsState>>,
171196
updates: serde_json::Map<String, serde_json::Value>,
172197
) -> Result<String, String> {
173-
let settings_state = state.lock().unwrap();
198+
let settings_state = state.lock().map_err(|_| {
199+
Error::new(
200+
ErrorCode::InternalError,
201+
"Failed to acquire lock on settings state".to_string(),
202+
).to_json()
203+
})?;
174204
settings_state
175205
.update_multiple_settings(&updates)
176206
.and_then(|updated| {
@@ -213,7 +243,12 @@ pub fn reset_settings_command(state: State<Arc<Mutex<SettingsState>>>) -> Result
213243
}
214244

215245
pub fn reset_settings_impl(state: Arc<Mutex<SettingsState>>) -> Result<String, String> {
216-
let settings_state = state.lock().unwrap();
246+
let settings_state = state.lock().map_err(|_| {
247+
Error::new(
248+
ErrorCode::InternalError,
249+
"Failed to acquire lock on settings state".to_string(),
250+
).to_json()
251+
})?;
217252
settings_state
218253
.reset_settings()
219254
.and_then(|updated| {

src-tauri/src/commands/sftp_file_system_operation_commands.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,10 @@ pub fn copy_directory_sftp(
266266
let entries = sftp.readdir(&source_path).map_err(|e| e.to_string())?;
267267

268268
for (path, stat) in entries {
269-
let new_path = format!("{}/{}", destination_path, path.file_name().unwrap().to_str().unwrap());
269+
let file_name = path.file_name()
270+
.and_then(|name| name.to_str())
271+
.unwrap_or("[invalid_filename]");
272+
let new_path = format!("{}/{}", destination_path, file_name);
270273

271274
if stat.is_file() {
272275
// Copy file
@@ -278,7 +281,8 @@ pub fn copy_directory_sftp(
278281
destination_file.write_all(&buffer).map_err(|e| e.to_string())?;
279282
} else if stat.is_dir() {
280283
// Recursively copy directory
281-
copy_directory_sftp(host.clone(), port, username.clone(), password.clone(), path.to_str().unwrap().to_string(), new_path)?;
284+
let path_str = path.to_str().unwrap_or("[invalid_path]").to_string();
285+
copy_directory_sftp(host.clone(), port, username.clone(), password.clone(), path_str, new_path)?;
282286
}
283287
}
284288

src-tauri/src/commands/template_commands.rs

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,11 @@ pub async fn add_template_impl(
105105

106106
// Extract what we need from the metadata state before any await points
107107
let dest_path = {
108-
let metadata_state = state.lock().unwrap();
108+
let metadata_state = state.lock().map_err(|e| {
109+
let error_msg = format!("Error acquiring lock on metadata state: {:?}", e);
110+
log_error!(error_msg.as_str());
111+
error_msg
112+
})?;
109113
let inner_metadata = metadata_state.0.lock().map_err(|e| {
110114
let error_msg = format!("Error acquiring lock on metadata state: {:?}", e);
111115
log_error!(error_msg.as_str());
@@ -128,7 +132,12 @@ pub async fn add_template_impl(
128132
}
129133

130134
// Copy the template using our helper function
131-
let size = copy_to_dest_path(template_path, dest_path.to_str().unwrap()).await?;
135+
let dest_path_str = dest_path.to_str().ok_or_else(|| {
136+
let error_msg = "Invalid destination path encoding".to_string();
137+
log_error!(error_msg.as_str());
138+
error_msg
139+
})?;
140+
let size = copy_to_dest_path(template_path, dest_path_str).await?;
132141

133142
// Update the template paths in the metadata state
134143
let update_result = {

src-tauri/src/error_handling.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,9 @@ impl Error {
5757
}
5858
}
5959
pub fn to_json(&self) -> String {
60-
serde_json::to_string(self).unwrap()
60+
serde_json::to_string(self).unwrap_or_else(|_| {
61+
r#"{"code":500,"message_from_code":"InternalError","custom_message":"Failed to serialize error"}"#.to_string()
62+
})
6163
}
6264
}
6365

src-tauri/src/search_engine/fast_fuzzy_v2.rs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -614,10 +614,9 @@ impl PathMatcher {
614614
let variation_index = variation_idx as f32 / variations.len() as f32;
615615
let mut score = 0.9 - (variation_index * 0.2);
616616
// Bonus for matching first char
617-
if !query_lower.is_empty() && !filename_lower.is_empty() {
618-
if query_lower.chars().next().unwrap()
619-
== filename_lower.chars().next().unwrap()
620-
{
617+
if let (Some(query_first), Some(filename_first)) =
618+
(query_lower.chars().next(), filename_lower.chars().next()) {
619+
if query_first == filename_first {
621620
score += 0.3;
622621
}
623622
}

0 commit comments

Comments
 (0)