From 3b2cb12272f1593aba9c1964fd41157cd0053bc5 Mon Sep 17 00:00:00 2001 From: Tristan Michael Date: Thu, 2 Apr 2026 08:23:20 -0700 Subject: [PATCH] fix: replace panicking unwrap/expect calls with graceful error handling Replace .expect() in config path resolution with a fallback default. Replace all .lock().unwrap() on Tauri AppState mutex with a helper that converts poisoned locks into error strings. Replace .expect() on Android app data dir with proper error propagation. Handle LAST_WRITE and WATCHER global mutex locks gracefully. Propagate subtask cascade delete errors instead of silently ignoring them. --- apps/tauri/src-tauri/src/lib.rs | 69 ++++++++++++++++++++------------- crates/onyx-core/src/config.rs | 6 +-- 2 files changed, 45 insertions(+), 30 deletions(-) diff --git a/apps/tauri/src-tauri/src/lib.rs b/apps/tauri/src-tauri/src/lib.rs index dc02145..b94fd14 100644 --- a/apps/tauri/src-tauri/src/lib.rs +++ b/apps/tauri/src-tauri/src/lib.rs @@ -34,6 +34,11 @@ struct AppState { repo: Option, } +/// Lock the AppState mutex, converting poisoned locks into an error string. +fn lock_state(state: &Mutex) -> Result, String> { + state.lock().map_err(|e| format!("State lock poisoned: {}", e)) +} + /// Serializable sync result for the frontend. #[derive(Debug, Serialize, Deserialize, Clone)] struct SyncResult { @@ -61,7 +66,9 @@ impl From for SyncResult { /// Suppress file watcher events for the next second (call before writes). #[cfg(not(target_os = "android"))] fn mute_watcher(_state: &mut AppState) { - *LAST_WRITE.lock().unwrap() = Some(Instant::now()); + if let Ok(mut t) = LAST_WRITE.lock() { + *t = Some(Instant::now()); + } } #[cfg(target_os = "android")] @@ -85,13 +92,13 @@ fn ensure_repo(state: &mut AppState) -> Result<(), String> { #[tauri::command] fn get_config(state: State<'_, Mutex>) -> Result { - let s = state.lock().unwrap(); + let s = lock_state(&state)?; Ok(s.config.clone()) } #[tauri::command] fn save_config(state: State<'_, Mutex>) -> Result<(), String> { - let s = state.lock().unwrap(); + let s = lock_state(&state)?; s.config.save_to_file(&s.config_path).map_err(|e| e.to_string()) } @@ -101,7 +108,7 @@ fn add_workspace( path: String, state: State<'_, Mutex>, ) -> Result<(), String> { - let mut s = state.lock().unwrap(); + let mut s = lock_state(&state)?; let ws = WorkspaceConfig::new(PathBuf::from(&path)); s.config.add_workspace(name.clone(), ws); s.config @@ -119,7 +126,7 @@ fn set_current_workspace( name: String, state: State<'_, Mutex>, ) -> Result<(), String> { - let mut s = state.lock().unwrap(); + let mut s = lock_state(&state)?; s.config .set_current_workspace(name) .map_err(|e| e.to_string())?; @@ -134,7 +141,7 @@ fn remove_workspace( name: String, state: State<'_, Mutex>, ) -> Result<(), String> { - let mut s = state.lock().unwrap(); + let mut s = lock_state(&state)?; s.config.remove_workspace(&name); s.repo = None; s.config @@ -155,7 +162,7 @@ fn init_workspace(path: String) -> Result<(), String> { #[tauri::command] fn get_lists(state: State<'_, Mutex>) -> Result, String> { - let mut s = state.lock().unwrap(); + let mut s = lock_state(&state)?; ensure_repo(&mut s)?; s.repo .as_ref() @@ -169,7 +176,7 @@ fn create_list( name: String, state: State<'_, Mutex>, ) -> Result { - let mut s = state.lock().unwrap(); + let mut s = lock_state(&state)?; ensure_repo(&mut s)?; mute_watcher(&mut s); s.repo @@ -184,7 +191,7 @@ fn delete_list( list_id: String, state: State<'_, Mutex>, ) -> Result<(), String> { - let mut s = state.lock().unwrap(); + let mut s = lock_state(&state)?; ensure_repo(&mut s)?; mute_watcher(&mut s); let id = Uuid::parse_str(&list_id).map_err(|e| e.to_string())?; @@ -202,7 +209,7 @@ fn list_tasks( list_id: String, state: State<'_, Mutex>, ) -> Result, String> { - let mut s = state.lock().unwrap(); + let mut s = lock_state(&state)?; ensure_repo(&mut s)?; let id = Uuid::parse_str(&list_id).map_err(|e| e.to_string())?; s.repo @@ -220,7 +227,7 @@ fn create_task( parent_id: Option, state: State<'_, Mutex>, ) -> Result { - let mut s = state.lock().unwrap(); + let mut s = lock_state(&state)?; ensure_repo(&mut s)?; mute_watcher(&mut s); let id = Uuid::parse_str(&list_id).map_err(|e| e.to_string())?; @@ -245,7 +252,7 @@ fn update_task( task: Task, state: State<'_, Mutex>, ) -> Result<(), String> { - let mut s = state.lock().unwrap(); + let mut s = lock_state(&state)?; ensure_repo(&mut s)?; mute_watcher(&mut s); let id = Uuid::parse_str(&list_id).map_err(|e| e.to_string())?; @@ -262,7 +269,7 @@ fn delete_task( task_id: String, state: State<'_, Mutex>, ) -> Result<(), String> { - let mut s = state.lock().unwrap(); + let mut s = lock_state(&state)?; ensure_repo(&mut s)?; mute_watcher(&mut s); let lid = Uuid::parse_str(&list_id).map_err(|e| e.to_string())?; @@ -276,7 +283,7 @@ fn delete_task( .map(|t| t.id) .collect(); for child_id in child_ids { - let _ = repo.delete_task(lid, child_id); + repo.delete_task(lid, child_id).map_err(|e| format!("Failed to delete subtask {}: {}", child_id, e))?; } repo.delete_task(lid, tid) .map_err(|e| e.to_string()) @@ -288,7 +295,7 @@ fn toggle_task( task_id: String, state: State<'_, Mutex>, ) -> Result { - let mut s = state.lock().unwrap(); + let mut s = lock_state(&state)?; ensure_repo(&mut s)?; mute_watcher(&mut s); let lid = Uuid::parse_str(&list_id).map_err(|e| e.to_string())?; @@ -322,7 +329,7 @@ fn reorder_task( new_position: usize, state: State<'_, Mutex>, ) -> Result<(), String> { - let mut s = state.lock().unwrap(); + let mut s = lock_state(&state)?; ensure_repo(&mut s)?; mute_watcher(&mut s); let lid = Uuid::parse_str(&list_id).map_err(|e| e.to_string())?; @@ -343,7 +350,7 @@ fn move_task( task_id: String, state: State<'_, Mutex>, ) -> Result<(), String> { - let mut s = state.lock().unwrap(); + let mut s = lock_state(&state)?; ensure_repo(&mut s)?; mute_watcher(&mut s); let from = Uuid::parse_str(&from_list_id).map_err(|e| e.to_string())?; @@ -362,7 +369,7 @@ fn rename_list( new_name: String, state: State<'_, Mutex>, ) -> Result<(), String> { - let mut s = state.lock().unwrap(); + let mut s = lock_state(&state)?; ensure_repo(&mut s)?; mute_watcher(&mut s); let id = Uuid::parse_str(&list_id).map_err(|e| e.to_string())?; @@ -379,7 +386,7 @@ fn set_group_by_due_date( enabled: bool, state: State<'_, Mutex>, ) -> Result<(), String> { - let mut s = state.lock().unwrap(); + let mut s = lock_state(&state)?; ensure_repo(&mut s)?; mute_watcher(&mut s); let id = Uuid::parse_str(&list_id).map_err(|e| e.to_string())?; @@ -395,7 +402,7 @@ fn get_group_by_due_date( list_id: String, state: State<'_, Mutex>, ) -> Result { - let mut s = state.lock().unwrap(); + let mut s = lock_state(&state)?; ensure_repo(&mut s)?; let id = Uuid::parse_str(&list_id).map_err(|e| e.to_string())?; s.repo @@ -413,7 +420,7 @@ fn set_webdav_config( webdav_url: String, state: State<'_, Mutex>, ) -> Result<(), String> { - let mut s = state.lock().unwrap(); + let mut s = lock_state(&state)?; if let Some(ws) = s.config.workspaces.get_mut(&workspace_name) { ws.webdav_url = Some(webdav_url); } @@ -477,7 +484,7 @@ async fn sync_workspace( // Persist last_sync timestamp to config { - let mut s = state.lock().unwrap(); + let mut s = lock_state(&state)?; if let Some(ws) = s.config.workspaces.get_mut(&workspace_name) { ws.last_sync = Some(Utc::now()); } @@ -504,16 +511,22 @@ fn start_watcher(handle: tauri::AppHandle, path: PathBuf) { }); if !has_data_change { return; } // Skip if we wrote recently (self-change suppression) - if let Some(t) = *LAST_WRITE.lock().unwrap() { - if t.elapsed() < std::time::Duration::from_secs(1) { return; } + if let Ok(guard) = LAST_WRITE.lock() { + if let Some(t) = *guard { + if t.elapsed() < std::time::Duration::from_secs(1) { return; } + } } let _ = handle.emit("fs-changed", ()); }, ); match debouncer { Ok(mut d) => { - let _ = d.watcher().watch(&path, notify::RecursiveMode::Recursive); - *WATCHER.lock().unwrap() = Some(d); + if let Err(e) = d.watcher().watch(&path, notify::RecursiveMode::Recursive) { + eprintln!("Failed to watch path {}: {e}", path.display()); + } + if let Ok(mut w) = WATCHER.lock() { + *w = Some(d); + } } Err(e) => eprintln!("Failed to start file watcher: {e}"), } @@ -545,7 +558,9 @@ pub fn run() { #[cfg(target_os = "android")] { use tauri::Manager; - app.path().app_data_dir().expect("Failed to get app data dir").join("config.json") + app.path().app_data_dir() + .map_err(|e| format!("Failed to get app data dir: {}", e))? + .join("config.json") } #[cfg(not(target_os = "android"))] { diff --git a/crates/onyx-core/src/config.rs b/crates/onyx-core/src/config.rs index 9667fe3..b13a2bd 100644 --- a/crates/onyx-core/src/config.rs +++ b/crates/onyx-core/src/config.rs @@ -82,9 +82,9 @@ impl AppConfig { } pub fn get_config_path() -> PathBuf { - let config_dir = directories::ProjectDirs::from("", "", "onyx") - .expect("Failed to determine config directory"); - config_dir.config_dir().join("config.json") + directories::ProjectDirs::from("", "", "onyx") + .map(|dirs| dirs.config_dir().join("config.json")) + .unwrap_or_else(|| PathBuf::from("onyx-config.json")) } }