mirror of
https://github.com/kuhyx/utils.git
synced 2026-11-10 22:27:13 +01:00
Make sync_quietly's "never fail" contract true by construction
Some checks failed
crdt-sync pre-commit / pre-commit (push) Has been cancelled
crdt-sync tests / test (push) Has been cancelled
dependency freshness / dependencies on newest stable (push) Has been cancelled
file length / file-length (push) Has been cancelled
freedays pre-commit / pre-commit (push) Has been cancelled
freedays tests / test (push) Has been cancelled
md-naming / markdown naming convention (push) Has been cancelled
palette drift / palette (push) Has been cancelled
shared gate tests / gate unit tests (push) Has been cancelled
Some checks failed
crdt-sync pre-commit / pre-commit (push) Has been cancelled
crdt-sync tests / test (push) Has been cancelled
dependency freshness / dependencies on newest stable (push) Has been cancelled
file length / file-length (push) Has been cancelled
freedays pre-commit / pre-commit (push) Has been cancelled
freedays tests / test (push) Has been cancelled
md-naming / markdown naming convention (push) Has been cancelled
palette drift / palette (push) Has been cancelled
shared gate tests / gate unit tests (push) Has been cancelled
sync_quietly exists so a background timer stays green when the pool cannot be synced. It caught only SyncUnavailableError, which meant the contract held only because of an upstream class hierarchy: today FirebaseAuthError happens to subclass RemoteSyncError, and requests' JSONDecodeError happens to subclass RequestException, so an expired refresh token or a malformed payload does get wrapped and caught. That is a coincidence, not a promise to this package -- and it is exactly the kind that goes quiet the day upstream reorganises its exceptions. The symptom would be a unit going red every 15 minutes, which is the outcome this function's own docstring says the design exists to prevent. It now also catches anything unexpected and logs it with a traceback, so a real bug is loud in the journal while the timer stays green. The narrow handler stays first and stays at debug level: offline is normal on a laptop and must not fill the journal. Two tests, neither of which the old suite could have failed on -- the existing one monkeypatched get_client to raise the one exception already known to be handled. One raises FirebaseAuthError *mid-sync* (a token expiring during the tick rather than at client construction, which is the realistic case per seed_session.py's own docstring), the other raises a RuntimeError that no hierarchy covers. 111 tests, 100% branch coverage. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
parent
aa032c95e4
commit
bb0f773b7d
@ -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
|
||||
|
||||
@ -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
|
||||
|
||||
Loading…
Reference in New Issue
Block a user