Got a question regarding apollo-kotlin and using a...

# apollo-kotlin

[Stylianos Gakis]

Got a question regarding apollo-kotlin and using an okhttp3.Authenticator with it.
So my use case is as such. We use okhttp interceptors to append the Auth token on our requests, and an okhttp3.Authenticator to respond to 401s by refreshing token, retrying the request and so on.
Thing is, apparently sometimes we get the 401 inside the graphql response with a 200 header so we can’t entirely rely on the okhttp3.Authenticator to check these not authorized cases, but we need an okhttp interceptor on top of it to catch those cases too.
Is my best bet to simply use a com.apollographql.apollo3.interceptor.ApolloInterceptor, look at the response body for the 401 in the error codes, and do what my okhttp3.Authenticator would do? Is it weird that our backend responds with 200 but 401 in the error message itself? I also worry about concurrent access from both my okhttp3.Authenticator and my com.apollographql.apollo3.interceptor.ApolloInterceptor both trying to get the refresh token and create a new access token. I guess I’d also need to manage the concurrency by locking on whatever class they both access, instead of locking inside the two authenticators/interceptors themselves.

Just asking here to see if I am going in the right direction or if I am missing something super obvious that would make my life easier 🤩

[mbonnin]

"sometimes" sounds a bit scary. If you have the option, I'd confirm and document what your backend expects and the cases for sending 401 as HTTP status code vs GraphQL error

[mbonnin]

If you need to handle both cases, I'd use an com.apollographql.apollo3.interceptor.ApolloInterceptor and catch ApolloHttpException in there to do everything in a single place

[mbonnin]

This way, you have both response.errors (in the HTTP 200 case) and the ApolloHttpException.statusCode (in HTTP 401 case)

[Stylianos Gakis]

Sometimes does indeed sound scary, thing is we use the same OkHttpClient instance for some normal Http calls, where that one may respond 401 normally, but when we use it through the ApolloClient instance then it comes as a http GraphQL error, hence our current need to handle both cases.

So catching everything in the ApolloHttpException case sounds like smth we can’t due since we use the raw OkHttpClient sometimes too.

As a side not, since this is an option, I guess not going with a okhttp3.Authenticator is great since then we can avoid having to do a runBlocking to enter the coroutines world since the apollo interceptor is suspending itself while the okhttp3 one is not.

[mbonnin]

when we use it through the ApolloClient instance then it comes as a http error

Did you mean "comes as a GraphQL error"?

[Stylianos Gakis]

Yes, sorry!

[mbonnin]

👍

[mbonnin]

Then I guess there's no other way than handling this in 2 places

[mbonnin]

Handling the concurrency of authentication interceptors is a surprinsigly difficult task

[mbonnin]

Even if you have only one authenticator class, making its main refreshToken() method synchronized is not optimal because you might end up with multiple request queued on the lock and all of them refreshing the token. So when your token expires you refresh it 2, 3 or more times

[Stylianos Gakis]

Yeah I think I do understand how this can be tricky, especially with 2 classes doing this.
Currently my okhttp authenticator looked smth like this https://github.com/HedvigInsurance/android/blob/809546a3af3fc63872f0e1b00a8f9d130a[…]com/hedvig/android/auth/interceptor/AccessTokenAuthenticator.kt where first the current token is received without locking, then inside the lock, check it again (if it’s changed assumed someone else just refreshed it) but if not take the time to refresh it and then use it, which this lets the rest of the calls waiting for the lock to then get the new token.
It had worked for me as far as I know, but dunno if I’m missing tricky edge cases. Now if I need to synchronize across two interceptors I’ll have to see what I figure out 😅 Don’t have a clear plan atm in my mind. Will probably have to introduce a 3rd class which handles the synchronization with locking, and have these two defer to it.

[mbonnin]

I think the same thing would work?

[mbonnin]

Technically, the GraphQL interceptor isn't different from another call to the HTTP interceptor? If you use the same lock for both it should continue working fine?

[Stylianos Gakis]

Yeah, just gotta keep the lock at a third place, since I have to have both interceptors have access to it right? I think it should work, just gotta sit down and do it 😄

[mbonnin]

Concurrency is hard 😅

[mbonnin]

But I think that should do it 👍

[mbonnin]

One thing that you will bump into is that interceptors are Request -> Flow<Response> for cache/`@defer` purposes. If you put your interceptor after configuring the cache and not using @defer, it should be safe to assume a single response

[Stylianos Gakis]

Awesome, so just to make sure I understand you there correctly, I want the interceptor to be after the cache configuration right? We’re not using @defer right now, but who knows if we might in the future, so don’t want to mess this up 😄

[mbonnin]

I want the interceptor to be after the cache configuration right?

Yes, correct ✅

[mbonnin]

I'm not sure about the @defer details but if you end up using it, please reach out, that'll open some interesting questions 🙂

[Stylianos Gakis]

Heh alright. I don’t think we will going forward, not quite in an exploratory mode right now, but I’d certainly let you know if we do and it interacts weirdly with the interceptor

[wasyl]

About using the same okhttp instance, you can perhaps inspect request headers and only do the token refreshing flow in okhttp interceptor for non-Apollo requests, and do it in Apollo interceptor for Apollo requests? Afair you should see X-APOLLO-OPERATION-NAME header for every Apollo request

[Stylianos Gakis]

So an update on this since I already started this conversation and not to leave you hanging 😄
Turns out, we were even getting “INTERNAL_SERVER_ERROR” in the error messages even in the cases where we were just unauthenticated, so had to scratch this idea completely, there was no way to make it work.
Now instead we’re working with the expiration dates, and have an interceptor which looks at the stored ones, refreshes if there’s a need, deletes them if access and refresh token are expired, and appends the necessary header if it’s all good. And we simply assume we’re good regarding authentication if the access token is valid. They’re super short lived so that’s a fair assumption to make most of the time.

So yeah, went with a straight okhttp interceptor and unfortunately had to do a runBlocking in there to access the datastore, but it seems to work 😄

Thanks for the help and the advice, too bad I couldn’t put it to good use.