spring-projects / spring-boot

Spring Boot helps you to create Spring-powered, production-grade applications and services with absolute minimum fuss.
https://spring.io/projects/spring-boot
Apache License 2.0
75.33k stars 40.72k forks source link

Consider providing a property to configure or select the preferred ClientHttpConnector #34962

Closed ciscoo closed 1 year ago

ciscoo commented 1 year ago

Currently, ClientHttpConnectorConfiguration configures a ClientHttpConnector based on what is available on the classpath. This works fine for the main application code, but can become an issue if dependencies bring in dependencies that activate the various client connector configuration.

This is the case when using WireMock. At my company, we use WireMock to mock out underlying backend/downstream calls for our test suites.

When migrating to HTTP Interface clients (from Spring Cloud OpenFeign), I noticed that in the main application code, JdkClient was being configured as I would expect.

But in tests, when WireMock is on the test classpath, HttpClient5 was being configured due to WireMock having a hard dependency on Apache HTTP related components. Attempting to exclude them leads to other classpath issues

I am suspecting that due to the ordering of the configuration where ClientHttpConnectorConfiguration.HttpClient5 is defined before ClientHttpConnectorConfiguration.JdkClient, HttpClient5 gets configured and as a result leads to the following classpath issue during test execution:

java.lang.NoClassDefFoundError: org/apache/hc/core5/reactive/ReactiveResponseConsumer
    at org.springframework.http.client.reactive.HttpComponentsClientHttpConnector.lambda$execute$2(HttpComponentsClientHttpConnector.java:125) ~[spring-web-6.0.7.jar:6.0.7]
    at reactor.core.publisher.MonoCreate.subscribe(MonoCreate.java:58) ~[reactor-core-3.5.4.jar:3.5.4]
    at reactor.core.publisher.MonoDefer.subscribe(MonoDefer.java:52) ~[reactor-core-3.5.4.jar:3.5.4]
...

Caused by: java.lang.ClassNotFoundException: org.apache.hc.core5.reactive.ReactiveResponseConsumer
    at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:641) ~[na:na]
    at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:188) ~[na:na]
    at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:521) ~[na:na]
    ... 67 common frames omitted

Including the following dependency fixes the issue:

testImplementation("org.apache.httpcomponents.core5:httpcore5-reactive") {
    because("Required by WireMock otherwise causes classpath issues")
}

It would be nice if Spring Boot could offer a property to specify the preferred ClientHttpConnector implementation in addition to what is available on the classpath.

The alternatives without the property are:

  1. Attempt to exclude transitive dependencies so that the correct ClientHttpConnector is selected.
  2. Define an explicit bean in the main application code for the preferred implementation (below)
@Bean
public JdkClientHttpConnector jdkConnector() {
    return new JdkClientHttpConnector();
}

Here is a sample project demonstrating the failing test with WireMock. Either define a JdkClientHttpConnector bean in the main code or add the above httpcore5-reactive dependency.

I don't think adding a property would add any meaningful value since the same can be accomplished by declaring an explicit bean, but wanted to hear your thoughts on the proposal.

wilkinsona commented 1 year ago

Thanks for the report, @ciscoo. Your analysis is correct. In order of preference, the connectors that will be used are the following:

  1. Reactor Netty
  2. Jetty Client
  3. Apache Http Client 5
  4. JDK Client

Defining your own bean is the currently recommended way of overriding this behavior.

java.lang.NoClassDefFoundError: org/apache/hc/core5/reactive/ReactiveResponseConsumer
  at org.springframework.http.client.reactive.HttpComponentsClientHttpConnector.lambda$execute$2(HttpComponentsClientHttpConnector.java:125) ~[spring-web-6.0.7.jar:6.0.7]
  at reactor.core.publisher.MonoCreate.subscribe(MonoCreate.java:58) ~[reactor-core-3.5.4.jar:3.5.4]
  at reactor.core.publisher.MonoDefer.subscribe(MonoDefer.java:52) ~[reactor-core-3.5.4.jar:3.5.4]

This looks like a bug to me. We should not be trying to use the Apache Client unless all of the required classes are available. I have opened https://github.com/spring-projects/spring-boot/issues/34964.

With regards to the benefits of adding a property, we have similar behavior with RestTemplate. By default, the auto-configured RestTemplateBuilder will detect the client request factory to use based on the classpath. There, the order of preference is the following:

  1. Apache Http Client 5
  2. Ok HTTP
  3. Simple (JDK URLConnection)

Similarly, you can override this behavior by providing your own RestTemplateBuilder bean, configured with your preferred request factory.

I mention the RestTemplate side of things as, if we were to offer a property for ClientHttpConnector, I think we'd want to offer one for the ClientHttpRequestFactory as well.

I'm a little unsure as to how much value a property would offer over defining your own bean. The advantage of the bean-based approach is that it's type safe – you cannot configure a connector or request factory that isn't on the classpath – but a property would make it easy to configure things incorrectly. I'll flag this for discussion with the team so that we can decide what to do.

philwebb commented 1 year ago

We discussed this today as a team and decided that we'd rather not offer a property to configure this. Defining a custom bean remains our preferred approach.