logging
This commit is contained in:
@@ -79,6 +79,20 @@ class SyncManager:
|
|||||||
def _notify_action_required(self, session: Any, 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:
|
if self.notifier is None or not user.notify_email_enabled or not user.notification_email:
|
||||||
return
|
return
|
||||||
|
# A notifier with no SMTP host configured silently no-ops in send()
|
||||||
|
# rather than raising -- without this check, that silence would look
|
||||||
|
# identical to "everything's fine" in the system log, leaving an
|
||||||
|
# opted-in user with no way to find out why no mail ever arrives.
|
||||||
|
if not getattr(self.notifier, "configured", True):
|
||||||
|
self._record_log(
|
||||||
|
session,
|
||||||
|
(
|
||||||
|
f"Skipped action-required email to {user.notification_email} for user {user.id} "
|
||||||
|
f"({user.name}): SMTP is not configured (SMTP_HOST unset)."
|
||||||
|
),
|
||||||
|
user_id=user.id,
|
||||||
|
)
|
||||||
|
return
|
||||||
try:
|
try:
|
||||||
self.notifier.send(
|
self.notifier.send(
|
||||||
to_address=user.notification_email,
|
to_address=user.notification_email,
|
||||||
@@ -89,17 +103,20 @@ class SyncManager:
|
|||||||
logger.warning(
|
logger.warning(
|
||||||
"sync_user: failed to send action-required notification for user %s", user.id, exc_info=True
|
"sync_user: failed to send action-required notification for user %s", user.id, exc_info=True
|
||||||
)
|
)
|
||||||
try:
|
self._record_log(
|
||||||
SystemLogRepository(session).add(
|
session,
|
||||||
source="email_notification",
|
(
|
||||||
message=(
|
|
||||||
f"Failed to email {user.notification_email} for user {user.id} ({user.name}): "
|
f"Failed to email {user.notification_email} for user {user.id} ({user.name}): "
|
||||||
f"{type(exc).__name__}: {exc}"
|
f"{type(exc).__name__}: {exc}"
|
||||||
),
|
),
|
||||||
user_id=user.id,
|
user_id=user.id,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
def _record_log(self, session: Any, message: str, *, user_id: int | None = None) -> None:
|
||||||
|
try:
|
||||||
|
SystemLogRepository(session).add(source="email_notification", message=message, user_id=user_id)
|
||||||
except Exception:
|
except Exception:
|
||||||
logger.warning("sync_user: failed to record email-notification failure in system log", exc_info=True)
|
logger.warning("sync_user: failed to record email-notification entry in system log", exc_info=True)
|
||||||
|
|
||||||
async def _lock_for(self, user_id: int) -> asyncio.Lock:
|
async def _lock_for(self, user_id: int) -> asyncio.Lock:
|
||||||
async with self._locks_guard:
|
async with self._locks_guard:
|
||||||
|
|||||||
@@ -8,6 +8,7 @@ from app.db.repositories import SystemLogRepository, UserRepository
|
|||||||
from app.garmin.uploader import UploadResult
|
from app.garmin.uploader import UploadResult
|
||||||
from app.mywhoosh.client import MyWhooshDeviceConflictError
|
from app.mywhoosh.client import MyWhooshDeviceConflictError
|
||||||
from app.mywhoosh.models import MyWhooshActivity
|
from app.mywhoosh.models import MyWhooshActivity
|
||||||
|
from app.notifications.emailer import EmailNotifier
|
||||||
from app.sync.manager import SyncManager
|
from app.sync.manager import SyncManager
|
||||||
from tests.sync.conftest import FakeFitConverter, _create_user
|
from tests.sync.conftest import FakeFitConverter, _create_user
|
||||||
from tests.sync.fakes import FakeGarminUploader, FakeMyWhooshClient, FakeNotifier
|
from tests.sync.fakes import FakeGarminUploader, FakeMyWhooshClient, FakeNotifier
|
||||||
@@ -343,6 +344,44 @@ async def test_email_send_failure_is_recorded_in_system_log_and_does_not_break_s
|
|||||||
assert "alerts@example.com" in entries[0].message
|
assert "alerts@example.com" in entries[0].message
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_unconfigured_smtp_is_recorded_in_system_log_not_silently_dropped(
|
||||||
|
session_factory, cipher, settings
|
||||||
|
) -> None:
|
||||||
|
"""Regression test: an EmailNotifier with no SMTP_HOST configured skips
|
||||||
|
sending without raising, which used to look identical to "notifications
|
||||||
|
disabled" -- an opted-in user got neither an email nor any trace of why,
|
||||||
|
with nothing to debug from. This must now leave a system log entry."""
|
||||||
|
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
|
||||||
|
|
||||||
|
unconfigured_notifier = EmailNotifier(
|
||||||
|
host=None, port=587, username=None, password=None, from_address=None, use_tls=True
|
||||||
|
)
|
||||||
|
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=unconfigured_notifier,
|
||||||
|
)
|
||||||
|
|
||||||
|
await manager.sync_user(user_id)
|
||||||
|
|
||||||
|
with session_factory() as session:
|
||||||
|
entries = SystemLogRepository(session).list_recent()
|
||||||
|
assert len(entries) == 1
|
||||||
|
assert entries[0].source == "email_notification"
|
||||||
|
assert "SMTP is not configured" in entries[0].message
|
||||||
|
assert "alerts@example.com" in entries[0].message
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_mfa_code_reaches_real_garmin_uploader_via_sync_manager(
|
async def test_mfa_code_reaches_real_garmin_uploader_via_sync_manager(
|
||||||
session_factory, cipher, settings, seeded_user: SyncUser
|
session_factory, cipher, settings, seeded_user: SyncUser
|
||||||
|
|||||||
Reference in New Issue
Block a user