Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rewrite retryOnException without delicate Coroutines API #8233

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ internal class PollAuthorizationSessionOAuthResults @Inject constructor(
return retryOnException(
PollTimingOptions(
initialDelayMs = 0,
maxNumberOfRetries = 300, // Stripe.js has 600 second timeout, 600 / 2 = 300 retries
attempts = 300, // Stripe.js has 600 second timeout, 600 / 2 = 300 retries
retryInterval = 2.seconds.inWholeMilliseconds
),
retryCondition = { exception -> exception.shouldRetry }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,10 @@ internal class SaveAccountToLink @Inject constructor(
return retryOnException(
options = PollTimingOptions(
initialDelayMs = 1.seconds.inWholeMilliseconds,
maxNumberOfRetries = 20,
attempts = 20,
),
retryCondition = { it.shouldRetry },
block = { accountsRepository.pollAccountNumbers(linkedAccountIds) },
action = { accountsRepository.pollAccountNumbers(linkedAccountIds) },
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,47 +3,52 @@ package com.stripe.android.financialconnections.utils
import androidx.annotation.RestrictTo
import com.stripe.android.core.exception.StripeException
import kotlinx.coroutines.delay
import kotlinx.coroutines.flow.channelFlow
import kotlinx.coroutines.flow.first
import java.net.HttpURLConnection
import kotlin.time.Duration.Companion.seconds

/**
* Executes and returns the result of the given [block].
* Executes and returns the result of the given [action].
* If the block execution fails, and [retryCondition] is met, the operation is retried.
* Otherwise the resulting exception will be thrown.
*/
internal suspend fun <T> retryOnException(
options: PollTimingOptions,
retryCondition: suspend (Throwable) -> Boolean,
block: suspend () -> T
): T = channelFlow {
var remainingTimes = options.maxNumberOfRetries - 1
while (!isClosedForSend) {
delay(
if (remainingTimes == options.maxNumberOfRetries - 1) {
options.initialDelayMs
} else {
options.retryInterval
}
)
val either = runCatching { block() }
either.fold(
action: suspend () -> T
): T {
val attempts = options.attempts
var attempt = 1
var result: T? = null

while (attempt <= attempts && result == null) {
if (attempt == 1) {
delay(options.initialDelayMs)
} else {
delay(options.retryInterval)
}

runCatching {
action()
}.fold(
onFailure = { exception ->
when {
remainingTimes == 0 -> throw PollingReachedMaxRetriesException(options)
retryCondition(exception).not() -> throw exception
if (!retryCondition(exception)) {
throw exception
}
},
onSuccess = { send(it) }
onSuccess = {
result = it
}
)
remainingTimes--

attempt += 1
}
}.first()

return result ?: throw PollingReachedMaxRetriesException(options)
}

internal data class PollTimingOptions(
val initialDelayMs: Long = 1.75.seconds.inWholeMilliseconds,
val maxNumberOfRetries: Int = 180,
val attempts: Int = 180,
val retryInterval: Long = 0.25.seconds.inWholeMilliseconds
)

Expand All @@ -53,7 +58,7 @@ internal data class PollTimingOptions(
internal class PollingReachedMaxRetriesException(
pollingOptions: PollTimingOptions
) : StripeException(
message = "reached max number of retries ${pollingOptions.maxNumberOfRetries}.",
message = "reached max number of attempts ${pollingOptions.attempts}.",
statusCode = HttpURLConnection.HTTP_ACCEPTED
) {
@RestrictTo(RestrictTo.Scope.LIBRARY_GROUP)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import com.stripe.android.financialconnections.model.FinancialConnectionsSession
import com.stripe.android.financialconnections.model.PartnerAccountsList
import com.stripe.android.financialconnections.repository.FinancialConnectionsAccountsRepository
import junit.framework.TestCase.assertEquals
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.test.runTest
import org.junit.Test
import org.mockito.Mockito.mock
Expand All @@ -25,7 +24,6 @@ import org.mockito.kotlin.whenever
import java.net.HttpURLConnection
import kotlin.test.assertIs

@OptIn(ExperimentalCoroutinesApi::class)
internal class PollAuthorizationSessionAccountsTest {

private val repository: FinancialConnectionsAccountsRepository = mock()
Expand Down Expand Up @@ -81,7 +79,7 @@ internal class PollAuthorizationSessionAccountsTest {

assertIs<AccountLoadError>(exception)

// Retries 180 times
// Attempts 180 times
verify(repository, times(180)).postAuthorizationSessionAccounts(
configuration.financialConnectionsSessionClientSecret,
sync.manifest.activeAuthSession!!.id
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,18 @@ package com.stripe.android.financialconnections.utils

import com.google.common.truth.Truth.assertThat
import com.stripe.android.core.exception.APIException
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.test.runTest
import org.junit.Test
import java.net.HttpURLConnection

@OptIn(ExperimentalCoroutinesApi::class)
internal class ErrorsKtTest {

@Test
fun `should throw timeout if reaches max times`() = runTest {
val testResult = kotlin.runCatching {
retryOnException(
PollTimingOptions(
maxNumberOfRetries = 5,
attempts = 5,
initialDelayMs = 0,
retryInterval = 1000
),
Expand All @@ -34,7 +32,7 @@ internal class ErrorsKtTest {
var counter = 0
val result = retryOnException(
PollTimingOptions(
maxNumberOfRetries = 5,
attempts = 5,
initialDelayMs = 0,
retryInterval = 1000
),
Expand Down Expand Up @@ -70,15 +68,16 @@ internal class ErrorsKtTest {
var counter = 0
retryOnException(
PollTimingOptions(
maxNumberOfRetries = 2,
attempts = 2,
initialDelayMs = 0,
retryInterval = 1000
),
retryCondition = { exception -> exception.shouldRetry }
) {
counter++
if (counter == 3) true else throw retryException()
}
retryCondition = { exception -> exception.shouldRetry },
action = {
counter++
if (counter == 3) true else throw retryException()
}
)
}
assertThat(testResult.exceptionOrNull()!!).isInstanceOf(PollingReachedMaxRetriesException::class.java)
}
Expand Down
Loading