Repository: apollographql/apollo-kotlin (kotlin)

Issue:
RetryOnNetworkErrorInterceptor should be configurable
### Use case

We currently have our own interceptor to handle retries on network errors, but 
1) it also retries if the http status code is 5xx
2) we'd like the ability to configure the backoff duration

Not a huge deal (we can obviously keep using our own) but I'd like to switch to using NetworkMonitor+failFastIfOffline+retryOnError if possible

### Describe the solution you'd like

_No response_

Pull request:
Closes #5982

Patch:
diff --git a/libraries/apollo-api/src/commonMain/kotlin/com/apollographql/apollo3/api/ApolloRequest.kt b/libraries/apollo-api/src/commonMain/kotlin/com/apollographql/apollo3/api/ApolloRequest.kt
index 11bae0faf5c..12a3a71d5e4 100644
--- a/libraries/apollo-api/src/commonMain/kotlin/com/apollographql/apollo3/api/ApolloRequest.kt
+++ b/libraries/apollo-api/src/commonMain/kotlin/com/apollographql/apollo3/api/ApolloRequest.kt
@@ -17,6 +17,10 @@ import com.benasher44.uuid.uuid4
  * val newRequest = apolloRequest.newBuilder().addHttpHeader("Authorization", "Bearer $token").build()
  * ```
  *
+ * @property operation the GraphQL operation for this request
+ * @property requestUuid a unique id for this request. For queries and mutations, this is only used for debug.
+ * For subscriptions, it is used as subscription id when multiplexing several subscription over a WebSocket.
+ *
  * @see [com.apollographql.apollo3.ApolloCall]
  */
 class ApolloRequest<D : Operation.Data>
diff --git a/libraries/apollo-runtime/src/commonMain/kotlin/com/apollographql/apollo3/ApolloClient.kt b/libraries/apollo-runtime/src/commonMain/kotlin/com/apollographql/apollo3/ApolloClient.kt
index 21b34cf8c32..81db78e34f8 100644
--- a/libraries/apollo-runtime/src/commonMain/kotlin/com/apollographql/apollo3/ApolloClient.kt
+++ b/libraries/apollo-runtime/src/commonMain/kotlin/com/apollographql/apollo3/ApolloClient.kt
@@ -22,10 +22,9 @@ import com.apollographql.apollo3.interceptor.ApolloInterceptor
 import com.apollographql.apollo3.interceptor.AutoPersistedQueryInterceptor
 import com.apollographql.apollo3.interceptor.DefaultInterceptorChain
 import com.apollographql.apollo3.interceptor.NetworkInterceptor
-import com.apollographql.apollo3.interceptor.RetryOnNetworkErrorInterceptor
+import com.apollographql.apollo3.interceptor.RetryOnErrorInterceptor
 import com.apollographql.apollo3.internal.ApolloClientListener
 import com.apollographql.apollo3.internal.defaultDispatcher
-import com.apollographql.apollo3.network.NetworkMonitor
 import com.apollographql.apollo3.network.NetworkTransport
 import com.apollographql.apollo3.network.http.BatchingHttpInterceptor
 import com.apollographql.apollo3.network.http.HttpEngine
@@ -83,8 +82,8 @@ private constructor(
   val subscriptionNetworkTransport: NetworkTransport
   val interceptors: List<ApolloInterceptor> = builder.interceptors
   val customScalarAdapters: CustomScalarAdapters = builder.customScalarAdapters
-  private val networkMonitor: NetworkMonitor?
   private val retryOnError: ((ApolloRequest<*>) -> Boolean)? = builder.retryOnError
+  private val retryOnErrorInterceptor: ApolloInterceptor? = builder.retryOnErrorInterceptor
   private val failFastIfOffline = builder.failFastIfOffline
   private val listeners = builder.listeners
 
@@ -97,8 +96,6 @@ private constructor(
   override val canBeBatched: Boolean? = builder.canBeBatched
 
   init {
-    networkMonitor = builder.networkMonitor
-
     networkTransport = if (builder.networkTransport != null) {
       check(builder.httpServerUrl == null) {
         "Apollo: 'httpServerUrl' has no effect if 'networkTransport' is set"
@@ -318,7 +315,7 @@ private constructor(
 
     val allInterceptors = buildList {
       addAll(interceptors)
-      add(RetryOnNetworkErrorInterceptor(networkMonitor))
+      add(retryOnErrorInterceptor ?: RetryOnErrorInterceptor())
       add(networkInterceptor)
     }
     return DefaultInterceptorChain(allInterceptors, 0)
@@ -399,11 +396,11 @@ private constructor(
       private set
 
     @ApolloExperimental
-    var networkMonitor: NetworkMonitor? = null
+    var retryOnError: ((ApolloRequest<*>) -> Boolean)? = null
       private set
 
     @ApolloExperimental
-    var retryOnError: ((ApolloRequest<*>) -> Boolean)? = null
+    var retryOnErrorInterceptor: ApolloInterceptor? = null
       private set
 
     @ApolloExperimental
@@ -412,26 +409,16 @@ private constructor(
 
     /**
      * Whether to fail fast if the device is offline.
+     * Requires setting an interceptor that is aware of the network state with [retryOnErrorInterceptor].
      *
-     * In that case, the returned [ApolloResponse.exception] is an instance of [com.apollographql.apollo3.exception.ApolloNetworkException]
-     *
-     * @see NetworkMonitor
+     * @see [retryOnErrorInterceptor]
+     * @see [com.apollographql.apollo3.network.NetworkMonitor]
      */
     @ApolloExperimental
     fun failFastIfOffline(failFastIfOffline: Boolean?): Builder = apply {
       this.failFastIfOffline = failFastIfOffline
     }
 
-    /**
-     * Configures the [NetworkMonitor] for this [ApolloClient]
-     *
-     * @param networkMonitor or `null` to use the default [NetworkMonitor]
-     */
-    @ApolloExperimental
-    fun networkMonitor(networkMonitor: NetworkMonitor?): Builder = apply {
-      this.networkMonitor = networkMonitor
-    }
-
     /**
      * Configures the [retryOnError] default if [ApolloRequest.retryOnError] is not set.
      *
@@ -451,6 +438,34 @@ private constructor(
       this.retryOnError = retryOnError
     }
 
+    /**
+     * Sets the [ApolloInterceptor] used to retry or fail fast a request. The interceptor may use [ApolloRequest.retryOnError]
+     * and [ApolloRequest.failFastIfOffline].
+     * The interceptor is also responsible for allocating a new [ApolloRequest.requestUuid] on retries if needed.
+     *
+     * By default [ApolloClient] uses a best effort interceptor that is not aware about network state, uses exponential backoff
+     * and ignores [ApolloRequest.failFastIfOffline].
+     *
+     * Use [RetryOnErrorInterceptor] to add network state awareness:
+     *
+     * ```
+     * apolloClient = ApolloClient.Builder()
+     *                 .serverUrl("https://...")
+     *                 .retryOnErrorInterceptor(RetryOnErrorInterceptor(NetworkMonitor(context)))
+     *                 .build()
+     * ```
+     *
+     * @param retryOnErrorInterceptor the [ApolloInterceptor] to use for retrying or `null` to use the default interceptor.
+     *
+     * @see [RetryOnErrorInterceptor]
+     * @see [ApolloRequest.retryOnError]
+     * @see [ApolloRequest.failFastIfOffline]
+     */
+    @ApolloExperimental
+    fun retryOnErrorInterceptor(retryOnErrorInterceptor: ApolloInterceptor?) = apply {
+      this.retryOnErrorInterceptor = retryOnErrorInterceptor
+    }
+
     /**
      * Configures the [HttpMethod] to use.
      *
@@ -930,7 +945,7 @@ private constructor(
           .webSocketIdleTimeoutMillis(webSocketIdleTimeoutMillis)
           .wsProtocol(wsProtocolFactory)
           .retryOnError(retryOnError)
-          .networkMonitor(networkMonitor)
+          .retryOnErrorInterceptor(retryOnErrorInterceptor)
           .failFastIfOffline(failFastIfOffline)
           .listeners(listeners)
     }
diff --git a/libraries/apollo-runtime/src/commonMain/kotlin/com/apollographql/apollo3/interceptor/RetryOnNetworkErrorInterceptor.kt b/libraries/apollo-runtime/src/commonMain/kotlin/com/apollographql/apollo3/interceptor/RetryOnErrorInterceptor.kt
similarity index 68%
rename from libraries/apollo-runtime/src/commonMain/kotlin/com/apollographql/apollo3/interceptor/RetryOnNetworkErrorInterceptor.kt
rename to libraries/apollo-runtime/src/commonMain/kotlin/com/apollographql/apollo3/interceptor/RetryOnErrorInterceptor.kt
index d88d85e9bbe..7456fac3d5e 100644
--- a/libraries/apollo-runtime/src/commonMain/kotlin/com/apollographql/apollo3/interceptor/RetryOnNetworkErrorInterceptor.kt
+++ b/libraries/apollo-runtime/src/commonMain/kotlin/com/apollographql/apollo3/interceptor/RetryOnErrorInterceptor.kt
@@ -1,6 +1,5 @@
 package com.apollographql.apollo3.interceptor
 
-import com.apollographql.apollo3.ApolloClient
 import com.apollographql.apollo3.annotations.ApolloExperimental
 import com.apollographql.apollo3.api.ApolloRequest
 import com.apollographql.apollo3.api.ApolloResponse
@@ -22,15 +21,34 @@ import kotlin.time.Duration.Companion.seconds
 
 
 /**
- * An [ApolloInterceptor] that monitors network errors and possibly retries the [Flow] when an [ApolloNetworkException] happens.
+ * Returns an [ApolloInterceptor] that monitors network errors and possibly retries the [Flow] when an [ApolloNetworkException] happens.
  *
- * Some other types of error might be recoverable as well (rate limit, ...) but are out of scope for this interceptor.
+ * The returned [RetryOnErrorInterceptor]:
+ * - allocates a new [ApolloRequest.requestUuid] for each retry.
+ * - if [ApolloRequest.retryOnError] is `true`, waits until network is available and retries the request.
+ * - if [ApolloRequest.failFastIfOffline] is `true` and [NetworkMonitor.isOnline] is `false`, returns early with [ApolloNetworkException].
  *
- * If no network monitor is available, the retry algorithm uses exponential backoff
+ * Use with [com.apollographql.apollo3.ApolloClient.Builder.retryOnErrorInterceptor]:
  *
- * @param networkMonitor a network monitor or `null` if none available.
+ * ```
+ * apolloClient = ApolloClient.Builder()
+ *                 .serverUrl("https://...")
+ *                 .retryOnErrorInterceptor(RetryOnErrorInterceptor(NetworkMonitor(context)))
+ *                 .build()
+ * ```
+ *
+ * Some other types of error than [ApolloNetworkException] might be recoverable as well (rate limit, ...) but are out of scope for this interceptor.
+ *
+ * @see [com.apollographql.apollo3.ApolloClient.Builder.retryOnErrorInterceptor]
+ * @see [ApolloRequest.retryOnError]
+ * @see [ApolloRequest.failFastIfOffline]
  */
-internal class RetryOnNetworkErrorInterceptor(private val networkMonitor: NetworkMonitor?) : ApolloInterceptor {
+@ApolloExperimental
+fun RetryOnErrorInterceptor(networkMonitor: NetworkMonitor): ApolloInterceptor = DefaultRetryOnErrorInterceptorImpl(networkMonitor)
+
+internal fun RetryOnErrorInterceptor(): ApolloInterceptor = DefaultRetryOnErrorInterceptorImpl(null)
+
+private class DefaultRetryOnErrorInterceptorImpl(private val networkMonitor: NetworkMonitor?) : ApolloInterceptor {
   override fun <D : Operation.Data> intercept(request: ApolloRequest<D>, chain: ApolloInterceptorChain): Flow<ApolloResponse<D>> {
     val failFastIfOffline = request.failFastIfOffline ?: false
     val retryOnError = request.retryOnError ?: false
@@ -106,25 +124,3 @@ internal fun <T> Flow<Flow<T>>.flattenConcatPolyfill(): Flow<T> = flow {
   collect { value -> emitAll(value) }
 }
 
-private fun <D : Operation.Data> Flow<ApolloResponse<D>>.retryOnError(block: suspend (ApolloException, Int) -> Boolean): Flow<ApolloResponse<D>> {
-  var attempt = 0
-  return onEach {
-    if (it.exception != null && block(it.exception!!, attempt)) {
-      attempt++
-      throw RetryException
-    }
-  }.retryWhen { cause, _ ->
-    cause is RetryException
-  }
-}
-
-internal class RetryOnErrorInterceptor(private val retryWhen: suspend (ApolloException, Int) -> Boolean) : ApolloInterceptor {
-  override fun <D : Operation.Data> intercept(request: ApolloRequest<D>, chain: ApolloInterceptorChain): Flow<ApolloResponse<D>> {
-    return chain.proceed(request).retryOnError(retryWhen)
-  }
-}
-
-@ApolloExperimental
-fun ApolloClient.Builder.addRetryOnErrorInterceptor(retryWhen: suspend (ApolloException, Int) -> Boolean) = apply {
-  addInterceptor(RetryOnErrorInterceptor(retryWhen))
-}
\ No newline at end of file


Test patch:
diff --git a/libraries/apollo-runtime/src/androidInstrumentedTest/kotlin/instrumented/NetworkMonitorTest.kt b/libraries/apollo-runtime/src/androidInstrumentedTest/kotlin/instrumented/NetworkMonitorTest.kt
index d978205fdac..4e7a9d32cd7 100644
--- a/libraries/apollo-runtime/src/androidInstrumentedTest/kotlin/instrumented/NetworkMonitorTest.kt
+++ b/libraries/apollo-runtime/src/androidInstrumentedTest/kotlin/instrumented/NetworkMonitorTest.kt
@@ -6,6 +6,7 @@ import com.apollographql.apollo3.api.http.HttpResponse
 import com.apollographql.apollo3.exception.ApolloNetworkException
 import com.apollographql.mockserver.assertNoRequest
 import com.apollographql.mockserver.enqueueString
+import com.apollographql.apollo3.interceptor.RetryOnErrorInterceptor
 import com.apollographql.apollo3.network.NetworkMonitor
 import com.apollographql.apollo3.network.http.DefaultHttpEngine
 import com.apollographql.apollo3.network.http.HttpEngine
@@ -37,7 +38,9 @@ class NetworkMonitorTest {
   @Test
   fun test() = mockServerTest(
       clientBuilder = {
-        networkMonitor(NetworkMonitor(InstrumentationRegistry.getInstrumentation().context))
+        retryOnErrorInterceptor(
+            RetryOnErrorInterceptor(NetworkMonitor(InstrumentationRegistry.getInstrumentation().context))
+        )
         retryOnError { true }
         httpEngine(FaultyHttpEngine())
       }
diff --git a/libraries/apollo-runtime/src/commonTest/kotlin/test/network/NetworkMonitorTest.kt b/libraries/apollo-runtime/src/commonTest/kotlin/test/network/NetworkMonitorTest.kt
index 9fb41e53428..ba02dabeadd 100644
--- a/libraries/apollo-runtime/src/commonTest/kotlin/test/network/NetworkMonitorTest.kt
+++ b/libraries/apollo-runtime/src/commonTest/kotlin/test/network/NetworkMonitorTest.kt
@@ -8,6 +8,7 @@ import com.apollographql.apollo3.api.Operation
 import com.apollographql.apollo3.exception.ApolloNetworkException
 import com.apollographql.apollo3.interceptor.ApolloInterceptor
 import com.apollographql.apollo3.interceptor.ApolloInterceptorChain
+import com.apollographql.apollo3.interceptor.RetryOnErrorInterceptor
 import com.apollographql.mockserver.MockServer
 import com.apollographql.mockserver.assertNoRequest
 import com.apollographql.mockserver.enqueueString
@@ -37,7 +38,7 @@ class NetworkMonitorTest {
     val fakeNetworkMonitor = FakeNetworkMonitor()
 
     return mockServerTest(clientBuilder = {
-      networkMonitor(fakeNetworkMonitor)
+      retryOnErrorInterceptor(RetryOnErrorInterceptor(fakeNetworkMonitor))
       failFastIfOffline(true)
     }) {
 
diff --git a/libraries/apollo-runtime/src/commonTest/kotlin/test/network/WebSocketNetworkTransportTest.kt b/libraries/apollo-runtime/src/commonTest/kotlin/test/network/WebSocketNetworkTransportTest.kt
index 547b2835cad..8aa6608d9ed 100644
--- a/libraries/apollo-runtime/src/commonTest/kotlin/test/network/WebSocketNetworkTransportTest.kt
+++ b/libraries/apollo-runtime/src/commonTest/kotlin/test/network/WebSocketNetworkTransportTest.kt
@@ -2,12 +2,17 @@ package test.network
 
 import app.cash.turbine.test
 import com.apollographql.apollo3.ApolloClient
+import com.apollographql.apollo3.annotations.ApolloExperimental
+import com.apollographql.apollo3.api.ApolloRequest
+import com.apollographql.apollo3.api.ApolloResponse
+import com.apollographql.apollo3.api.Operation
 import com.apollographql.apollo3.exception.ApolloException
 import com.apollographql.apollo3.exception.ApolloNetworkException
 import com.apollographql.apollo3.exception.ApolloWebSocketClosedException
 import com.apollographql.apollo3.exception.DefaultApolloException
 import com.apollographql.apollo3.exception.SubscriptionOperationException
-import com.apollographql.apollo3.interceptor.addRetryOnErrorInterceptor
+import com.apollographql.apollo3.interceptor.ApolloInterceptor
+import com.apollographql.apollo3.interceptor.ApolloInterceptorChain
 import com.apollographql.apollo3.network.websocket.WebSocketNetworkTransport
 import com.apollographql.apollo3.network.websocket.closeConnection
 import com.apollographql.apollo3.testing.FooSubscription
@@ -28,7 +33,10 @@ import com.apollographql.mockserver.enqueueWebSocket
 import com.apollographql.mockserver.headerValueOf
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.delay
+import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.merge
+import kotlinx.coroutines.flow.onEach
+import kotlinx.coroutines.flow.retryWhen
 import okio.use
 import kotlin.test.Test
 import kotlin.test.assertEquals
@@ -304,7 +312,7 @@ class WebSocketNetworkTransportTest {
                 .serverUrl(mockServer.url())
                 .build()
         )
-        .addRetryOnErrorInterceptor { e, _ ->
+        .retryWhen { e, _ ->
           check(exception == null)
           exception = e
           true
@@ -410,3 +418,28 @@ fun mockServerWebSocketTest(customizeTransport: WebSocketNetworkTransport.Builde
         }
   }
 }
+
+private object RetryException : Exception()
+
+private fun <D : Operation.Data> Flow<ApolloResponse<D>>.retryOnError(block: suspend (ApolloException, Int) -> Boolean): Flow<ApolloResponse<D>> {
+  var attempt = 0
+  return onEach {
+    if (it.exception != null && block(it.exception!!, attempt)) {
+      attempt++
+      throw RetryException
+    }
+  }.retryWhen { cause, _ ->
+    cause is RetryException
+  }
+}
+
+internal class RetryOnErrorInterceptor(private val retryWhen: suspend (ApolloException, Int) -> Boolean) : ApolloInterceptor {
+  override fun <D : Operation.Data> intercept(request: ApolloRequest<D>, chain: ApolloInterceptorChain): Flow<ApolloResponse<D>> {
+    return chain.proceed(request).retryOnError(retryWhen)
+  }
+}
+
+@ApolloExperimental
+internal fun ApolloClient.Builder.retryWhen(retryWhen: suspend (ApolloException, Int) -> Boolean) = apply {
+  retryOnErrorInterceptor(RetryOnErrorInterceptor(retryWhen))
+}
\ No newline at end of file
diff --git a/libraries/apollo-runtime/src/jvmTest/kotlin/RetryWebSocketsTest.kt b/libraries/apollo-runtime/src/jvmTest/kotlin/RetryWebSocketsTest.kt
index 94b342bf3ca..574db4c83e3 100644
--- a/libraries/apollo-runtime/src/jvmTest/kotlin/RetryWebSocketsTest.kt
+++ b/libraries/apollo-runtime/src/jvmTest/kotlin/RetryWebSocketsTest.kt
@@ -1,10 +1,16 @@
 
 import app.cash.turbine.test
 import com.apollographql.apollo3.ApolloClient
+import com.apollographql.apollo3.annotations.ApolloExperimental
+import com.apollographql.apollo3.api.ApolloRequest
+import com.apollographql.apollo3.api.ApolloResponse
+import com.apollographql.apollo3.api.Operation
 import com.apollographql.apollo3.api.Subscription
+import com.apollographql.apollo3.exception.ApolloException
 import com.apollographql.apollo3.exception.ApolloHttpException
 import com.apollographql.apollo3.exception.ApolloNetworkException
-import com.apollographql.apollo3.interceptor.addRetryOnErrorInterceptor
+import com.apollographql.apollo3.interceptor.ApolloInterceptor
+import com.apollographql.apollo3.interceptor.ApolloInterceptorChain
 import com.apollographql.mockserver.MockResponse
 import com.apollographql.mockserver.MockServer
 import com.apollographql.mockserver.awaitWebSocketRequest
@@ -31,6 +37,7 @@ import kotlin.test.assertIs
 import kotlin.test.assertNotEquals
 import kotlin.time.Duration.Companion.seconds
 import test.network.awaitSubscribe
+import test.network.retryWhen
 
 class RetryWebSocketsTest {
   @Test
@@ -101,7 +108,7 @@ class RetryWebSocketsTest {
                 .serverUrl(mockServer.url())
                 .build()
         )
-        .addRetryOnErrorInterceptor { _, _ ->
+        .retryWhen { _, _ ->
           true
         }
         .build()
@@ -258,7 +265,7 @@ class RetryWebSocketsTest {
                 .build()
         )
         .serverUrl("https://unused.com/")
-        .addRetryOnErrorInterceptor { _, _ ->
+        .retryWhen { _, _ ->
           reopenCount++
           delay(500)
           true
@@ -325,4 +332,4 @@ class RetryWebSocketsTest {
       }
     }
   }
-}
\ No newline at end of file
+}