fix: free the UI on cancel, and scope the rate-limit gate to its route

Addresses review on #812.

Cancelling left the spinner up for as long as the status check took.
Forcing checkStatus() past its guard was not enough: resolveApkStatus()
calls checkForUpdate(), which reaches the network and can sit on the
60-second route timeout while Tor bootstraps. Nothing clears the state
in the meantime -- the cancelled job maps to Idle, which the observer
ignores -- so the stop button looked broken for the whole wait. The
state now leaves Downloading immediately and the check resolves it
afterwards.

The rate-limit gate was process-wide. GitHub counts unauthenticated
requests per IP, so a cooldown earned through a shared Tor exit was
being applied to a direct connection with an entirely different quota,
and vice versa -- potentially suppressing a usable route for an hour.
The gate now records which route earned it and is dropped when the
current route differs.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Moe Hamade 2026-07-29 15:31:20 +03:00
parent 8e5bb2ea9a
commit 77f3c0cc05
2 changed files with 41 additions and 5 deletions

View File

@ -197,10 +197,17 @@ class ApkDownloadViewModel(application: Application) : AndroidViewModel(applicat
private fun onCancelDownload() {
downloader.cancelDownload()
// Nothing else will move the UI off the spinner. checkStatus() refuses to
// overwrite a Downloading state, and the Idle that WorkManager reports for a
// cancelled job is ignored for the same reason -- both guards protect a job
// that is still running, which this one is not.
// Leave Downloading now, not when the check returns. resolveApkStatus() reaches
// the network and can sit on the route timeout for a full minute, and nothing
// else would clear the spinner in the meantime -- the cancelled job maps to Idle,
// which the observer ignores. Without this the stop button looks broken for the
// whole wait.
_state.update {
it.copy(apkStatus = ApkPreparationStatus.Loading, downloadProgress = 0)
}
// force, because the guards in checkStatus() protect a job that is still
// running, which this one is not.
checkStatus(force = true)
}

View File

@ -47,6 +47,27 @@ object GitHubReleaseClient {
@Volatile
private var blockedUntilMillis = 0L
/**
* Whether the route was proxied when [blockedUntilMillis] was recorded, or null when
* no gate is set. GitHub counts unauthenticated requests per IP, so a Tor exit and a
* direct connection have separate quotas -- a cooldown earned on one must not be
* served to the other.
*/
private var blockedRouteUsedProxy: Boolean? = null
/** Drops a gate earned on a route the app is no longer using. */
private fun clearGateIfRouteChanged() {
val recordedRoute = blockedRouteUsedProxy ?: return
val currentRoute = runCatching { ArtiTorManager.getInstance().isProxyEnabled() }
.getOrNull() ?: return
if (recordedRoute != currentRoute) {
Log.i(TAG, "Route changed since the rate limit was recorded; clearing the gate")
blockedUntilMillis = 0L
blockedRouteUsedProxy = null
}
}
private val client
get() = OkHttpProvider.httpClient().newBuilder()
// GitHub requests may travel through Tor, where a 15-second total
@ -86,6 +107,8 @@ object GitHubReleaseClient {
// Honoured even on an explicit refresh: sending a request GitHub has already said
// it will reject helps nobody and pushes the reset further out. A stale release is
// a better answer than an error the user cannot act on.
clearGateIfRouteChanged()
if (now < blockedUntilMillis) {
val waitMinutes = (blockedUntilMillis - now) / 60_000 + 1
Log.w(TAG, "Rate limited; not contacting GitHub for another ${waitMinutes}min")
@ -192,7 +215,12 @@ object GitHubReleaseClient {
retryAfterSeconds = response.header("Retry-After"),
nowMillis = System.currentTimeMillis(),
)
if (blockedUntil != null) blockedUntilMillis = blockedUntil
if (blockedUntil != null) {
blockedUntilMillis = blockedUntil
// Record which route earned it: the quota belongs to that IP.
blockedRouteUsedProxy =
runCatching { ArtiTorManager.getInstance().isProxyEnabled() }.getOrNull()
}
val message = if (blockedUntil != null) {
val waitMinutes =
@ -237,6 +265,7 @@ object GitHubReleaseClient {
// revalidate to a 304 that confirms a release we no longer hold.
cachedEtag = response.header("ETag")
blockedUntilMillis = 0L
blockedRouteUsedProxy = null
Result.success(release)
}
} catch (e: IOException) {