square / retrofit

A type-safe HTTP client for Android and the JVM
https://square.github.io/retrofit/
Apache License 2.0
42.82k stars 7.29k forks source link

Retrofit Interceptors #3202

Open alecholmes opened 4 years ago

alecholmes commented 4 years ago

Like OkHttp Interceptors, but exposing things at Retrofit’s layer of abstraction instead of OkHttp’s.

We’ve done our own thing in Misk and it’s unsatisfying because we need to build our own dynamic proxy in front of Retrofit’s dynamic proxy. https://github.com/cashapp/misk/blob/master/misk/src/main/kotlin/misk/client/ClientApplicationInterceptor.kt

alecholmes commented 4 years ago

Motivating use cases:

JakeWharton commented 4 years ago

Deadlines can be supported on Retrofit's Call which lazily creates the backing OkHttp Call by exposing an Okio Timeout. I started work on this in a branch, jakew/timeouts/2019-02-28, but never got around to finishing it.

I'm not clear on what your tracing need entails specifically, but perhaps this is better supported by something like an event listener at the Retrofit level akin to OkHttp's?

I'm skeptical of the benefit of interceptors in the same vein as OkHttp. Retrofit does not expose a model object that can be manipulated in the way OkHttp does, so I have a hard time seeing a hook as general as an interceptor being useful. You couldn't add or remove parameters or change the types of serialization. Its unclear whether the underlying OkHttp Request / Response would be available, but I would lean toward no.

I'm open to trying to support the use cases you have, but I don't think the interceptor model fits well at Retrofit's level of abstraction unless we expose a much larger API surface area (which I'm also hesitant about).

alecholmes commented 4 years ago

Thanks @JakeWharton. Your timeout branch would be useful and is something we'd use in Misk, since we don't have a clean way of doing it at the moment.

Misk tacked interceptors onto Retrofit but the way it's done is a bit tortured and required creating proxies for the Retrofit-created classes. Having this built into Retrofit would be nice.

To give a sense of how we're intercepting Retrofit calls, here are some use cases we've implemented. None mutate the object model but some mutate the HTTP request.

I agree that carte blanch to change anything about the request, like serialization, doesn't make sense.