[3.7.4~] Exception thrown from ApolloInterceptor became uncatchable

[omtians9425]

Version

3.4.0~ (Confirmed it also occurs in 3.8.0)

Summary

Previously (~3.7.3) it was possible to handle the exceptions thrown from custom ApolloInterceptor, but now they were no longer caught and caused the app to crash.

Steps to reproduce the behavior

My Apollo Client

val apolloClient: ApolloClient = ApolloClient.Builder()
        .addCustomScalarAdapter(ISO8601Date.type, dateCustomScalarAdapter)
        .addCustomScalarAdapter(ISO8601DateTime.type, dateTimeCustomScalarAdapter)
        .autoPersistedQueries(httpMethodForHashedQueries = HttpMethod.Get)
        .addInterceptor(...)
        .okHttpClient(okHttpClient)
        .addInterceptor(ApolloNetworkConnectivityInterceptor(context)) // 👈
        .addInterceptor(...)
        .serverUrl(serverUrl)
        .build()
}

The interceptor: throws an exception on no connectivity

class ApolloNetworkConnectivityInterceptor(private val context: Context) : ApolloInterceptor {

    override fun <D : Operation.Data> intercept(
        request: ApolloRequest<D>,
        chain: ApolloInterceptorChain
    ): Flow<ApolloResponse<D>> {
        if (context.isNetworkConnected()) {
            return chain.proceed(request)
        } else {
            throw NoNetworkException() // 👈
        }
    }
}

Previously, we can catch its exception:

apolloClient.query(HogeQuery()).toFlow()
    .map{
        Result.Ok(it)
    }.onStart {
         emit(Result.Loading)
    }.catch {
         emit(Result.Error(it)) // 👈 Exception caught here
    }

but now the app crashes.

It seems to be related to the order of APQ interceptor. From my research, reverting this PR on tag v.3.7.4 fixes this issue: #4628

Logs

E  FATAL EXCEPTION: main
Process: sample.app, PID: 24746
java.lang.RuntimeException: java.lang.reflect.InvocationTargetException
at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:558)
at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1003)

Caused by: java.lang.reflect.InvocationTargetException
at java.lang.reflect.Method.invoke(Native Method)
at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:548)
at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1003) 

Caused by: sample.app.NoNetworkException
at sample.app.apollo.ApolloNetworkConnectivityInterceptor.intercept(ApolloNetworkConnectivityInterceptor.kt:22)
at com.apollographql.apollo3.interceptor.DefaultInterceptorChain.proceed(ApolloInterceptor.kt:23)
at sample.app.apollo.ApolloClientBuilder$apolloClient$1.intercept(ApolloClientBuilder.kt:83)
at com.apollographql.apollo3.interceptor.DefaultInterceptorChain.proceed(ApolloInterceptor.kt:23)
at com.apollographql.apollo3.ApolloClient.executeAsFlow(ApolloClient.kt:178)
at com.apollographql.apollo3.ApolloCall.toFlow(ApolloCall.kt:97)

[BoD]

Thank you for reporting this!

To have an interceptor throw an exception, you shouldn't throw directly in intercept but instead return a Flow that throws the exception:

class ApolloNetworkConnectivityInterceptor(private val context: Context) : ApolloInterceptor {

    override fun <D : Operation.Data> intercept(
        request: ApolloRequest<D>,
        chain: ApolloInterceptorChain
    ): Flow<ApolloResponse<D>> {
        return if (context.isNetworkConnected()) {
            chain.proceed(request)
        } else {
            flow {
                throw NoNetworkException() // 👈
            }
        }
    }
}

I am not entirely sure why throwing directly used to work in previous versions, but I do believe this way makes more sense.

Also, talking about exception handling, heads-up that we are planning changes in this area in the upcoming major version 4.0 - you can have a look at this issue to have more information.

[omtians9425]

@BoD Thanks for your kind advice ❤️
Returning Flow solved the issue!!

exception handling, heads-up that we are planning changes in this area in the upcoming major version 4.0

Let me read it 🚀