Add shouldRetry overload for retries (#69)

* Add shouldRetry overload for retries Resolves #68 * Deprecate old * Clean up test
slackhq · Sep 26, 2023 · 87aaa22 · 87aaa22
1 parent ba0e7b9
commit 87aaa22
Show file tree

Hide file tree

Showing 3 changed files with 50 additions and 2 deletions.
diff --git a/api/eithernet.api b/api/eithernet.api
@@ -88,8 +88,10 @@ public abstract interface annotation class com/slack/eithernet/ResultType : java
 }
 
 public final class com/slack/eithernet/RetriesKt {
-	public static final fun retryWithExponentialBackoff-3FA4DCs (IJDJDLkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
+	public static final synthetic fun retryWithExponentialBackoff-3FA4DCs (IJDJDLkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
 	public static synthetic fun retryWithExponentialBackoff-3FA4DCs$default (IJDJDLkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lkotlin/coroutines/Continuation;ILjava/lang/Object;)Ljava/lang/Object;
+	public static final fun retryWithExponentialBackoff-3c68mSE (IJDJDLkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function2;Lkotlin/jvm/functions/Function1;Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
+	public static synthetic fun retryWithExponentialBackoff-3c68mSE$default (IJDJDLkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function2;Lkotlin/jvm/functions/Function1;Lkotlin/coroutines/Continuation;ILjava/lang/Object;)Ljava/lang/Object;
 }
 
 public abstract interface annotation class com/slack/eithernet/StatusCode : java/lang/annotation/Annotation {

diff --git a/src/main/java/com/slack/eithernet/Retries.kt b/src/main/java/com/slack/eithernet/Retries.kt
@@ -23,6 +23,29 @@ import kotlin.time.Duration.Companion.seconds
 import kotlin.time.times
 import kotlinx.coroutines.delay
 
+@Suppress("LongParameterList")
+@Deprecated("Left for ABI compatibility", level = DeprecationLevel.HIDDEN)
+public suspend fun <T : Any, E : Any> retryWithExponentialBackoff(
+  maxAttempts: Int = 3,
+  initialDelay: Duration = 500.milliseconds,
+  delayFactor: Double = 2.0,
+  maxDelay: Duration = 10.seconds,
+  jitterFactor: Double = 0.25,
+  onFailure: ((failure: ApiResult.Failure<E>) -> Unit)? = null,
+  block: suspend () -> ApiResult<T, E>
+): ApiResult<T, E> {
+  return retryWithExponentialBackoff(
+    maxAttempts = maxAttempts,
+    initialDelay = delayFactor * initialDelay,
+    delayFactor = delayFactor,
+    maxDelay = maxDelay,
+    jitterFactor = jitterFactor,
+    onFailure = onFailure,
+    shouldRetry = { true },
+    block = block
+  )
+}
+
 /**
  * Retries a [block] of code with exponential backoff.
  *
@@ -43,6 +66,8 @@ import kotlinx.coroutines.delay
  * @param jitterFactor The maximum factor of jitter to introduce. For example, a value of 0.1 will
  *   introduce up to 10% jitter (both positive and negative).
  * @param onFailure An optional callback for failures, useful for logging.
+ * @param shouldRetry An optional callback for indicating whether to retry a failure. This can be
+ *   used to short-circuit attempts in the event of some non-retry-able condition.
  * @return The result of the operation if it's successful, or the last failure result if all
  *   attempts fail.
  */
@@ -54,6 +79,7 @@ public tailrec suspend fun <T : Any, E : Any> retryWithExponentialBackoff(
   maxDelay: Duration = 10.seconds,
   jitterFactor: Double = 0.25,
   onFailure: ((failure: ApiResult.Failure<E>) -> Unit)? = null,
+  shouldRetry: suspend ((failure: ApiResult.Failure<E>) -> Boolean) = { true },
   block: suspend () -> ApiResult<T, E>
 ): ApiResult<T, E> {
   require(maxAttempts > 0) { "maxAttempts must be greater than 0" }
@@ -63,7 +89,7 @@ public tailrec suspend fun <T : Any, E : Any> retryWithExponentialBackoff(
     is ApiResult.Failure -> {
       val attemptsRemaining = maxAttempts - 1
       onFailure?.invoke(result)
-      if (attemptsRemaining == 0) {
+      if (attemptsRemaining == 0 || !shouldRetry(result)) {
         result
       } else {
         val jitter = 1 + Random.nextDouble(-jitterFactor, jitterFactor.nextUp())
@@ -76,6 +102,7 @@ public tailrec suspend fun <T : Any, E : Any> retryWithExponentialBackoff(
           maxDelay = maxDelay,
           jitterFactor = jitterFactor,
           onFailure = onFailure,
+          shouldRetry = shouldRetry,
           block = block
         )
       }

diff --git a/src/test/kotlin/com/slack/eithernet/RetriesTest.kt b/src/test/kotlin/com/slack/eithernet/RetriesTest.kt
@@ -16,6 +16,7 @@
 package com.slack.eithernet
 
 import com.google.common.truth.Truth.assertThat
+import java.io.IOException
 import kotlin.time.Duration.Companion.milliseconds
 import kotlinx.coroutines.test.currentTime
 import kotlinx.coroutines.test.runTest
@@ -123,4 +124,22 @@ class RetriesTest {
     // Check that all delays are the same, which would indicate that jitter was not applied
     assertThat(delays.distinct().size).isEqualTo(1)
   }
+
+  @Test
+  fun `does not retry on matching condition`() = runTest {
+    var attempts = 0
+    retryWithExponentialBackoff(
+      maxAttempts = 5,
+      shouldRetry = { it !is ApiResult.Failure.UnknownFailure }
+    ) {
+      attempts++
+      if (attempts > 2) {
+        ApiResult.unknownFailure(RuntimeException("error"))
+      } else {
+        ApiResult.networkFailure(IOException("error"))
+      }
+    }
+    // Check that we only attempted until an unknown failure happened
+    assertThat(attempts).isEqualTo(3)
+  }
 }