Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Not compatible with okhttp 4.x #1933

Closed
AlexLandau opened this issue Apr 5, 2021 · 3 comments · Fixed by #2709
Closed

Not compatible with okhttp 4.x #1933

AlexLandau opened this issue Apr 5, 2021 · 3 comments · Fixed by #2709

Comments

@AlexLandau
Copy link

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
Copy link
Author

Presumably this is most likely going to be fixed by removing support for retrofit: #1853

@carterkozak
Copy link
Contributor

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
Copy link
Contributor

This should help in the more immediate future: #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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants