michaelbull / kotlin-retry Goto Github PK

In my project, I wrote

`import com.github.michaelbull.retry.ContinueRetrying
import com.github.michaelbull.retry.StopRetrying
import com.github.michaelbull.retry.policy.RetryPolicy

fun filter(test: (Throwable) -> Boolean): RetryPolicy = {
if (test(this.reason)) {
ContinueRetrying
} else {
StopRetrying
}
}`

It seems that issue #4 was closed, but I have also inadvertently run into this situation today with similar code:

    withTimeout(1000) {
        retryResult {
            // This crashes because coroutineContext is obtained from the withTimeout scope
//            println("My retryStatus is ${coroutineContext.retryStatus}")

            // This is okay (naming conflict with coroutineContext)
            println("My retryStatus is ${kotlin.coroutines.coroutineContext.retryStatus}")

            // This is also okay
            coroutineScope {
                println("My retryStatus is ${coroutineContext.retryStatus}")
            }

            Ok(Unit)
        }
    }

I was able to workaround this issue fairly easily, but it is easy to shoot yourself in the foot. I understand it would be a breaking api change to update the type signature of ResultProducer. Is there any other reason why you wouldn't want to provide the proper scope to the retryResult block?

It seems that only the tests need JVM-librarys. So this project could also target Kotlin/Js Kotlin/Native... That would be really nice!

See: https://youtrack.jetbrains.com/issue/KTOR-3544

for examples of how the call being retried may receive information to adjust the next retry delay.

Also consider: maximum concurrent retries, rate limiting affected by the retries.

It would be extremely useful for a retry method to exist that uses a RetryPolicy that does not require the use of a Throwable.

An example use-case of this might be when an API call is made, no error is thrown but the HTTP code is one that is unexpected

For example this code lifted from a side project of mine:

private val retryServerCallPolicy: RetryPolicy<NetworkResponse<*, *>> = {
        when (reason) {
            is NetworkResponse.ApiError -> when (reason.code) {
                in 500..599 -> ContinueRetrying
                else -> StopRetrying
            }
            else -> StopRetrying
        }
    }

Which ideally could be used in this way:

retry(retryServerCallPolicy + limitAttempts(3)) {
    myApiCallMethod()
}

I'd love to know what your thoughts are on this.

Hi,

It would be useful to be able to define on which exception types we want to retry on.

spypunk

Super short request: could you release a new version with 201a368350020b5d54cc112d0a9dd9024df467fb included in it?

It would be useful to log out a warning on a retry for monitoring connection issues that may succeed on retry.

Is this possible?

First of all, thanks for this lightweight library!

I tried emitting a warning log message whenever a retry attempt fails. Unfortunately, I wasn't able to log out the current attempt number. Is there a way to do this?

One possibility would be to add a new field "failedAttempt" to RetryFailure. Then one could log out the attempt number in the RetryPolicy.

Thanks for this lib! I've had one minor hiccup whilst utilising in my current code, as demonstrated by this simple unit test:

package somepackage

import com.github.michaelbull.retry.ContinueRetrying
import com.github.michaelbull.retry.StopRetrying
import com.github.michaelbull.retry.policy.RetryPolicy
import com.github.michaelbull.retry.policy.fullJitterBackoff
import com.github.michaelbull.retry.policy.limitAttempts
import com.github.michaelbull.retry.policy.plus
import com.github.michaelbull.retry.retry
import kotlinx.coroutines.runBlocking
import org.junit.Test

class RetryTest {
    private val retryFailures: RetryPolicy<Throwable> = {
        when (reason) {
            is RuntimeException -> ContinueRetrying

            else -> StopRetrying
        }
    }

    @Test
    fun doesnotwork() = runBlocking {
        // this fails because the RetryPolicy is considered incorrect...delayMillis equals 0 after first '+'         
        retry(retryFailures + limitAttempts(5) + fullJitterBackoff(100, 1000)) {
            doSomethingWrong()
        }
    }

    @Test
    fun works() = runBlocking {
        retry(fullJitterBackoff(100, 1000) + retryFailures + limitAttempts(5)) {
            doSomethingWrong()
        }
    }


    private fun doSomethingWrong() {
        throw RuntimeException("Gotcha")
    }
}

It seems that the plus operator is prematurely enforcing the validation of the final Policy before all elements might have been added in. Perhaps the 'delayMillis' validation can be done after this line in Retry.kt?

                val instruction = RetryFailure(t).policy()

Hello,
Thanks for very cool library!
I'd like to include it to my work project, but my org. policies doesn't allow me to use bintray repositories.
Could you please add you library to Jcenter?
I'm also ready to help if you need any.
Thanks!

I'm trying to adapt some existing inflexible retry code to use your library.

One thing we do is that in some cases our retry function throws a different exception than the wrapped code. eg, we say "retry if the exception is T; if we have given up after too many Ts, throw T1, but if an unexpected exception happened just throw it directly".

We can implement this by having a RetryPolicy that actually throws T1 instead of returning StopRetrying, and it appears to work in the current implementation. Is this something we can rely on?

Any plans to bring back retryResult?

It looks like it was dropped in db1c515 ("Rewrite to remove dependency on CoroutineContext").

Maybe it would make sense (for library consumers that don't use Result) to provide retryResult via an additional Maven artifact?

Thank you!

I am using this library with ktor on JVM and really liking it. Thanks for all the work done on this library. Are there any plans to support other kotlin targets, especially web and native ?

This makes it more usable

fun onException(vararg types: KClass<out Throwable>): RetryPolicy<Throwable> {
    return {
        if (types.any { it.isInstance(reason) }) ContinueRetrying else StopRetrying
    }
}

In some cases there is not a common base class for all exceptions that should trigger retry.

Thanks for your work on this nice library!

I found out that the following exception may be thrown if the backoff policy full jitter is used:

java.lang.IllegalArgumentException: delayMillis must be positive: 0
	at com.github.michaelbull.retry.policy.BackoffKt$fullJitterBackoff$3.invokeSuspend(Backoff.kt:120)
	at com.github.michaelbull.retry.policy.BackoffKt$fullJitterBackoff$3.invoke(Backoff.kt)
	at com.github.michaelbull.retry.policy.BackoffTest$simulate$2.invokeSuspend(BackoffTest.kt:43)
	at com.github.michaelbull.retry.policy.BackoffTest$simulate$2.invoke(BackoffTest.kt)
	at kotlinx.coroutines.intrinsics.UndispatchedKt.startUndispatchedOrReturn(Undispatched.kt:91)
	at kotlinx.coroutines.BuildersKt__Builders_commonKt.withContext(Builders.common.kt:160)
	at kotlinx.coroutines.BuildersKt.withContext(Unknown Source)

This is caused by the use of the random function without a lower bound. Thus the random delay can be 0. If you don't mind, I'll provide a pull request.

So, there's some sort of issue with Graalvm where it complains about "merging incompatible frames". It results in an error as shown in oracle/graal#3342 but manifests when rethrowing exceptions. Thankfully, the fix is really simple and I will open up a pull request.

It would be helpful for logging to be able to access the coroutine scope inside of the retry function. For example being able to see:

    retry(limitAttempts(5)) {
        println(coroutineContext.retryStatus.attempt)
    }

thanks!