diff --git a/app/src/main/java/com/bitchat/android/util/ApkDownloadSource.kt b/app/src/main/java/com/bitchat/android/util/ApkDownloadSource.kt index 73f168ba..226e6842 100644 --- a/app/src/main/java/com/bitchat/android/util/ApkDownloadSource.kt +++ b/app/src/main/java/com/bitchat/android/util/ApkDownloadSource.kt @@ -150,8 +150,17 @@ internal object ApkDownloadHttpErrors { rateLimitResetEpochSeconds = rateLimitResetEpochSeconds, nowMillis = nowMillis ) + // X-RateLimit-Reset rides on every GitHub response, an ordinary 403 included, so it + // cannot tell an exhausted quota from a permissions failure. Only a spent quota or an + // explicit Retry-After says this request was the one that got limited. The reset header + // still supplies the deadline below, once being limited is established some other way. + val retryAfterMillis = retryAtMillis( + retryAfter = retryAfter, + rateLimitResetEpochSeconds = null, + nowMillis = nowMillis + ) val rateLimited = code == 429 || - (code == 403 && (rateLimitRemaining?.trim() == "0" || retryAt != null)) + (code == 403 && (rateLimitRemaining?.trim() == "0" || retryAfterMillis != null)) if (rateLimited) { return ApkDownloadException( diff --git a/app/src/test/kotlin/com/bitchat/android/util/ApkDownloadSourceTest.kt b/app/src/test/kotlin/com/bitchat/android/util/ApkDownloadSourceTest.kt index 4d6702d6..e14a968d 100644 --- a/app/src/test/kotlin/com/bitchat/android/util/ApkDownloadSourceTest.kt +++ b/app/src/test/kotlin/com/bitchat/android/util/ApkDownloadSourceTest.kt @@ -101,6 +101,47 @@ class ApkDownloadSourceTest { assertEquals(ApkDownloadFailureReason.RateLimited, quotaFailure.reason) } + @Test + fun `a reset header alone does not make a 403 a rate limit`() { + // GitHub sends X-RateLimit-Reset on every response, so a permissions failure carries one + // while the quota is untouched. Reading it as a limit would park the route in a cooldown + // and serve stale metadata until a window the failure has nothing to do with. + val failure = ApkDownloadHttpErrors.fromResponse( + source = source, + code = 403, + responseMessage = "Forbidden", + retryAfter = null, + rateLimitRemaining = "4999", + rateLimitResetEpochSeconds = (now / 1000L + 1_800L).toString(), + nowMillis = now + ) + + assertEquals(ApkDownloadFailureReason.HttpFailure, failure.reason) + assertNull(failure.retryAtMillis) + assertEquals( + listOf(source.displayName, "403", "Forbidden"), + failure.messageArgs + ) + } + + @Test + fun `a secondary limit is still caught by its Retry-After`() { + // The quota is intact, so only Retry-After marks this one. It has to keep working, or + // tightening the reset-header case would blind the client to secondary limits. + val failure = ApkDownloadHttpErrors.fromResponse( + source = source, + code = 403, + responseMessage = "Forbidden", + retryAfter = "90", + rateLimitRemaining = "4999", + rateLimitResetEpochSeconds = (now / 1000L + 1_800L).toString(), + nowMillis = now + ) + + assertEquals(ApkDownloadFailureReason.RateLimited, failure.reason) + assertEquals(now + 90_000L, failure.retryAtMillis) + } + @Test fun `invalid or overflowing retry headers never crash error mapping`() { assertNull(