diff --git a/freedays/freedays/_sync.py b/freedays/freedays/_sync.py index 72d5b09..35a4f82 100644 --- a/freedays/freedays/_sync.py +++ b/freedays/freedays/_sync.py @@ -115,14 +115,29 @@ def sync( def sync_quietly(*, paths: Paths | None = None) -> bool: - """Sync, swallowing unavailability. Returns whether it actually ran. + """Sync, swallowing every failure. Returns whether it actually ran. For callers on a timer that want a fresher pool but must not fail, and - must not say anything, if the network is down. + must not say anything, when the sync cannot happen. + + The broad catch is deliberate and is the whole point of this function. + Today ``FirebaseAuthError`` happens to subclass ``RemoteSyncError`` and + ``requests``' ``JSONDecodeError`` happens to subclass + ``RequestException``, so the narrow handler above would in fact catch an + expired refresh token or a malformed payload. But that is an upstream + class hierarchy, not a promise to this package -- and a background sync + whose contract is "never fail" must not have that contract hold only by + coincidence. Anything unexpected is logged with its traceback, so a real + bug is loud in the journal while the timer stays green. """ try: sync(paths=paths) except SyncUnavailableError as exc: + # The expected case -- offline, or not configured yet. Debug level: + # this is normal on a laptop and must not fill the journal. _logger.debug("free-day sync skipped: %s", exc) return False + except Exception: + _logger.exception("free-day sync failed unexpectedly") + return False return True diff --git a/freedays/freedays/tests/test_sync.py b/freedays/freedays/tests/test_sync.py index e1cd60b..604a186 100644 --- a/freedays/freedays/tests/test_sync.py +++ b/freedays/freedays/tests/test_sync.py @@ -159,3 +159,43 @@ def test_the_pool_can_be_pointed_elsewhere_wholesale(tmp_path: Path) -> None: mark(LATER, now=TODAY, paths=paths) sync(client=FakeRemote(), paths=paths) assert is_free_day(LATER, log_path=paths.log, now=TODAY) + + +def test_an_expired_credential_mid_sync_does_not_fail_the_timer( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A refresh token expiring during the tick, not at client construction.""" + + class ExpiringRemote(FakeRemote): + def list_directory(self, path: str) -> list[str]: + msg = f"refresh token rejected while listing {path}" + raise FirebaseAuthError(msg) + + monkeypatch.setattr( + "freedays._sync.firebase_client_for", lambda _app: ExpiringRemote() + ) + assert sync_quietly() is False + + +def test_an_unexpected_error_is_logged_with_a_traceback_and_swallowed( + monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture +) -> None: + """The contract is "never fail", and it must not hold only by coincidence. + + FirebaseAuthError happens to subclass RemoteSyncError today. Something + that does not -- a bug, a changed upstream hierarchy -- must still leave + the timer green rather than turning it red every 15 minutes. + """ + + class BrokenRemote(FakeRemote): + def list_directory(self, path: str) -> list[str]: + msg = f"something nobody predicted, at {path}" + raise RuntimeError(msg) + + monkeypatch.setattr( + "freedays._sync.firebase_client_for", lambda _app: BrokenRemote() + ) + with caplog.at_level("ERROR"): + assert sync_quietly() is False + assert "failed unexpectedly" in caplog.text + assert "RuntimeError" in caplog.text