google / cronet-transport-for-okhttp

This package allows OkHttp and Retrofit users to use Cronet as their transport layer, benefiting from features like QUIC/HTTP3 support or connection migration.
Apache License 2.0
452 stars 34 forks source link

adding it as an OkHttp interceptor, breaks okhttp3.Authenticator #11

Closed marius-bardan closed 1 year ago

marius-bardan commented 1 year ago
OkHttpClient.Builder builder = new OkHttpClient.Builder()
  .authenticator(authenticator)
  .addInterceptor(cronetInterceptor);

OkHttpClient client = builder.build();

If OkHttpClient is created as per above and request returns with 401 (Unauthorized), then the passed okhttp3.Authenticator is no longer invoked, since that's basically implemented as an interceptor (RetryAndFollowUpInterceptor) added after the cronet interceptor is set (i.e. when the request is executed), as it can be seen in the attached screenshot.

Due to this, the user session won't be refreshed.

image
Danstahrg commented 1 year ago

This is an unfortunate known limitation of OkHttp interceptors: https://github.com/google/cronet-transport-for-okhttp#common-incompatibilities, and there's not much we can do about it from this library's standpoint. The only alternative approach I'm aware of would be to add the Cronet interceptor as a network interceptor but those are handled after after OkHttp's connection level logic (e.g. establishing and pooling connections) so it comes with its own drawbacks.

The existence of this library uncovered some limitations which are being addressed (https://github.com/square/okhttp/issues/7164) so there's hope to address this properly, but realistically I won't have time to start any actual work before the end of the year.

marius-bardan commented 1 year ago

Maybe it might prove useful to also state this in the Readme page (i.e. that the OkHttp 'authenticator' will also get bypassed).

The entirety of OkHttp core is bypassed. This includes caching, retries, and network interceptors. These features have to be enabled directly on the Cronet engine or built on top of this library.

Danstahrg commented 1 year ago

That surely doesn't hurt, done.