add ApolloClient.Builder.retryOnErrorInterceptor (#5989)

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>

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())
       }

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)
     }

libraries/apollo-runtime/src/commonMain/kotlin/com/apollographql/apollo3/interceptor/RetryOnNetworkErrorInterceptor.kt → 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))
-}

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)
     }) {
 

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))
+}

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 {
       }
     }
   }
-}
+}