SteelDynamite/onyx-tasks
1
0
Fork 0
Code Issues Pull requests Actions Packages Projects Releases Wiki Activity

fix: replace panicking unwrap/expect calls with graceful error handling

Browse source 
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.
This commit is contained in:
Tristan Michael 2026-04-02 08:23:20 -07:00
parent 68f1bff93b
commit 3b2cb12272
2 changed files with 45 additions and 30 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

67
apps/tauri/src-tauri/src/lib.rs
View file

@ -34,6 +34,11 @@ struct AppState {
repo: Option<TaskRepository>, repo: Option<TaskRepository>,
} }
/// Lock the AppState mutex, converting poisoned locks into an error string.
fn lock_state(state: &Mutex<AppState>) -> Result<std::sync::MutexGuard<'_, AppState>, String> {
state.lock().map_err(|e| format!("State lock poisoned: {}", e))
}
/// Serializable sync result for the frontend. /// Serializable sync result for the frontend.
#[derive(Debug, Serialize, Deserialize, Clone)] #[derive(Debug, Serialize, Deserialize, Clone)]
struct SyncResult { struct SyncResult {
@ -61,7 +66,9 @@ impl From<CoreSyncResult> for SyncResult {
/// Suppress file watcher events for the next second (call before writes). /// Suppress file watcher events for the next second (call before writes).
#[cfg(not(target_os = "android"))] #[cfg(not(target_os = "android"))]
fn mute_watcher(_state: &mut AppState) { 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")] #[cfg(target_os = "android")]
@ -85,13 +92,13 @@ fn ensure_repo(state: &mut AppState) -> Result<(), String> {
#[tauri::command] #[tauri::command]
fn get_config(state: State<'_, Mutex<AppState>>) -> Result<AppConfig, String> { fn get_config(state: State<'_, Mutex<AppState>>) -> Result<AppConfig, String> {
let s = state.lock().unwrap(); let s = lock_state(&state)?;
Ok(s.config.clone()) Ok(s.config.clone())
} }
#[tauri::command] #[tauri::command]
fn save_config(state: State<'_, Mutex<AppState>>) -> Result<(), String> { fn save_config(state: State<'_, Mutex<AppState>>) -> 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()) s.config.save_to_file(&s.config_path).map_err(|e| e.to_string())
} }
@ -101,7 +108,7 @@ fn add_workspace(
path: String, path: String,
state: State<'_, Mutex<AppState>>, state: State<'_, Mutex<AppState>>,
) -> Result<(), String> { ) -> Result<(), String> {
let mut s = state.lock().unwrap(); let mut s = lock_state(&state)?;
let ws = WorkspaceConfig::new(PathBuf::from(&path)); let ws = WorkspaceConfig::new(PathBuf::from(&path));
s.config.add_workspace(name.clone(), ws); s.config.add_workspace(name.clone(), ws);
s.config s.config
@ -119,7 +126,7 @@ fn set_current_workspace(
name: String, name: String,
state: State<'_, Mutex<AppState>>, state: State<'_, Mutex<AppState>>,
) -> Result<(), String> { ) -> Result<(), String> {
let mut s = state.lock().unwrap(); let mut s = lock_state(&state)?;
s.config s.config
.set_current_workspace(name) .set_current_workspace(name)
.map_err(|e| e.to_string())?; .map_err(|e| e.to_string())?;
@ -134,7 +141,7 @@ fn remove_workspace(
name: String, name: String,
state: State<'_, Mutex<AppState>>, state: State<'_, Mutex<AppState>>,
) -> Result<(), String> { ) -> Result<(), String> {
let mut s = state.lock().unwrap(); let mut s = lock_state(&state)?;
s.config.remove_workspace(&name); s.config.remove_workspace(&name);
s.repo = None; s.repo = None;
s.config s.config
@ -155,7 +162,7 @@ fn init_workspace(path: String) -> Result<(), String> {
#[tauri::command] #[tauri::command]
fn get_lists(state: State<'_, Mutex<AppState>>) -> Result<Vec<TaskList>, String> { fn get_lists(state: State<'_, Mutex<AppState>>) -> Result<Vec<TaskList>, String> {
let mut s = state.lock().unwrap(); let mut s = lock_state(&state)?;
ensure_repo(&mut s)?; ensure_repo(&mut s)?;
s.repo s.repo
.as_ref() .as_ref()
@ -169,7 +176,7 @@ fn create_list(
name: String, name: String,
state: State<'_, Mutex<AppState>>, state: State<'_, Mutex<AppState>>,
) -> Result<TaskList, String> { ) -> Result<TaskList, String> {
let mut s = state.lock().unwrap(); let mut s = lock_state(&state)?;
ensure_repo(&mut s)?; ensure_repo(&mut s)?;
mute_watcher(&mut s); mute_watcher(&mut s);
s.repo s.repo
@ -184,7 +191,7 @@ fn delete_list(
list_id: String, list_id: String,
state: State<'_, Mutex<AppState>>, state: State<'_, Mutex<AppState>>,
) -> Result<(), String> { ) -> Result<(), String> {
let mut s = state.lock().unwrap(); let mut s = lock_state(&state)?;
ensure_repo(&mut s)?; ensure_repo(&mut s)?;
mute_watcher(&mut s); mute_watcher(&mut s);
let id = Uuid::parse_str(&list_id).map_err(|e| e.to_string())?; let id = Uuid::parse_str(&list_id).map_err(|e| e.to_string())?;
@ -202,7 +209,7 @@ fn list_tasks(
list_id: String, list_id: String,
state: State<'_, Mutex<AppState>>, state: State<'_, Mutex<AppState>>,
) -> Result<Vec<Task>, String> { ) -> Result<Vec<Task>, String> {
let mut s = state.lock().unwrap(); let mut s = lock_state(&state)?;
ensure_repo(&mut s)?; ensure_repo(&mut s)?;
let id = Uuid::parse_str(&list_id).map_err(|e| e.to_string())?; let id = Uuid::parse_str(&list_id).map_err(|e| e.to_string())?;
s.repo s.repo
@ -220,7 +227,7 @@ fn create_task(
parent_id: Option<String>, parent_id: Option<String>,
state: State<'_, Mutex<AppState>>, state: State<'_, Mutex<AppState>>,
) -> Result<Task, String> { ) -> Result<Task, String> {
let mut s = state.lock().unwrap(); let mut s = lock_state(&state)?;
ensure_repo(&mut s)?; ensure_repo(&mut s)?;
mute_watcher(&mut s); mute_watcher(&mut s);
let id = Uuid::parse_str(&list_id).map_err(|e| e.to_string())?; let id = Uuid::parse_str(&list_id).map_err(|e| e.to_string())?;
@ -245,7 +252,7 @@ fn update_task(
task: Task, task: Task,
state: State<'_, Mutex<AppState>>, state: State<'_, Mutex<AppState>>,
) -> Result<(), String> { ) -> Result<(), String> {
let mut s = state.lock().unwrap(); let mut s = lock_state(&state)?;
ensure_repo(&mut s)?; ensure_repo(&mut s)?;
mute_watcher(&mut s); mute_watcher(&mut s);
let id = Uuid::parse_str(&list_id).map_err(|e| e.to_string())?; let id = Uuid::parse_str(&list_id).map_err(|e| e.to_string())?;
@ -262,7 +269,7 @@ fn delete_task(
task_id: String, task_id: String,
state: State<'_, Mutex<AppState>>, state: State<'_, Mutex<AppState>>,
) -> Result<(), String> { ) -> Result<(), String> {
let mut s = state.lock().unwrap(); let mut s = lock_state(&state)?;
ensure_repo(&mut s)?; ensure_repo(&mut s)?;
mute_watcher(&mut s); mute_watcher(&mut s);
let lid = Uuid::parse_str(&list_id).map_err(|e| e.to_string())?; let lid = Uuid::parse_str(&list_id).map_err(|e| e.to_string())?;
@ -276,7 +283,7 @@ fn delete_task(
.map(|t| t.id) .map(|t| t.id)
.collect(); .collect();
for child_id in child_ids { 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) repo.delete_task(lid, tid)
.map_err(|e| e.to_string()) .map_err(|e| e.to_string())
@ -288,7 +295,7 @@ fn toggle_task(
task_id: String, task_id: String,
state: State<'_, Mutex<AppState>>, state: State<'_, Mutex<AppState>>,
) -> Result<Task, String> { ) -> Result<Task, String> {
let mut s = state.lock().unwrap(); let mut s = lock_state(&state)?;
ensure_repo(&mut s)?; ensure_repo(&mut s)?;
mute_watcher(&mut s); mute_watcher(&mut s);
let lid = Uuid::parse_str(&list_id).map_err(|e| e.to_string())?; let lid = Uuid::parse_str(&list_id).map_err(|e| e.to_string())?;
@ -322,7 +329,7 @@ fn reorder_task(
new_position: usize, new_position: usize,
state: State<'_, Mutex<AppState>>, state: State<'_, Mutex<AppState>>,
) -> Result<(), String> { ) -> Result<(), String> {
let mut s = state.lock().unwrap(); let mut s = lock_state(&state)?;
ensure_repo(&mut s)?; ensure_repo(&mut s)?;
mute_watcher(&mut s); mute_watcher(&mut s);
let lid = Uuid::parse_str(&list_id).map_err(|e| e.to_string())?; let lid = Uuid::parse_str(&list_id).map_err(|e| e.to_string())?;
@ -343,7 +350,7 @@ fn move_task(
task_id: String, task_id: String,
state: State<'_, Mutex<AppState>>, state: State<'_, Mutex<AppState>>,
) -> Result<(), String> { ) -> Result<(), String> {
let mut s = state.lock().unwrap(); let mut s = lock_state(&state)?;
ensure_repo(&mut s)?; ensure_repo(&mut s)?;
mute_watcher(&mut s); mute_watcher(&mut s);
let from = Uuid::parse_str(&from_list_id).map_err(|e| e.to_string())?; let from = Uuid::parse_str(&from_list_id).map_err(|e| e.to_string())?;
@ -362,7 +369,7 @@ fn rename_list(
new_name: String, new_name: String,
state: State<'_, Mutex<AppState>>, state: State<'_, Mutex<AppState>>,
) -> Result<(), String> { ) -> Result<(), String> {
let mut s = state.lock().unwrap(); let mut s = lock_state(&state)?;
ensure_repo(&mut s)?; ensure_repo(&mut s)?;
mute_watcher(&mut s); mute_watcher(&mut s);
let id = Uuid::parse_str(&list_id).map_err(|e| e.to_string())?; 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, enabled: bool,
state: State<'_, Mutex<AppState>>, state: State<'_, Mutex<AppState>>,
) -> Result<(), String> { ) -> Result<(), String> {
let mut s = state.lock().unwrap(); let mut s = lock_state(&state)?;
ensure_repo(&mut s)?; ensure_repo(&mut s)?;
mute_watcher(&mut s); mute_watcher(&mut s);
let id = Uuid::parse_str(&list_id).map_err(|e| e.to_string())?; 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, list_id: String,
state: State<'_, Mutex<AppState>>, state: State<'_, Mutex<AppState>>,
) -> Result<bool, String> { ) -> Result<bool, String> {
let mut s = state.lock().unwrap(); let mut s = lock_state(&state)?;
ensure_repo(&mut s)?; ensure_repo(&mut s)?;
let id = Uuid::parse_str(&list_id).map_err(|e| e.to_string())?; let id = Uuid::parse_str(&list_id).map_err(|e| e.to_string())?;
s.repo s.repo
@ -413,7 +420,7 @@ fn set_webdav_config(
webdav_url: String, webdav_url: String,
state: State<'_, Mutex<AppState>>, state: State<'_, Mutex<AppState>>,
) -> Result<(), String> { ) -> 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) { if let Some(ws) = s.config.workspaces.get_mut(&workspace_name) {
ws.webdav_url = Some(webdav_url); ws.webdav_url = Some(webdav_url);
} }
@ -477,7 +484,7 @@ async fn sync_workspace(
// Persist last_sync timestamp to config // 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) { if let Some(ws) = s.config.workspaces.get_mut(&workspace_name) {
ws.last_sync = Some(Utc::now()); ws.last_sync = Some(Utc::now());
} }
@ -504,16 +511,22 @@ fn start_watcher(handle: tauri::AppHandle, path: PathBuf) {
}); });
if !has_data_change { return; } if !has_data_change { return; }
// Skip if we wrote recently (self-change suppression) // Skip if we wrote recently (self-change suppression)
if let Some(t) = *LAST_WRITE.lock().unwrap() { if let Ok(guard) = LAST_WRITE.lock() {
if let Some(t) = *guard {
if t.elapsed() < std::time::Duration::from_secs(1) { return; } if t.elapsed() < std::time::Duration::from_secs(1) { return; }
} }
}
let _ = handle.emit("fs-changed", ()); let _ = handle.emit("fs-changed", ());
}, },
); );
match debouncer { match debouncer {
Ok(mut d) => { Ok(mut d) => {
let _ = d.watcher().watch(&path, notify::RecursiveMode::Recursive); if let Err(e) = d.watcher().watch(&path, notify::RecursiveMode::Recursive) {
*WATCHER.lock().unwrap() = Some(d); 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}"), Err(e) => eprintln!("Failed to start file watcher: {e}"),
} }
@ -545,7 +558,9 @@ pub fn run() {
#[cfg(target_os = "android")] #[cfg(target_os = "android")]
{ {
use tauri::Manager; 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"))] #[cfg(not(target_os = "android"))]
{ {

6
crates/onyx-core/src/config.rs
View file

@ -82,9 +82,9 @@ impl AppConfig {
} }
pub fn get_config_path() -> PathBuf { pub fn get_config_path() -> PathBuf {
let config_dir = directories::ProjectDirs::from("", "", "onyx") directories::ProjectDirs::from("", "", "onyx")
.expect("Failed to determine config directory"); .map(|dirs| dirs.config_dir().join("config.json"))
config_dir.config_dir().join("config.json") .unwrap_or_else(|| PathBuf::from("onyx-config.json"))
} }
} }