diff --git a/app/db/models.py b/app/db/models.py index 4be39bd..6c66abb 100644 --- a/app/db/models.py +++ b/app/db/models.py @@ -79,6 +79,16 @@ class Activity(Base): updated_at: Mapped[datetime] = mapped_column(DateTime(timezone=True), default=utcnow, onupdate=utcnow) +class SystemLogEntry(Base): + __tablename__ = "system_log_entries" + + id: Mapped[int] = mapped_column(Integer, primary_key=True) + created_at: Mapped[datetime] = mapped_column(DateTime(timezone=True), default=utcnow, index=True) + source: Mapped[str] = mapped_column(String(64), nullable=False) + message: Mapped[str] = mapped_column(Text, nullable=False) + user_id: Mapped[int | None] = mapped_column(ForeignKey("sync_users.id", ondelete="SET NULL")) + + class SyncRun(Base): __tablename__ = "sync_runs" diff --git a/app/db/repositories.py b/app/db/repositories.py index 254d5b0..2535299 100644 --- a/app/db/repositories.py +++ b/app/db/repositories.py @@ -5,7 +5,7 @@ from sqlalchemy import and_, or_, select from sqlalchemy.exc import IntegrityError from sqlalchemy.orm import Session -from app.db.models import Activity, ActivityStatus, SyncRun, SyncRunStatus, SyncUser, utcnow +from app.db.models import Activity, ActivityStatus, SyncRun, SyncRunStatus, SystemLogEntry, SyncUser, utcnow @dataclass(frozen=True) @@ -191,6 +191,26 @@ class ActivityRepository: ) +class SystemLogRepository: + def __init__(self, session: Session) -> None: + self.session = session + + def add(self, *, source: str, message: str, user_id: int | None = None) -> SystemLogEntry: + entry = SystemLogEntry(source=source, message=message[:2000], user_id=user_id) + self.session.add(entry) + self.session.commit() + return entry + + def list_recent(self, limit: int = 50) -> list[SystemLogEntry]: + return list( + self.session.scalars( + select(SystemLogEntry) + .order_by(SystemLogEntry.created_at.desc(), SystemLogEntry.id.desc()) + .limit(limit) + ) + ) + + class SyncRunRepository: def __init__(self, session: Session) -> None: self.session = session diff --git a/app/sync/manager.py b/app/sync/manager.py index 264c7ed..304d577 100644 --- a/app/sync/manager.py +++ b/app/sync/manager.py @@ -6,7 +6,7 @@ from pathlib import Path from typing import Any, Callable from app.db.models import ActivityStatus, HealthState, SyncRunStatus -from app.db.repositories import ActivityRepository, SyncRunRepository, UserRepository +from app.db.repositories import ActivityRepository, SyncRunRepository, SystemLogRepository, UserRepository from app.fit.rewriter import FitFormatError from app.garmin.uploader import ( GarminAuthError, @@ -76,7 +76,7 @@ class SyncManager: self._locks: dict[int, asyncio.Lock] = {} self._locks_guard = asyncio.Lock() - def _notify_action_required(self, user: Any, message: str | None) -> None: + def _notify_action_required(self, session: Any, user: Any, message: str | None) -> None: if self.notifier is None or not user.notify_email_enabled or not user.notification_email: return try: @@ -85,10 +85,21 @@ class SyncManager: subject=f"MyWhoosh-Garmin Sync: action required for {user.name}", body=message or "Your sync requires attention. Check the dashboard for details.", ) - except Exception: + except Exception as exc: logger.warning( "sync_user: failed to send action-required notification for user %s", user.id, exc_info=True ) + try: + SystemLogRepository(session).add( + source="email_notification", + message=( + f"Failed to email {user.notification_email} for user {user.id} ({user.name}): " + f"{type(exc).__name__}: {exc}" + ), + user_id=user.id, + ) + except Exception: + logger.warning("sync_user: failed to record email-notification failure in system log", exc_info=True) async def _lock_for(self, user_id: int) -> asyncio.Lock: async with self._locks_guard: @@ -389,7 +400,7 @@ class SyncManager: ) session.commit() if user.action_reason is not None and user.action_reason != previous_action_reason: - self._notify_action_required(user, summary_error) + self._notify_action_required(session, user, summary_error) return SyncOutcome( user_id=user.id, status=status.value, diff --git a/app/web/operations.py b/app/web/operations.py index c4433a4..507ba2c 100644 --- a/app/web/operations.py +++ b/app/web/operations.py @@ -5,7 +5,7 @@ from sqlalchemy import func, select from app.auth.admin import require_admin from app.auth.csrf import ensure_csrf_token, validate_csrf from app.db.models import Activity -from app.db.repositories import ActivityRepository, UserRepository +from app.db.repositories import ActivityRepository, SystemLogRepository, UserRepository from app.sync.manager import SyncAlreadyRunning from app.web.routes import templates @@ -103,6 +103,7 @@ def system_page(request: Request): with request.app.state.session_factory() as session: user_count = len(UserRepository(session).list_all()) activity_count = session.scalar(select(func.count()).select_from(Activity)) or 0 + log_entries = SystemLogRepository(session).list_recent(limit=50) return templates.TemplateResponse(request, "system.html", { "csrf_token": ensure_csrf_token(request), "app_version": APP_VERSION, @@ -111,4 +112,5 @@ def system_page(request: Request): "next_tick": scheduler.next_tick, "user_count": user_count, "activity_count": activity_count, + "log_entries": log_entries, }) diff --git a/app/web/templates/system.html b/app/web/templates/system.html index 945ea83..94442ff 100644 --- a/app/web/templates/system.html +++ b/app/web/templates/system.html @@ -31,4 +31,30 @@ + +

System log

+
+ {% if log_entries %} + + + + + + + + + + {% for entry in log_entries %} + + + + + + {% endfor %} + +
TimeSourceMessage
{{ entry.created_at }}{{ entry.source }}{{ entry.message }}
+ {% else %} +

No system log entries.

+ {% endif %} +
{% endblock %} diff --git a/tests/conftest.py b/tests/conftest.py index 9d1ed73..686b514 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -9,7 +9,7 @@ from sqlalchemy.pool import StaticPool from app.config import Settings from app.db.models import Activity, Base, HealthState -from app.db.repositories import ActivityRepository, SyncRunRepository, UserRepository +from app.db.repositories import ActivityRepository, SyncRunRepository, SystemLogRepository, UserRepository from app.main import create_app @@ -44,6 +44,11 @@ def sync_run_repository(db_session: Session) -> SyncRunRepository: return SyncRunRepository(db_session) +@pytest.fixture +def system_log_repository(db_session: Session) -> SystemLogRepository: + return SystemLogRepository(db_session) + + @pytest.fixture def app(tmp_path: Path): settings = Settings( diff --git a/tests/db/test_repositories.py b/tests/db/test_repositories.py index 130deec..9d4d67e 100644 --- a/tests/db/test_repositories.py +++ b/tests/db/test_repositories.py @@ -64,3 +64,32 @@ def test_activity_external_id_is_unique_per_user(user_repository, activity_repos assert inserted_again is False assert created.id == same.id assert same.status == ActivityStatus.DISCOVERED + + +def test_system_log_lists_most_recent_first(system_log_repository, user_repository) -> None: + user = user_repository.create( + name="Max", + enabled=True, + health_state=HealthState.HEALTHY, + mywhoosh_email_enc="mw-1", + mywhoosh_password_enc="mw-pw-1", + garmin_email_enc="g-1", + garmin_password_enc="g-pw-1", + ) + system_log_repository.add(source="email_notification", message="first failure", user_id=user.id) + system_log_repository.add(source="email_notification", message="second failure", user_id=user.id) + + entries = system_log_repository.list_recent() + + assert [entry.message for entry in entries] == ["second failure", "first failure"] + assert entries[0].source == "email_notification" + assert entries[0].user_id == user.id + + +def test_system_log_respects_limit(system_log_repository) -> None: + for i in range(5): + system_log_repository.add(source="test", message=f"entry {i}") + + entries = system_log_repository.list_recent(limit=2) + + assert len(entries) == 2 diff --git a/tests/sync/test_manager.py b/tests/sync/test_manager.py index 8508e7d..c9d694f 100644 --- a/tests/sync/test_manager.py +++ b/tests/sync/test_manager.py @@ -4,7 +4,7 @@ import pytest from sqlalchemy import select from app.db.models import ActivityStatus, HealthState, SyncRun, SyncRunStatus, SyncUser -from app.db.repositories import UserRepository +from app.db.repositories import SystemLogRepository, UserRepository from app.garmin.uploader import UploadResult from app.mywhoosh.client import MyWhooshDeviceConflictError from app.mywhoosh.models import MyWhooshActivity @@ -305,6 +305,44 @@ async def test_does_not_renotify_for_unresolved_unchanged_reason(session_factory assert len(notifier.sent) == 1 +class FailingNotifier: + def send(self, *, to_address: str, subject: str, body: str) -> None: + raise RuntimeError("SMTP connection refused") + + +@pytest.mark.asyncio +async def test_email_send_failure_is_recorded_in_system_log_and_does_not_break_sync( + session_factory, cipher, settings +) -> None: + with session_factory() as session: + user = _create_user(session, cipher) + user.notify_email_enabled = True + user.notification_email = "alerts@example.com" + session.commit() + user_id = user.id + + manager = SyncManager( + session_factory=session_factory, + credential_cipher=cipher, + settings=settings, + mywhoosh_factory=lambda token_store: DeviceConflictMyWhooshClient(), + garmin_factory=lambda email, password, tokenstore: FakeGarminUploader(), + fit_converter=FakeFitConverter(), + notifier=FailingNotifier(), + ) + + outcome = await manager.sync_user(user_id) + assert outcome.status == "failed" + + with session_factory() as session: + entries = SystemLogRepository(session).list_recent() + assert len(entries) == 1 + assert entries[0].source == "email_notification" + assert entries[0].user_id == user_id + assert "SMTP connection refused" in entries[0].message + assert "alerts@example.com" in entries[0].message + + @pytest.mark.asyncio async def test_mfa_code_reaches_real_garmin_uploader_via_sync_manager( session_factory, cipher, settings, seeded_user: SyncUser diff --git a/tests/web/test_operations.py b/tests/web/test_operations.py index 5f14c5f..1e3f527 100644 --- a/tests/web/test_operations.py +++ b/tests/web/test_operations.py @@ -1,5 +1,7 @@ from fastapi.testclient import TestClient +from app.db.repositories import SystemLogRepository + def test_manual_sync_calls_shared_manager(authenticated_client, fake_sync_manager) -> None: response = authenticated_client.post( @@ -60,3 +62,30 @@ def test_system_page_shows_scheduler_state(app, authenticated_client) -> None: assert "1.0.0" in response.text assert "5" in response.text # sync_interval_minutes assert "0" in response.text # user_count / activity_count fresh DB + + +class _FakeScheduler: + def __init__(self) -> None: + self.last_tick = None + self.next_tick = None + + +def test_system_page_shows_empty_log_state(app, authenticated_client) -> None: + app.state.scheduler = _FakeScheduler() + + response = authenticated_client.get("/system") + assert response.status_code == 200 + assert "No system log entries" in response.text + + +def test_system_page_shows_recorded_log_entries(app, authenticated_client) -> None: + app.state.scheduler = _FakeScheduler() + with app.state.session_factory() as session: + SystemLogRepository(session).add( + source="email_notification", message="Failed to email alerts@example.com: SMTP timeout" + ) + + response = authenticated_client.get("/system") + assert response.status_code == 200 + assert "email_notification" in response.text + assert "SMTP timeout" in response.text