Add support for retrying rate-limited API requests

stripe/src/main/java/com/stripe/android/networking/ApiRequestExecutor.kt

@@ -3,33 +3,59 @@ package com.stripe.android.networking
 import androidx.annotation.VisibleForTesting
 import com.stripe.android.Logger
 import com.stripe.android.exception.APIConnectionException
+import kotlinx.coroutines.delay
 import java.io.IOException
 
 internal interface ApiRequestExecutor {
-    fun execute(request: ApiRequest): StripeResponse
+    suspend fun execute(request: ApiRequest): StripeResponse
 
-    fun execute(request: FileUploadRequest): StripeResponse
+    suspend fun execute(request: FileUploadRequest): StripeResponse
 
     /**
      * Used by [StripeApiRepository] to make Stripe API requests
      */
     class Default internal constructor(
         private val connectionFactory: ConnectionFactory = ConnectionFactory.Default(),
+        private val retryDelaySupplier: RetryDelaySupplier = RetryDelaySupplier(),
         private val logger: Logger = Logger.noop()
     ) : ApiRequestExecutor {
+        override suspend fun execute(
+            request: ApiRequest
+        ): StripeResponse = executeInternal(request, MAX_RETRIES)
 
-        override fun execute(request: ApiRequest): StripeResponse {
-            return executeInternal(request)
-        }
-
-        override fun execute(request: FileUploadRequest): StripeResponse {
-            return executeInternal(request)
-        }
+        override suspend fun execute(
+            request: FileUploadRequest
+        ): StripeResponse = executeInternal(request, MAX_RETRIES)
 
         @VisibleForTesting
-        internal fun executeInternal(request: StripeRequest): StripeResponse {
-            logger.info(request.toString())
+        internal suspend fun executeInternal(
+            request: StripeRequest,
+            remainingRetries: Int
+        ): StripeResponse {
+            logger.info("Firing request: $request")
+
+            val stripeResponse = makeRequest(request)
+
+            return if (stripeResponse.isRateLimited && remainingRetries > 0) {
+                logger.info(
+                    "Request was rate-limited with $remainingRetries remaining retries."
+                )
 
+                delay(
+                    retryDelaySupplier.getDelayMillis(
+                        MAX_RETRIES,
+                        remainingRetries
+                    )
+                )
+                executeInternal(request, remainingRetries - 1)
+            } else {
+                stripeResponse
+            }
+        }
+
+        private fun makeRequest(
+            request: StripeRequest
+        ): StripeResponse {
             return connectionFactory.create(request).use {
                 runCatching {
                     val stripeResponse = it.response
@@ -45,5 +71,17 @@ internal interface ApiRequestExecutor {
                 }
             }
         }
+
+        private companion object {
+            /**
+             * If the SDK receives a "Too Many Requests" (429) status code from Stripe,
+             * it will automatically retry the request using exponential backoff.
+             *
+             * The default value is 3.
+             *
+             * See https://stripe.com/docs/rate-limits for more information.
+             */
+            private const val MAX_RETRIES = 3
+        }
     }
 }

stripe/src/main/java/com/stripe/android/networking/StripeResponse.kt

@@ -28,6 +28,7 @@ internal data class StripeResponse internal constructor(
 ) {
     internal val isOk: Boolean = code == HttpURLConnection.HTTP_OK
     internal val isError: Boolean = code < 200 || code >= 300
+    internal val isRateLimited = code == 429
 
     internal val requestId: RequestId? = RequestId.fromString(
         getHeaderValue(REQUEST_ID_HEADER)?.firstOrNull()

stripe/src/test/java/com/stripe/android/networking/StripeApiRepositoryTest.kt

@@ -84,15 +84,6 @@ internal class StripeApiRepositoryTest {
         whenever(fingerprintDataRepository.get()).thenReturn(
             FingerprintDataFixtures.create(Calendar.getInstance().timeInMillis)
         )
-
-        whenever(stripeApiRequestExecutor.execute(any<FileUploadRequest>()))
-            .thenReturn(
-                StripeResponse(
-                    200,
-                    StripeFileFixtures.DEFAULT.toString(),
-                    emptyMap()
-                )
-            )
     }
 
     @AfterTest
@@ -783,6 +774,15 @@ internal class StripeApiRepositoryTest {
 
     @Test
     fun createFile_shouldFireExpectedRequests() = testDispatcher.runBlockingTest {
+        whenever(stripeApiRequestExecutor.execute(any<FileUploadRequest>()))
+            .thenReturn(
+                StripeResponse(
+                    200,
+                    StripeFileFixtures.DEFAULT.toString(),
+                    emptyMap()
+                )
+            )
+
         val stripeRepository = create()
 
         stripeRepository.createFile(

stripe/src/test/java/com/stripe/android/networking/StripeApiRequestExecutorTest.kt

@@ -3,17 +3,29 @@ package com.stripe.android.networking
 import com.google.common.truth.Truth.assertThat
 import com.stripe.android.exception.APIConnectionException
 import com.stripe.android.exception.InvalidRequestException
+import kotlinx.coroutines.ExperimentalCoroutinesApi
+import kotlinx.coroutines.test.TestCoroutineDispatcher
+import kotlinx.coroutines.test.runBlockingTest
 import org.junit.runner.RunWith
 import org.robolectric.RobolectricTestRunner
 import java.io.ByteArrayOutputStream
 import java.io.UnsupportedEncodingException
 import java.net.UnknownHostException
 import java.util.UUID
+import kotlin.test.AfterTest
 import kotlin.test.Test
 import kotlin.test.assertFailsWith
 
+@ExperimentalCoroutinesApi
 @RunWith(RobolectricTestRunner::class)
-class StripeApiRequestExecutorTest {
+internal class StripeApiRequestExecutorTest {
+    private val testDispatcher = TestCoroutineDispatcher()
+
+    @AfterTest
+    fun cleanup() {
+        testDispatcher.cleanupTestCoroutines()
+    }
+
     @Test
     fun bodyBytes_shouldHandleUnsupportedEncodingException() {
         val stripeRequest = object : StripeRequest() {
@@ -39,38 +51,82 @@ class StripeApiRequestExecutorTest {
     }
 
     @Test
-    fun `executeInternal with IllegalStateException should throw the exception`() {
-        val executor = ApiRequestExecutor.Default(
-            connectionFactory = FakeConnectionFactory(
-                FailingConnection(
-                    IllegalStateException("Failure")
+    fun `executeInternal with IllegalStateException should throw the exception`() =
+        testDispatcher.runBlockingTest {
+            val executor = ApiRequestExecutor.Default(
+                connectionFactory = FakeConnectionFactory(
+                    FailingConnection(
+                        IllegalStateException("Failure")
+                    )
                 )
             )
-        )
 
-        val failure = assertFailsWith<IllegalStateException> {
-            executor.executeInternal(FakeStripeRequest())
+            val failure = assertFailsWith<IllegalStateException> {
+                executor.executeInternal(FakeStripeRequest(), MAX_RETRIES)
+            }
+            assertThat(failure.message)
+                .isEqualTo("Failure")
         }
-        assertThat(failure.message)
-            .isEqualTo("Failure")
-    }
 
     @Test
-    fun `executeInternal with IOException should throw an APIConnectionException`() {
-        val executor = ApiRequestExecutor.Default(
-            connectionFactory = FakeConnectionFactory(
-                FailingConnection(
-                    UnknownHostException("Could not connect to Stripe API")
+    fun `executeInternal with IOException should throw an APIConnectionException`() =
+        testDispatcher.runBlockingTest {
+            val executor = ApiRequestExecutor.Default(
+                connectionFactory = FakeConnectionFactory(
+                    FailingConnection(
+                        UnknownHostException("Could not connect to Stripe API")
+                    )
                 )
             )
-        )
 
-        val failure = assertFailsWith<APIConnectionException> {
-            executor.executeInternal(FakeStripeRequest())
+            val failure = assertFailsWith<APIConnectionException> {
+                executor.executeInternal(FakeStripeRequest(), MAX_RETRIES)
+            }
+            assertThat(failure.message)
+                .isEqualTo("IOException during API request to Stripe (https://api.stripe.com): Could not connect to Stripe API. Please check your internet connection and try again. If this problem persists, you should check Stripe's service status at https://twitter.com/stripestatus, or let us know at support@stripe.com.")
+        }
+
+    @Test
+    fun `executeInternal when retries exhausted should return rate-limited response`() =
+        testDispatcher.runBlockingTest {
+            val connectionFactory = FakeConnectionFactory(
+                FakeConnection(429)
+            )
+            val executor = ApiRequestExecutor.Default(
+                connectionFactory = connectionFactory,
+                retryDelaySupplier = RetryDelaySupplier(0)
+            )
+
+            val response = executor.executeInternal(FakeStripeRequest(), MAX_RETRIES)
+            assertThat(connectionFactory.createInvocations)
+                .isEqualTo(MAX_RETRIES + 1)
+
+            assertThat(response.isRateLimited)
+                .isTrue()
+        }
+
+    @Test
+    fun `executeInternal when rate-limited once then succeeds should return OK response`() =
+        testDispatcher.runBlockingTest {
+            val connectionFactory = FakeConnectionFactory { count ->
+                if (count <= 1) {
+                    FakeConnection(429)
+                } else {
+                    FakeConnection(200)
+                }
+            }
+            val executor = ApiRequestExecutor.Default(
+                connectionFactory = connectionFactory,
+                retryDelaySupplier = RetryDelaySupplier(0)
+            )
+
+            val response = executor.executeInternal(FakeStripeRequest(), MAX_RETRIES)
+            assertThat(connectionFactory.createInvocations)
+                .isEqualTo(2)
+
+            assertThat(response.isOk)
+                .isTrue()
         }
-        assertThat(failure.message)
-            .isEqualTo("IOException during API request to Stripe (https://api.stripe.com): Could not connect to Stripe API. Please check your internet connection and try again. If this problem persists, you should check Stripe's service status at https://twitter.com/stripestatus, or let us know at support@stripe.com.")
-    }
 
     private class FakeStripeRequest : StripeRequest() {
         override val method: Method = Method.POST
@@ -85,10 +141,21 @@ class StripeApiRequestExecutorTest {
     }
 
     private class FakeConnectionFactory(
-        private val stripeConnection: StripeConnection
+        private val stripeConnection: (Int) -> StripeConnection
     ) : ConnectionFactory {
-        override fun create(request: StripeRequest): StripeConnection {
-            return stripeConnection
+        constructor(
+            stripeConnection: StripeConnection
+        ) : this(
+            { stripeConnection }
+        )
+
+        var createInvocations = 0
+
+        override fun create(
+            request: StripeRequest
+        ): StripeConnection {
+            createInvocations++
+            return stripeConnection(createInvocations)
         }
     }
 
@@ -105,4 +172,21 @@ class StripeApiRequestExecutorTest {
         override fun close() {
         }
     }
+
+    private class FakeConnection(
+        override val responseCode: Int
+    ) : StripeConnection {
+        override val response: StripeResponse
+            get() = StripeResponse(
+                code = responseCode,
+                body = null
+            )
+
+        override fun close() {
+        }
+    }
+
+    private companion object {
+        private const val MAX_RETRIES = 3
+    }
 }