diff --git a/app/src/main/java/ink/trmnl/android/di/NetworkModule.kt b/app/src/main/java/ink/trmnl/android/di/NetworkModule.kt index c5c13d0..891150f 100644 --- a/app/src/main/java/ink/trmnl/android/di/NetworkModule.kt +++ b/app/src/main/java/ink/trmnl/android/di/NetworkModule.kt @@ -11,6 +11,7 @@ import com.squareup.moshi.kotlin.reflect.KotlinJsonAdapterFactory import dagger.Module import dagger.Provides import ink.trmnl.android.BuildConfig +import ink.trmnl.android.network.RateLimitInterceptor import ink.trmnl.android.network.TrmnlApiService import ink.trmnl.android.network.TrmnlUserApiService import okhttp3.Cache @@ -48,6 +49,9 @@ object NetworkModule { return OkHttpClient .Builder() + // Add rate limit interceptor to handle HTTP 429 with exponential backoff + // This must be added BEFORE other interceptors to retry at the network layer + .addInterceptor(RateLimitInterceptor()) .addInterceptor { chain -> val request = chain diff --git a/app/src/main/java/ink/trmnl/android/network/RateLimitInterceptor.kt b/app/src/main/java/ink/trmnl/android/network/RateLimitInterceptor.kt new file mode 100644 index 0000000..d3c8558 --- /dev/null +++ b/app/src/main/java/ink/trmnl/android/network/RateLimitInterceptor.kt @@ -0,0 +1,146 @@ +package ink.trmnl.android.network + +import okhttp3.Interceptor +import okhttp3.Response +import timber.log.Timber +import java.io.IOException +import kotlin.math.min +import kotlin.math.pow +import kotlin.random.Random + +/** + * OkHttp interceptor that handles HTTP 429 (Too Many Requests) responses with exponential backoff. + * + * This interceptor: + * - Detects HTTP 429 rate limit responses + * - Implements exponential backoff with jitter to avoid thundering herd + * - Respects the Retry-After header if provided by the server + * - Retries the request up to a maximum number of attempts + * - Logs retry attempts for debugging + * + * Exponential backoff formula: + * - Base delay: 1 second + * - Delay = base * (2 ^ attempt) with jitter + * - Jitter: delay * (0.5 + 0.5 * random) to distribute load + * - Max delay: 32 seconds per retry + * + * See: https://github.com/usetrmnl/trmnl-android/issues/260 + */ +class RateLimitInterceptor : Interceptor { + companion object { + private const val TAG = "RateLimitInterceptor" + + /** + * Maximum number of retry attempts for rate-limited requests. + * After this many retries, the interceptor gives up and returns the 429 response. + */ + private const val MAX_RETRIES = 5 + + /** + * Initial backoff delay in milliseconds (1 second). + */ + private const val INITIAL_BACKOFF_MS = 1000L + + /** + * Maximum backoff delay in milliseconds (32 seconds). + * Prevents exponential backoff from growing too large. + */ + private const val MAX_BACKOFF_MS = 32_000L + + /** + * HTTP status code for "Too Many Requests" (rate limiting). + */ + private const val HTTP_TOO_MANY_REQUESTS = 429 + } + + override fun intercept(chain: Interceptor.Chain): Response { + val request = chain.request() + var response = chain.proceed(request) + var attempt = 0 + + // Retry loop for handling 429 responses + while (response.code == HTTP_TOO_MANY_REQUESTS && attempt < MAX_RETRIES) { + attempt++ + + // Calculate backoff delay + val backoffDelay = calculateBackoffDelay(attempt, response) + + Timber.tag(TAG).w( + "Rate limit exceeded (HTTP 429) for ${request.url}. " + + "Retry attempt $attempt/$MAX_RETRIES after ${backoffDelay}ms", + ) + + // Close the previous response before retrying + response.close() + + // Wait for the backoff delay + try { + Thread.sleep(backoffDelay) + } catch (e: InterruptedException) { + Thread.currentThread().interrupt() + throw IOException("Interrupted while waiting for rate limit backoff", e) + } + + // Retry the request + response = chain.proceed(request) + } + + // Log if we exhausted all retries + if (response.code == HTTP_TOO_MANY_REQUESTS && attempt >= MAX_RETRIES) { + Timber.tag(TAG).e( + "Rate limit exceeded (HTTP 429) for ${request.url}. " + + "Exhausted all $MAX_RETRIES retry attempts. Giving up.", + ) + } + + return response + } + + /** + * Calculates the backoff delay for the current retry attempt. + * + * Priority: + * 1. Use Retry-After header if present (seconds or HTTP-date) + * 2. Use exponential backoff with jitter + * + * @param attempt Current retry attempt (1-indexed) + * @param response The 429 response containing potential Retry-After header + * @return Backoff delay in milliseconds + */ + private fun calculateBackoffDelay( + attempt: Int, + response: Response, + ): Long { + // Check for Retry-After header (RFC 7231) + val retryAfterHeader = response.header("Retry-After") + if (retryAfterHeader != null) { + val retryAfterSeconds = retryAfterHeader.toLongOrNull() + if (retryAfterSeconds != null) { + // Retry-After is in seconds, convert to milliseconds + val delayMs = retryAfterSeconds * 1000 + Timber.tag(TAG).d("Using Retry-After header: ${retryAfterSeconds}s (${delayMs}ms)") + return min(delayMs, MAX_BACKOFF_MS) + } + // Note: We don't handle HTTP-date format for Retry-After as it's rarely used + // If needed, it can be parsed using SimpleDateFormat or java.time APIs + } + + // Use exponential backoff with jitter + // Formula: base * (2^attempt) * jitter + // Jitter: random value between 0.5 and 1.0 to prevent thundering herd + val exponentialDelay = INITIAL_BACKOFF_MS * (2.0.pow(attempt - 1)).toLong() + val jitter = 0.5 + (0.5 * Random.nextDouble()) + val delayMs = (exponentialDelay * jitter).toLong() + + // Cap at maximum backoff delay + val cappedDelayMs = min(delayMs, MAX_BACKOFF_MS) + + Timber.tag(TAG).d( + "Using exponential backoff: attempt=$attempt, " + + "exponential=${exponentialDelay}ms, jitter=${"%.2f".format(jitter)}, " + + "delay=${delayMs}ms, capped=${cappedDelayMs}ms", + ) + + return cappedDelayMs + } +} diff --git a/app/src/main/java/ink/trmnl/android/work/TrmnlWorkScheduler.kt b/app/src/main/java/ink/trmnl/android/work/TrmnlWorkScheduler.kt index 13e3fbb..22714ea 100644 --- a/app/src/main/java/ink/trmnl/android/work/TrmnlWorkScheduler.kt +++ b/app/src/main/java/ink/trmnl/android/work/TrmnlWorkScheduler.kt @@ -12,7 +12,6 @@ import androidx.work.PeriodicWorkRequestBuilder import androidx.work.WorkInfo import androidx.work.WorkManager import androidx.work.WorkQuery -import androidx.work.WorkRequest import androidx.work.workDataOf import com.squareup.anvil.annotations.optional.SingleIn import ink.trmnl.android.data.TrmnlDeviceConfigDataStore @@ -137,8 +136,10 @@ class TrmnlWorkScheduler .setBackoffCriteria( // Exponential backoff for retrying failed work // To avoid overwhelming the server with requests + // Using 60 seconds initial delay (increased from 30s default) + // to give more breathing room for rate-limited requests BackoffPolicy.EXPONENTIAL, - WorkRequest.DEFAULT_BACKOFF_DELAY_MILLIS, + 60_000L, // 60 seconds initial backoff TimeUnit.MILLISECONDS, ).setInputData( workDataOf( @@ -191,8 +192,10 @@ class TrmnlWorkScheduler OneTimeWorkRequestBuilder() .setConstraints(constraints) .setBackoffCriteria( + // Exponential backoff for retrying failed work + // Using 60 seconds initial delay (increased from 30s default) BackoffPolicy.EXPONENTIAL, - WorkRequest.DEFAULT_BACKOFF_DELAY_MILLIS, + 60_000L, // 60 seconds initial backoff TimeUnit.MILLISECONDS, ).setInputData( workDataOf( diff --git a/app/src/test/java/ink/trmnl/android/network/RateLimitInterceptorTest.kt b/app/src/test/java/ink/trmnl/android/network/RateLimitInterceptorTest.kt new file mode 100644 index 0000000..3027bfb --- /dev/null +++ b/app/src/test/java/ink/trmnl/android/network/RateLimitInterceptorTest.kt @@ -0,0 +1,244 @@ +package ink.trmnl.android.network + +import com.google.common.truth.Truth.assertThat +import okhttp3.Interceptor +import okhttp3.MediaType.Companion.toMediaType +import okhttp3.OkHttpClient +import okhttp3.Protocol +import okhttp3.Request +import okhttp3.Response +import okhttp3.ResponseBody.Companion.toResponseBody +import org.junit.Before +import org.junit.Test +import java.util.concurrent.TimeUnit + +/** + * Tests for [RateLimitInterceptor] to verify exponential backoff behavior for HTTP 429 responses. + */ +class RateLimitInterceptorTest { + private lateinit var interceptor: RateLimitInterceptor + private lateinit var testRequest: Request + + @Before + fun setup() { + interceptor = RateLimitInterceptor() + testRequest = + Request + .Builder() + .url("https://test.com/api/display") + .build() + } + + @Test + fun `intercept allows successful response to pass through`() { + // Arrange + val mockChain = createMockChain(200, "OK") + + // Act + val response = interceptor.intercept(mockChain) + + // Assert + assertThat(response.code).isEqualTo(200) + assertThat(response.message).isEqualTo("OK") + } + + @Test + fun `intercept retries on HTTP 429 and eventually succeeds`() { + // Arrange - Fail twice with 429, then succeed + val responses = + mutableListOf( + createResponse(429, "Too Many Requests"), + createResponse(429, "Too Many Requests"), + createResponse(200, "OK"), + ) + val mockChain = createMockChainWithMultipleResponses(responses) + + // Act + val startTime = System.currentTimeMillis() + val response = interceptor.intercept(mockChain) + val duration = System.currentTimeMillis() - startTime + + // Assert + assertThat(response.code).isEqualTo(200) + // Should have retried twice, taking at least 1s + 2s = 3s total + // Using a lower bound to account for jitter (0.5x factor) + assertThat(duration).isAtLeast(1500L) // 1.5s minimum (with jitter) + } + + @Test + fun `intercept respects Retry-After header in seconds`() { + // Arrange - 429 with Retry-After header, then success + val responses = + mutableListOf( + createResponse(429, "Too Many Requests", retryAfterSeconds = "2"), + createResponse(200, "OK"), + ) + val mockChain = createMockChainWithMultipleResponses(responses) + + // Act + val startTime = System.currentTimeMillis() + val response = interceptor.intercept(mockChain) + val duration = System.currentTimeMillis() - startTime + + // Assert + assertThat(response.code).isEqualTo(200) + // Should have waited approximately 2 seconds + assertThat(duration).isAtLeast(1900L) // Account for slight timing variations + assertThat(duration).isAtMost(2500L) + } + + @Test + fun `intercept gives up after max retries`() { + // Arrange - Always return 429 + val responses = mutableListOf() + repeat(10) { + // More than MAX_RETRIES + responses.add(createResponse(429, "Too Many Requests")) + } + val mockChain = createMockChainWithMultipleResponses(responses) + + // Act + val response = interceptor.intercept(mockChain) + + // Assert - Should still return 429 after exhausting retries + assertThat(response.code).isEqualTo(429) + } + + @Test + fun `intercept applies exponential backoff with proper delays`() { + // Arrange - Fail multiple times with 429 + val responses = + mutableListOf( + createResponse(429, "Too Many Requests"), + createResponse(429, "Too Many Requests"), + createResponse(429, "Too Many Requests"), + createResponse(200, "OK"), + ) + val mockChain = createMockChainWithMultipleResponses(responses) + + // Act + val startTime = System.currentTimeMillis() + val response = interceptor.intercept(mockChain) + val duration = System.currentTimeMillis() - startTime + + // Assert + assertThat(response.code).isEqualTo(200) + // Expected delays: ~1s, ~2s, ~4s = ~7s total (with jitter 0.5x-1.0x) + // Minimum: 0.5 * (1 + 2 + 4) = 3.5s + assertThat(duration).isAtLeast(3500L) + } + + @Test + fun `intercept caps backoff at max delay`() { + // Create client with interceptor + val client = + OkHttpClient + .Builder() + .addInterceptor(RateLimitInterceptor()) + .connectTimeout(1, TimeUnit.MINUTES) + .readTimeout(1, TimeUnit.MINUTES) + .build() + + // This test verifies that MAX_BACKOFF_MS (32s) is respected + // We can't easily test this without a real server, so we'll just + // verify the interceptor is properly integrated + assertThat(client.interceptors).hasSize(1) + assertThat(client.interceptors[0]).isInstanceOf(RateLimitInterceptor::class.java) + } + + // Helper functions + + private fun createMockChain( + statusCode: Int, + message: String, + ): Interceptor.Chain = + object : Interceptor.Chain { + override fun request(): Request = testRequest + + override fun proceed(request: Request): Response = createResponse(statusCode, message) + + override fun connection() = null + + override fun call() = throw UnsupportedOperationException("Not implemented for test") + + override fun connectTimeoutMillis() = 30_000 + + override fun withConnectTimeout( + timeout: Int, + unit: TimeUnit, + ) = this + + override fun readTimeoutMillis() = 30_000 + + override fun withReadTimeout( + timeout: Int, + unit: TimeUnit, + ) = this + + override fun writeTimeoutMillis() = 30_000 + + override fun withWriteTimeout( + timeout: Int, + unit: TimeUnit, + ) = this + } + + private fun createMockChainWithMultipleResponses(responses: MutableList): Interceptor.Chain = + object : Interceptor.Chain { + private var callCount = 0 + + override fun request(): Request = testRequest + + override fun proceed(request: Request): Response { + val response = responses.removeFirstOrNull() ?: createResponse(500, "Out of responses") + callCount++ + return response + } + + override fun connection() = null + + override fun call() = throw UnsupportedOperationException("Not implemented for test") + + override fun connectTimeoutMillis() = 30_000 + + override fun withConnectTimeout( + timeout: Int, + unit: TimeUnit, + ) = this + + override fun readTimeoutMillis() = 30_000 + + override fun withReadTimeout( + timeout: Int, + unit: TimeUnit, + ) = this + + override fun writeTimeoutMillis() = 30_000 + + override fun withWriteTimeout( + timeout: Int, + unit: TimeUnit, + ) = this + } + + private fun createResponse( + statusCode: Int, + message: String, + retryAfterSeconds: String? = null, + ): Response { + val responseBuilder = + Response + .Builder() + .request(testRequest) + .protocol(Protocol.HTTP_2) + .code(statusCode) + .message(message) + .body("{}".toResponseBody("application/json".toMediaType())) + + if (retryAfterSeconds != null) { + responseBuilder.header("Retry-After", retryAfterSeconds) + } + + return responseBuilder.build() + } +}