refactor: log best-effort cleanup failures instead of swallowing them#78
Conversation
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.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR introduces a ChangesNon-fatal cleanup logging
🎯 2 (Simple) | ⏱️ ~5 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
What
Routes three best-effort cleanup calls in the mutation handler through a new
log_non_fatalhelper instead of discarding their errors withlet _ = …. Audit backlog item P2 #19 (swallowed-error sweep). Offmain.Why
A silent cleanup failure hides real state drift:
mxr drafts list;None of these should abort the caller, but they shouldn't vanish either.
How
A small
log_non_fatal(context, result)helper logsErras awarnwith context and otherwise does nothing. Applied to the threedelete_undo_entry/delete_draftcleanups.Scope note (honest)
The audit framed this as a broad sweep, but a survey of
let _ = …awaitacross the daemon found only these three are store writes whose failure is meaningful — the rest are genuine fire-and-forget (channel sends, broadcast, notify) that are correct to ignore. So the change is deliberately narrow rather than a blanket rewrite.Verification
mxrdaemon undo/mutation tests (95) pass; clippy + fmt clean. No user-facing contract change (internal observability), so no docs update.Generated with Claude Code
Summary by cubic
Log best-effort cleanup failures in the mutation handler instead of swallowing them. Warnings now surface when deleting expired/spent undo entries or sent drafts fails, preventing silent state drift (lingering drafts or replayable undo IDs) while keeping these errors non-fatal.
Written for commit 72e2365. Summary will update on new commits.
Summary by CodeRabbit