conjure-java-runtime icon indicating copy to clipboard operation
conjure-java-runtime copied to clipboard

Not compatible with okhttp 4.x

Open AlexLandau opened this issue 4 years ago • 3 comments

What happened?

We had a repo where com.squareup.okhttp3:okhttp:4.9.0 was in use due to a dependency on org.cometd.java:cometd-java-client-websocket-okhttp:5.0.2. This resulted in the following runtime error:

java.lang.VerifyError: class com.palantir.conjure.java.okhttp.ForwardingOkHttpClient overrides final method cookieJar.()Lokhttp3/CookieJar;

Per https://square.github.io/okhttp/upgrading_to_okhttp_4/#backwards-incompatible-changes:

OkHttpClient has 26 accessors like interceptors() and writeTimeoutMillis() that were non-final in OkHttp 3.x and are final in 4.x. These were made non-final for use with mocking frameworks like Mockito. We believe subtyping OkHttpClient is the wrong way to test with OkHttp. If you must, mock Call.Factory which is the interface that OkHttpClient implements.

This obviously doesn't cover what the RemotingOkHttpClient is doing (as it's not test code), so I don't know if this will turn out to be hard to fix.

Currently I'm working around this by down-pinning to a 3.x version of okhttp.

What did you want to happen?

Compatibility with okhttp 4.x.

AlexLandau avatar Apr 05 '21 23:04 AlexLandau

Presumably this is most likely going to be fixed by removing support for retrofit: https://github.com/palantir/conjure-java-runtime/pull/1853

AlexLandau avatar Apr 05 '21 23:04 AlexLandau

That’s correct, we don’t use okhttp in our recommended clients any longer, but there are still a few places that use it through retrofit. We have an active project to prodice an annotation processor which can replace retrofit uses. I’m hoping to bundle the breaks with retrofit removal and remove okhttp from most classpaths where it’s pulled in via conjure.

carterkozak avatar Apr 05 '21 23:04 carterkozak

This should help in the more immediate future: https://github.com/palantir/conjure-java-runtime/pull/1955 WC no longer depends on retrofit, though some projects have opted into it, but it does still depend on the jaxrs module. This will remove that dependency by default, unless it's explicitly requested.

carterkozak avatar Apr 26 '21 19:04 carterkozak