fix: stop the GitHub release check exhausting its own rate limit

Opening the About sheet runs a release check, and an exhausted quota fed
itself: only successes were cached, and a 403 reporting zero remaining
was classed retryable, so every sheet open spent three more requests
rediscovering the same limit. Unauthenticated GitHub allows 60 requests
an hour per IP, and over Tor that IP is an exit node shared with every
other user on it, so the ceiling arrives far sooner than per-user maths
suggests.

Three changes:

- Conditional requests. The client now stores the release ETag and
  replays it as If-None-Match. GitHub does not charge a 304 against the
  rate limit, so revalidating an expired cache is free where an
  unconditional refetch costs one of the 60. This is why neither polling
  nor long polling is the right answer here.

- A rate-limit gate. X-RateLimit-Reset and Retry-After were read only to
  interpolate into an error string; they now set a deadline before which
  no request is sent at all. While blocked, a stale cached release is
  served in preference to an error the user cannot act on. Clamped to an
  hour so a bad header cannot lock the feature out, and a reset time in
  the past falls back to a fixed backoff rather than unblocking a skewed
  clock immediately.

- Rate limits are no longer retried in-loop. The gate decides when it is
  worth asking again. A plain 403 is a permissions failure and is no
  longer retried either.

The gate's decision logic is pure and unit tested. The wiring around it
is not: that needs a MockWebServer, which is not currently a dependency.

Known gap: the cache and ETag are in memory only, so a process restart
still costs one request. Persisting them needs a Context threaded into
what is currently a context-free object; left as a follow-up.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Moe Hamade 2026-07-28 20:36:43 +03:00
parent c02dda308e
commit 657fee0de6
3 changed files with 261 additions and 17 deletions

View File

@ -0,0 +1,54 @@
package com.bitchat.android.util
/**
* Reads GitHub's rate-limit rejections so the app can stop asking.
*
* Unauthenticated requests are capped at 60 an hour *per IP*, and when the app routes through Tor
* that IP belongs to an exit node shared with every other user on it, so the ceiling arrives much
* sooner than the per-user maths suggests. Retrying a rejection is pure waste, and repeating it on
* every screen open is what turns a brief limit into a permanent one.
*/
internal object GitHubRateLimit {
/** Used when GitHub rejects a request without saying when to come back. */
const val DEFAULT_BACKOFF_MILLIS = 10 * 60 * 1000L
/** Never sit out longer than this, however far ahead the reset header claims to be. */
const val MAX_BACKOFF_MILLIS = 60 * 60 * 1000L
/**
* A 403 alone is not enough: GitHub also uses it for ordinary permission failures. Only a 403
* that reports zero remaining quota, or an explicit 429, is a rate limit.
*/
fun isRateLimited(code: Int, remaining: String?): Boolean =
code == 429 || (code == 403 && remaining?.trim() == "0")
/**
* Epoch millis before which no further request should be sent, or null when the response was
* not a rate-limit rejection at all.
*/
fun blockedUntilMillis(
code: Int,
remaining: String?,
resetEpochSeconds: String?,
retryAfterSeconds: String?,
nowMillis: Long,
): Long? {
if (!isRateLimited(code, remaining)) return null
// Retry-After is a delta and is what GitHub sends for secondary limits, which can lift
// sooner than the primary window X-RateLimit-Reset describes.
val fromRetryAfter = retryAfterSeconds?.trim()?.toLongOrNull()
?.takeIf { it > 0 }
?.let { nowMillis + it * 1000 }
// Dropped when it is not in the future: a skewed device clock must not turn a genuine
// rejection into "retry immediately".
val fromReset = resetEpochSeconds?.trim()?.toLongOrNull()
?.let { it * 1000 }
?.takeIf { it > nowMillis }
val target = fromRetryAfter ?: fromReset ?: (nowMillis + DEFAULT_BACKOFF_MILLIS)
return target.coerceIn(nowMillis, nowMillis + MAX_BACKOFF_MILLIS)
}
}

View File

@ -23,12 +23,30 @@ object GitHubReleaseClient {
private const val CACHE_TTL_MILLIS = 10 * 60 * 1000L
private const val MAX_FETCH_ATTEMPTS = 3
private const val ROUTE_READY_TIMEOUT_MILLIS = 60_000L
private const val HTTP_NOT_MODIFIED = 304
private val fetchMutex = Mutex()
@Volatile
private var cachedRelease: CachedRelease? = null
/**
* ETag of the cached release, replayed as `If-None-Match`. GitHub does not charge a 304
* against the rate limit, so revalidating an expired cache this way costs nothing where an
* unconditional refetch costs one of only 60 hourly requests.
*/
@Volatile
private var cachedEtag: String? = null
/**
* Epoch millis before which GitHub has already told us it will reject anything we send.
*
* Without this, an exhausted quota fed itself: nothing cached the failure, so every screen
* that asked for release info spent three more requests discovering the same limit.
*/
@Volatile
private var blockedUntilMillis = 0L
private val client
get() = OkHttpProvider.httpClient().newBuilder()
// GitHub requests may travel through Tor, where a 15-second total
@ -46,10 +64,31 @@ object GitHubReleaseClient {
suspend fun fetchLatestRelease(forceRefresh: Boolean = false): Result<Release> =
withContext(Dispatchers.IO) {
fetchMutex.withLock {
if (!forceRefresh) {
cachedRelease
?.takeIf { System.currentTimeMillis() - it.fetchedAtMillis < CACHE_TTL_MILLIS }
?.let { return@withLock Result.success(it.release) }
val now = System.currentTimeMillis()
val cached = cachedRelease
if (!forceRefresh &&
cached != null &&
now - cached.fetchedAtMillis < CACHE_TTL_MILLIS
) {
return@withLock Result.success(cached.release)
}
// 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.
if (now < blockedUntilMillis) {
val waitMinutes = (blockedUntilMillis - now) / 60_000 + 1
Log.w(TAG, "Rate limited; not contacting GitHub for another ${waitMinutes}min")
cached?.let { return@withLock Result.success(it.release) }
return@withLock Result.failure(
ReleaseFetchException(
message = "GitHub API rate limit reached. Try again in " +
"$waitMinutes minute${if (waitMinutes == 1L) "" else "s"}.",
httpCode = 429,
retryable = false
)
)
}
if (!awaitSelectedNetworkRoute()) {
@ -95,6 +134,8 @@ object GitHubReleaseClient {
}
private fun fetchLatestReleaseOnce(): Result<Release> {
val cached = cachedRelease
val etag = cachedEtag
return try {
Log.d(TAG, "Fetching latest release from GitHub API")
val request = Request.Builder()
@ -102,29 +143,48 @@ object GitHubReleaseClient {
.addHeader("User-Agent", USER_AGENT)
.addHeader("Accept", "application/vnd.github+json")
.addHeader("X-GitHub-Api-Version", "2022-11-28")
.apply {
// Revalidate rather than refetch. GitHub does not charge a 304 against the
// hourly quota, so an unchanged release costs nothing to confirm.
if (cached != null && etag != null) addHeader("If-None-Match", etag)
}
.build()
client.newCall(request).execute().use { response ->
if (response.code == HTTP_NOT_MODIFIED && cached != null) {
Log.d(TAG, "Release unchanged; cache revalidated at no quota cost")
cachedRelease = cached.copy(fetchedAtMillis = System.currentTimeMillis())
return Result.success(cached.release)
}
if (!response.isSuccessful) {
val remaining = response.header("X-RateLimit-Remaining")
val resetAt = response.header("X-RateLimit-Reset")
val message = when {
response.code == 403 && remaining == "0" ->
"GitHub API rate limit exceeded. Try again after reset time $resetAt."
response.code == 429 ->
"GitHub API rate limit exceeded. Please try again later."
else ->
"GitHub release request failed: HTTP ${response.code} ${response.message}"
val blockedUntil = GitHubRateLimit.blockedUntilMillis(
code = response.code,
remaining = response.header("X-RateLimit-Remaining"),
resetEpochSeconds = response.header("X-RateLimit-Reset"),
retryAfterSeconds = response.header("Retry-After"),
nowMillis = System.currentTimeMillis(),
)
if (blockedUntil != null) blockedUntilMillis = blockedUntil
val message = if (blockedUntil != null) {
val waitMinutes =
(blockedUntil - System.currentTimeMillis()) / 60_000 + 1
"GitHub API rate limit exceeded. Try again in " +
"$waitMinutes minute${if (waitMinutes == 1L) "" else "s"}."
} else {
"GitHub release request failed: HTTP ${response.code} ${response.message}"
}
Log.e(TAG, message)
return Result.failure(
ReleaseFetchException(
message = message,
httpCode = response.code,
retryable = response.code == 403 ||
response.code == 408 ||
response.code == 429 ||
response.code >= 500
// A rate limit is never worth an in-loop retry: the gate in
// fetchLatestRelease decides when it is worth asking again. A plain
// 403 is a permissions failure and will not fix itself either.
retryable = blockedUntil == null &&
(response.code == 408 || response.code >= 500)
)
)
}
@ -146,6 +206,10 @@ object GitHubReleaseClient {
retryable = false
)
)
// Kept alongside the release so the pair can never drift: a stale ETag would
// revalidate to a 304 that confirms a release we no longer hold.
cachedEtag = response.header("ETag")
blockedUntilMillis = 0L
Result.success(release)
}
} catch (e: IOException) {

View File

@ -0,0 +1,126 @@
package com.bitchat.android.util
import org.junit.Assert.assertEquals
import org.junit.Assert.assertFalse
import org.junit.Assert.assertNull
import org.junit.Assert.assertTrue
import org.junit.Test
/**
* Unauthenticated GitHub allows 60 requests an hour per IP, and over Tor that IP is an exit node
* shared with everyone else using it. Reading the rejection correctly is what keeps the app from
* hammering a quota it has already exhausted.
*/
class GitHubRateLimitTest {
private val now = 1_700_000_000_000L
@Test
fun `a 403 that still has quota is a permissions error, not a rate limit`() {
assertFalse(GitHubRateLimit.isRateLimited(code = 403, remaining = "42"))
assertNull(
GitHubRateLimit.blockedUntilMillis(
code = 403,
remaining = "42",
resetEpochSeconds = null,
retryAfterSeconds = null,
nowMillis = now,
)
)
}
@Test
fun `a 403 with no quota left blocks until the advertised reset`() {
val resetSeconds = now / 1000 + 900
assertTrue(GitHubRateLimit.isRateLimited(code = 403, remaining = "0"))
assertEquals(
resetSeconds * 1000,
GitHubRateLimit.blockedUntilMillis(
code = 403,
remaining = "0",
resetEpochSeconds = resetSeconds.toString(),
retryAfterSeconds = null,
nowMillis = now,
)
)
}
@Test
fun `a 429 is a rate limit even without a remaining header`() {
assertTrue(GitHubRateLimit.isRateLimited(code = 429, remaining = null))
}
@Test
fun `Retry-After takes precedence over the reset header`() {
// Retry-After is a delta and is what GitHub sends for secondary limits, which can expire
// sooner than the primary window the reset header describes.
assertEquals(
now + 30_000,
GitHubRateLimit.blockedUntilMillis(
code = 429,
remaining = "0",
resetEpochSeconds = (now / 1000 + 3_000).toString(),
retryAfterSeconds = "30",
nowMillis = now,
)
)
}
@Test
fun `a rejection with no timing headers falls back to a fixed backoff`() {
assertEquals(
now + GitHubRateLimit.DEFAULT_BACKOFF_MILLIS,
GitHubRateLimit.blockedUntilMillis(
code = 429,
remaining = null,
resetEpochSeconds = null,
retryAfterSeconds = null,
nowMillis = now,
)
)
}
@Test
fun `a reset time already in the past falls back rather than unblocking immediately`() {
// A skewed device clock must not turn a real rejection into "retry right now".
assertEquals(
now + GitHubRateLimit.DEFAULT_BACKOFF_MILLIS,
GitHubRateLimit.blockedUntilMillis(
code = 429,
remaining = null,
resetEpochSeconds = (now / 1000 - 500).toString(),
retryAfterSeconds = null,
nowMillis = now,
)
)
}
@Test
fun `an absurd reset time is clamped so the app is never locked out for long`() {
assertEquals(
now + GitHubRateLimit.MAX_BACKOFF_MILLIS,
GitHubRateLimit.blockedUntilMillis(
code = 429,
remaining = null,
resetEpochSeconds = (now / 1000 + 86_400).toString(),
retryAfterSeconds = null,
nowMillis = now,
)
)
}
@Test
fun `unparseable headers fall back instead of throwing`() {
assertEquals(
now + GitHubRateLimit.DEFAULT_BACKOFF_MILLIS,
GitHubRateLimit.blockedUntilMillis(
code = 429,
remaining = null,
resetEpochSeconds = "not-a-number",
retryAfterSeconds = "Wed, 21 Oct 2015 07:28:00 GMT",
nowMillis = now,
)
)
}
}