diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 5d448e9..176bc52 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -156,6 +156,12 @@ configured to write LF line endings so formatting is consistent across Windows and Linux. Fix reported lint problems in the code rather than weakening the shared rule set. +Catch the narrowest expected exception in normal application code. A broad +exception boundary is acceptable only where SchedPlus must keep a background +service alive or provide a last-resort startup error report; those boundaries +must log or report the failure and explain why the broad catch is intentional. +Never silently discard an exception. + --- ## Licensing Contributions diff --git a/src/logic/reminder_service.py b/src/logic/reminder_service.py index bee51e0..23a0e4d 100644 --- a/src/logic/reminder_service.py +++ b/src/logic/reminder_service.py @@ -1,5 +1,6 @@ """Background reminder service that checks for upcoming tasks and sends notifications.""" +import logging import subprocess import sys import threading @@ -12,6 +13,8 @@ from . import local_time +LOGGER = logging.getLogger(__name__) + class ReminderService: """Polls for tasks due soon and sends native notifications.""" @@ -46,7 +49,7 @@ def _run(self) -> None: try: self._check_tasks() except Exception: - pass + LOGGER.exception("Reminder check failed") time.sleep(self._poll_interval) def _check_tasks(self) -> None: @@ -95,8 +98,8 @@ def _send_notification(self, title: str, date: str, time: str) -> None: windll.user32.MessageBoxW(0, body, f"Reminder: {title}", 0x40 | 0x1000) return - except Exception: - pass + except (AttributeError, ImportError, OSError): + LOGGER.exception("Windows reminder notification failed") if sys.platform == "darwin": try: script = f'display notification "{self._escape_applescript(body)}" with title "{self._escape_applescript(title)}"' diff --git a/src/logic/undo_manager.py b/src/logic/undo_manager.py index 42a4813..386b488 100644 --- a/src/logic/undo_manager.py +++ b/src/logic/undo_manager.py @@ -1,11 +1,14 @@ """Undo manager for task actions with a bounded history stack.""" +import logging from dataclasses import dataclass from typing import TYPE_CHECKING if TYPE_CHECKING: from .scheduler import Scheduler, Task +LOGGER = logging.getLogger(__name__) + @dataclass class UndoAction: @@ -67,7 +70,8 @@ def undo(self) -> str | None: self._scheduler.complete_task(action.task_id) return "Undid uncomplete" except Exception: - pass + self._history.append(action) + LOGGER.exception("Unable to undo %s action", action.action_type) return None def _push(self, action: UndoAction) -> None: diff --git a/src/startup/controller.py b/src/startup/controller.py index 0e43a05..d62d054 100644 --- a/src/startup/controller.py +++ b/src/startup/controller.py @@ -108,7 +108,7 @@ def _launch_mode(mode: StartupMode, arguments: list[str] | None = None): if mode == StartupMode.TK: try: from ui.tkinter_ui import run_ui - except Exception: + except ImportError: print("Tkinter UI is not available on this system.") return 1 @@ -119,7 +119,7 @@ def _launch_mode(mode: StartupMode, arguments: list[str] | None = None): elif mode == StartupMode.PYQT: try: from ui.pyqt_ui import run_pyqt_ui - except Exception as exc: + except ImportError as exc: print(f"PyQt UI is not available on this system: {exc}", file=sys.stderr) return 1 @@ -151,7 +151,7 @@ def _report_storage_error(mode: StartupMode, error: StorageError) -> None: messagebox.showerror("SchedPlus database error", str(error), parent=root) root.destroy() return - except Exception as reporter_exc: + except Exception as reporter_exc: # noqa: BLE001 - last-resort error reporter print( f"Unable to display the Tkinter database error: {reporter_exc}", file=sys.stderr, @@ -164,7 +164,7 @@ def _report_storage_error(mode: StartupMode, error: StorageError) -> None: QMessageBox.critical(None, "SchedPlus database error", str(error)) app.quit() return - except Exception as reporter_exc: + except Exception as reporter_exc: # noqa: BLE001 - last-resort error reporter print( f"Unable to display the PyQt database error: {reporter_exc}", file=sys.stderr, diff --git a/src/startup/selector.py b/src/startup/selector.py index aac968d..ef62c6c 100644 --- a/src/startup/selector.py +++ b/src/startup/selector.py @@ -13,7 +13,7 @@ try: import tkinter as tk from tkinter import ttk -except Exception: +except ImportError: tk = None ttk = None diff --git a/tests/test_reminder_service.py b/tests/test_reminder_service.py index e99a805..83cd649 100644 --- a/tests/test_reminder_service.py +++ b/tests/test_reminder_service.py @@ -107,7 +107,7 @@ def worker_add(): with service._lock: service._notified.add(f"task-{i}") time.sleep(0.0001) - except Exception as e: + except Exception as e: # noqa: BLE001 - surface worker failures to the test errors.append(e) def worker_remove(): @@ -116,7 +116,7 @@ def worker_remove(): with service._lock: service._notified.discard(f"task-{i}") time.sleep(0.0001) - except Exception as e: + except Exception as e: # noqa: BLE001 - surface worker failures to the test errors.append(e) threads = [threading.Thread(target=worker_add) for _ in range(3)] + [ @@ -165,10 +165,11 @@ def test_send_notification_linux_calls_notify_send(mock_run): @patch("logic.reminder_service.subprocess.run") def test_send_notification_windows_calls_message_box(mock_run): - with patch("sys.platform", "win32"): + with patch("sys.platform", "win32"), patch("ctypes.windll", create=True) as windll: service = ReminderService(MemoryScheduler()) service._send_notification("Title", "2026-08-26", "12:00") mock_run.assert_not_called() # uses ctypes directly + windll.user32.MessageBoxW.assert_called_once() def test_check_tasks_skips_completed_and_no_reminder(): diff --git a/tests/test_undo_manager.py b/tests/test_undo_manager.py new file mode 100644 index 0000000..1fa25fa --- /dev/null +++ b/tests/test_undo_manager.py @@ -0,0 +1,35 @@ +from logic.undo_manager import UndoManager + + +class RecordingScheduler: + def __init__(self): + self.deleted = [] + + def delete_task(self, task_id): + self.deleted.append(task_id) + + +class FailingScheduler: + def delete_task(self, _task_id): + raise RuntimeError("storage unavailable") + + +def test_successful_undo_consumes_action(): + scheduler = RecordingScheduler() + manager = UndoManager(scheduler) + manager.record_add("task-1") + + assert manager.undo() == "Undid add" + assert scheduler.deleted == ["task-1"] + assert not manager.can_undo() + + +def test_failed_undo_is_logged_and_remains_available(caplog): + manager = UndoManager(FailingScheduler()) + manager.record_add("task-1") + + with caplog.at_level("ERROR"): + assert manager.undo() is None + + assert manager.can_undo() + assert "Unable to undo add action" in caplog.text