mirror of
https://github.com/kuhyx/screen-locker.git
synced 2026-11-10 21:27:13 +01:00
Krzysztof kuhy Rudnicki
b0d9165a79
The morning routine is started by systemd right after boot and right after
resume, so its sync ran seconds before DHCP/DNS were up. Every such run failed
outright — and said this:
Workout sync push FAILED for 6 workout(s): network error reading
screen-locker-sync/devices — a 403 here means the token lacks
contents:write on kuhyx/syncs
Both halves of that were wrong. It was not a 403, and the token was fine: the
same six workouts pushed successfully by hand minutes later on the same token,
which reports push=true on the repo. The message cost a real investigation
chasing a permissions bug that did not exist.
- _sync_retry.with_sync_retry wraps the three sync call sites in a bounded
retry (4 attempts, 2s/4s/8s backoff — ~14s, enough for the network to come
up, short enough not to drag the routine). Verified offline in a network
namespace: three retries, then an honest give-up.
- Any GitHubSyncError retries, deliberately including RepoNotFoundError.
Classifying transient vs permanent would couple us to which exception
crdt_sync chains where; a few wasted seconds on a genuinely bad token is the
cheaper mistake.
- The push failure log now reports what actually happened. The contents:write
hint survives only for RepoNotFoundError, which is the one error that
actually implicates the repo or the token.
Retry at the application layer rather than ordering the unit after
network-online.target: the same race hits on resume, where boot ordering does
not apply, and these are user units that cannot depend on a system target.
The tests' autouse no-sleep fixture patches the module-local `sleep` name
rather than `_sync_retry.time.sleep` — the latter mutates the shared time
module object and silently disabled sleep for the whole suite, breaking an
unrelated temperature timeout test.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
96 lines
3.2 KiB
Python
96 lines
3.2 KiB
Python
"""Bounded retry for sync calls that run before the network is up.
|
|
|
|
Every sync entry point here is reached from the morning routine, which systemd
|
|
starts immediately after boot and after resume — routinely a few seconds before
|
|
DHCP/DNS have settled. A single-shot call in that window fails for a reason
|
|
that has nothing to do with the sync itself, and (before this module) reported
|
|
it as a token/permissions problem.
|
|
|
|
Retrying at the application layer rather than ordering the unit after
|
|
``network-online.target``: the same race happens on *resume*, where boot
|
|
ordering does not apply, and the user services involved cannot depend on a
|
|
system target anyway.
|
|
|
|
Any :class:`GitHubSyncError` is retried, deliberately including
|
|
:class:`RepoNotFoundError`. Classifying "transient" vs "permanent" would mean
|
|
depending on which exception ``crdt_sync`` chains where, which is internal to
|
|
that library and has changed before; a few wasted seconds on a genuinely
|
|
misconfigured token is a much cheaper mistake than skipping a real retry.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import logging
|
|
from time import sleep
|
|
from typing import TYPE_CHECKING, TypeVar
|
|
|
|
from crdt_sync import GitHubSyncError
|
|
|
|
from screen_locker._constants import (
|
|
SYNC_RETRY_ATTEMPTS,
|
|
SYNC_RETRY_DELAY_SECONDS,
|
|
)
|
|
|
|
if TYPE_CHECKING:
|
|
from collections.abc import Callable
|
|
|
|
_logger = logging.getLogger(__name__)
|
|
|
|
T = TypeVar("T")
|
|
|
|
|
|
def with_sync_retry(
|
|
operation: Callable[[], T],
|
|
*,
|
|
description: str,
|
|
attempts: int = SYNC_RETRY_ATTEMPTS,
|
|
delay_seconds: float = SYNC_RETRY_DELAY_SECONDS,
|
|
) -> T:
|
|
"""Run *operation*, retrying a failed sync with exponential backoff.
|
|
|
|
Args:
|
|
operation: The zero-argument sync call to run.
|
|
description: Human phrase naming the operation, used in the retry
|
|
logs (e.g. ``"push PC workouts"``).
|
|
attempts: Total tries, including the first. Values below 1 still run
|
|
the operation exactly once.
|
|
delay_seconds: Base backoff; attempt *n* waits ``delay * 2**(n-1)``.
|
|
|
|
Returns:
|
|
Whatever *operation* returns.
|
|
|
|
Raises:
|
|
GitHubSyncError: If every attempt failed. The caller keeps its own
|
|
handling — this only buys time, it never hides the outcome.
|
|
"""
|
|
last_attempt = max(attempts, 1)
|
|
# Every attempt but the last swallows-and-waits; the last one is run
|
|
# outside the loop so its failure propagates with no unreachable branch.
|
|
for attempt in range(1, last_attempt):
|
|
try:
|
|
return operation()
|
|
except GitHubSyncError as exc:
|
|
wait = delay_seconds * (2 ** (attempt - 1))
|
|
_logger.warning(
|
|
"Could not %s (attempt %d/%d): %s — retrying in %.0fs; "
|
|
"this is normal right after boot or resume, when the "
|
|
"network is not up yet",
|
|
description,
|
|
attempt,
|
|
last_attempt,
|
|
exc,
|
|
wait,
|
|
)
|
|
sleep(wait)
|
|
|
|
try:
|
|
return operation()
|
|
except GitHubSyncError as exc:
|
|
_logger.warning(
|
|
"Could not %s after %d attempt(s): %s — giving up",
|
|
description,
|
|
last_attempt,
|
|
exc,
|
|
)
|
|
raise
|