playframework/play-ws

Exceptions when using together with AsyncHttpClient

Closed this issue · 11 comments

Sorry for cross-posting. I've written down the details of this issue at:
playframework/playframework#7056

It looks like (at least in my Play 2.6.0-M1 app) that the shaded AsyncHttpClientDefaults class reads the ahc-default.properties that are pulled in from the non-shaded version on my classpath.

The issue probably comes from
https://github.com/AsyncHttpClient/async-http-client/blob/async-http-client-project-2.0.27/client/src/main/java/org/asynchttpclient/config/AsyncHttpClientConfigHelper.java#L47

Where the current ThreadContext Classloader is used to lookup the properties instead a classloader based on the current class (which would then find the ahc-default.properties with the play.shaded prefix.

So what we would need to do is to also rename the ahc-default.properties to something like play.shaded-ahc-default.properties and replace references to it.
I can provide a PR for that if that is OK for you @wsargent / @marcospereira

Yes please.

Ok, unfortunately, "jar jar links" only supports changing the package name of the classes transformed. I've managed to rename the ahc-default.properties and ahc.properties files themselves, but unfortunately, references to these are hardcoded (as constants) in
https://github.com/AsyncHttpClient/async-http-client/blob/async-http-client-project-2.0.27/client/src/main/java/org/asynchttpclient/config/AsyncHttpClientConfigHelper.java#L33
respectively in
https://github.com/AsyncHttpClient/async-http-client/blob/async-http-client-project-2.0.27/client/src/main/java/org/asynchttpclient/config/AsyncHttpClientConfigHelper.java#L37 / https://github.com/AsyncHttpClient/async-http-client/blob/async-http-client-project-2.0.27/client/src/main/java/org/asynchttpclient/config/AsyncHttpClientConfigHelper.java#L41

Unfortunately, the whole AysncHttpClientConfigHelper is written in a way that does not really allow outside modification/configuration. We could try to exclude it in the shading process and provide our own version of it, but that feels like a rather bad hack.

A alternative to shading the async-http-client on a class-file level could be to include the async-http-client project as a sbt-source-dependency .. that way it might be possible to basically take the files and rewrite them directly (and thus making also changes to the constants in AsyncHttpClientConfigHelper .. but its another "hack".

@domdorn do you have a PR for this?

@wsargent I tried to adjust the shading process, but the shading library used only allows simple byte-code changes (changing package names) -> I was unable to change the constant.
I once did something similar with cglib, but I'm not sure we want to go down that route.

Hmm... I've got an idea... what if instead of replacing the variables in the ahc.properties, we're just adding to them?
so instead of only writing
play.shaded.ahc.org.asynchttpclient.threadPoolName=AsyncHttpClient
to the file, we write

play.shaded.ahc.org.asynchttpclient.threadPoolName=AsyncHttpClient
org.asynchttpclient.threadPoolName=AsyncHttpClient

to the file? That way, we still be providing the default file, and also provide our own properties. People can still change their settings in the other config file. WDYT?

Okay, I can address that in a patch release.

@wsargent I have a PR for this: #149

Closing by way of #149