fix: check the cooldown on every attempt, not once before the loop

Addresses review on #812.

Each retry resampled the route but never rechecked the gate against it,
so a route change during a request or its backoff walked straight past a
cooldown. A Tor attempt failing with a 500, then the user switching to a
direct connection that is already rate limited, and the next attempt
contacts it regardless.

The check moves inside the loop, immediately after the route is sampled,
which makes it cover the first attempt too -- the separate post-wait
check it replaces was only ever that first iteration. The check before
the route wait stays: knowing the selected route is blocked is worth
avoiding a sixty-second Tor bootstrap for.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Moe Hamade 2026-07-29 16:07:59 +03:00
parent 309df1ab61
commit 821ec7c5f7

View File

@ -185,12 +185,6 @@ object GitHubReleaseClient {
// for the whole metadata request.
onResolvingRelease?.invoke()
// That wait can last a minute, in which time the user may have switched
// routes. The cooldown that matters is the one for the route the request
// will actually take, which is only known now.
blockedResultOrNull(System.currentTimeMillis(), selectedRouteUsesTor(), cached)
?.let { return@withLock it }
var lastFailure: Throwable = ReleaseFetchException(
"Failed to fetch the latest release from GitHub"
)
@ -201,6 +195,12 @@ object GitHubReleaseClient {
// the request did not use.
val routeUsesTor = selectedRouteUsesTor()
// Per attempt rather than once before the loop. The route can change
// during the wait above, during a request, or during a backoff, and
// the one we have just switched to may carry a cooldown of its own.
blockedResultOrNull(System.currentTimeMillis(), routeUsesTor, cached)
?.let { return@withLock it }
val result = fetchLatestReleaseOnce(routeUsesTor)
result.onSuccess { release ->
cachedRelease = CachedRelease(release, System.currentTimeMillis())