From 72e2365f07259c720628b3d66332f3e3dcf10c6d Mon Sep 17 00:00:00 2001
From: Bhekani Khumalo <bhekanik@gmail.com>
Date: Wed, 10 Jun 2026 21:15:27 +0100
Subject: [PATCH] refactor: log best-effort cleanup failures instead of
 swallowing them
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Three post-success cleanup calls in the mutation handler discarded their
errors with `let _ = …`: deleting a spent or expired undo entry, and
deleting a draft after it sends. None should abort the caller, but a
silent failure hides real state drift — a sent draft lingering in the
list, or an undo id that stays replayable until it expires.

Add a small `log_non_fatal` helper and route the three through it so the
failures surface as warnings with context instead of vanishing.
---
 crates/daemon/src/handler/mutations.rs | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/crates/daemon/src/handler/mutations.rs b/crates/daemon/src/handler/mutations.rs
index 36d7f60d..86fd3f2b 100644
--- a/crates/daemon/src/handler/mutations.rs
+++ b/crates/daemon/src/handler/mutations.rs
@@ -996,6 +996,16 @@ fn undoable_kind(cmd: &MutationCommand) -> Option<UndoableMutationKind> {
     }
 }
 
+/// Log a non-fatal error rather than silently discarding it. For
+/// best-effort cleanup — dropping a spent undo entry, removing a draft
+/// after it sends — where a failure must not abort the caller but
+/// shouldn't vanish without a trace either.
+fn log_non_fatal<T, E: std::fmt::Display>(context: &str, result: Result<T, E>) {
+    if let Err(error) = result {
+        tracing::warn!(%error, "{context}");
+    }
+}
+
 /// Reverse a recent undoable mutation by id.
 ///
 /// Restores both local state (label memberships and read flag) and
@@ -1013,7 +1023,10 @@ pub(super) async fn undo_mutation(state: &AppState, mutation_id: &str) -> Handle
     };
     let now = chrono::Utc::now().timestamp();
     if entry.expires_at <= now {
-        let _ = state.store.delete_undo_entry(&entry.mutation_id).await;
+        log_non_fatal(
+            "undo: failed to delete expired undo entry",
+            state.store.delete_undo_entry(&entry.mutation_id).await,
+        );
         return Err(format!("undo: window expired for mutation `{mutation_id}`").into());
     }
 
@@ -1060,7 +1073,10 @@ pub(super) async fn undo_mutation(state: &AppState, mutation_id: &str) -> Handle
     }
 
     // Successful undo: drop the entry so the same id can't be replayed.
-    let _ = state.store.delete_undo_entry(&entry.mutation_id).await;
+    log_non_fatal(
+        "undo: failed to delete spent undo entry (the id could be replayed until it expires)",
+        state.store.delete_undo_entry(&entry.mutation_id).await,
+    );
     Ok(ResponseData::Ack)
 }
 
@@ -2060,7 +2076,10 @@ pub(crate) async fn send_stored_draft(
         )
         .await
         .map_err(|e| format!("send succeeded but receipt persistence failed: {e}"))?;
-    let _ = state.store.delete_draft(draft_id).await;
+    log_non_fatal(
+        "send: failed to delete draft after a successful send (it may linger in the drafts list)",
+        state.store.delete_draft(draft_id).await,
+    );
     Ok(ResponseData::SendReceipt {
         local_message_id,
         provider_message_id: receipt.provider_message_id,