SpringBoot setReadTimeout can't be set via Reflective factory on JettyClientHttpRequestFactory

https://github.com/spring-projects/spring-boot/blob/8b8e263835206d50200abf1aced10be62c6fbfb7/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/client/ClientHttpRequestFactories.java#L394

The above line (in the Reflective factory) tries to find and invoke the setReadTimeout method with a parameter of type int. However, some of the used factories, e.g. the Jetty one, have other parameter types of this method. Jetty has long (here) and Duration (here).

Hence the readTimeout parameter can't be set via the Reflective factory if the real factory is the Jetty one.

IMO the code should look for "setReadTimeout" methods with different parameters (at least int, long and Duration), and call the first one found.

Comment From: scottfrederick

None of Spring Boot's built-in request factory implementations (HttpComponentsClientHttpRequestFactory, JettyClientHttpRequestFactory, JdkClientHttpRequestFactory, SimpleClientHttpRequestFactory, OkHttp3ClientHttpRequestFactory) use this Reflective class for setting properties. When Jetty is being used for the client connection, the read timeout is set here using the Jetty setReadTimeout method that takes a long:

https://github.com/spring-projects/spring-boot/blob/8b8e263835206d50200abf1aced10be62c6fbfb7/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/client/ClientHttpRequestFactories.java#L263

If you are somehow jave a ClientHttpRequestFactory implementation with Jetty that goes through the Reflective code path, can you show us an example of what you're doing?

Comment From: fml2

Yes, I do have such a path. We create a RestTemplate using the RestTemplateBuilder. We'd like every request sent by the template to be wrapped by a BufferingRequest (in order to be able to log the request before it's sent), but the nested request should be created as configured by other factories. Hence we use this:

restTemplateBuilder
    .requestFactory(() ->
        new BufferingClientHttpRequestFactory(ClientHttpRequestFactories.get(ClientHttpRequestFactorySettings.DEFAULTS)))

This form of the "requestFactory" method uses Reflective.

Comment From: mhalbritter

Reproducer here: sb-40635.zip

Works with Boot 3.1.x, fails with 3.2.x and 3.3.x:

java.lang.IllegalStateException: Request factory org.springframework.http.client.JettyClientHttpRequestFactory does not have a suitable setReadTimeout method
    at org.springframework.util.Assert.state(Assert.java:97) ~[spring-core-6.1.6.jar:6.1.6]
    at org.springframework.boot.web.client.ClientHttpRequestFactories$Reflective.findMethod(ClientHttpRequestFactories.java:402) ~[spring-boot-3.3.0-RC1.jar:3.3.0-RC1]
    at org.springframework.boot.web.client.ClientHttpRequestFactories$Reflective.setReadTimeout(ClientHttpRequestFactories.java:394) ~[spring-boot-3.3.0-RC1.jar:3.3.0-RC1]
    at org.springframework.boot.web.client.ClientHttpRequestFactories$Reflective.lambda$configure$1(ClientHttpRequestFactories.java:369) ~[spring-boot-3.3.0-RC1.jar:3.3.0-RC1]
    at org.springframework.boot.context.properties.PropertyMapper$Source.to(PropertyMapper.java:294) ~[spring-boot-3.3.0-RC1.jar:3.3.0-RC1]
    at org.springframework.boot.web.client.ClientHttpRequestFactories$Reflective.configure(ClientHttpRequestFactories.java:369) ~[spring-boot-3.3.0-RC1.jar:3.3.0-RC1]
    at org.springframework.boot.web.client.ClientHttpRequestFactories$Reflective.get(ClientHttpRequestFactories.java:360) ~[spring-boot-3.3.0-RC1.jar:3.3.0-RC1]
    at org.springframework.boot.web.client.ClientHttpRequestFactories.get(ClientHttpRequestFactories.java:168) ~[spring-boot-3.3.0-RC1.jar:3.3.0-RC1]
    at org.springframework.boot.web.client.RestTemplateBuilder.lambda$requestFactory$1(RestTemplateBuilder.java:320) ~[spring-boot-3.3.0-RC1.jar:3.3.0-RC1]
    at org.springframework.boot.web.client.RestTemplateBuilder.buildRequestFactory(RestTemplateBuilder.java:659) ~[spring-boot-3.3.0-RC1.jar:3.3.0-RC1]
    at org.springframework.boot.web.client.RestTemplateBuilder.configure(RestTemplateBuilder.java:625) ~[spring-boot-3.3.0-RC1.jar:3.3.0-RC1]
    at org.springframework.boot.web.client.RestTemplateBuilder.build(RestTemplateBuilder.java:600) ~[spring-boot-3.3.0-RC1.jar:3.3.0-RC1]
    at org.example.sb40635.CLR.run(CLR.java:31) ~[main/:na]
    at org.springframework.boot.SpringApplication.lambda$callRunner$5(SpringApplication.java:790) ~[spring-boot-3.3.0-RC1.jar:3.3.0-RC1]
    at org.springframework.util.function.ThrowingConsumer$1.acceptWithException(ThrowingConsumer.java:83) ~[spring-core-6.1.6.jar:6.1.6]
    at org.springframework.util.function.ThrowingConsumer.accept(ThrowingConsumer.java:60) ~[spring-core-6.1.6.jar:6.1.6]
    at org.springframework.util.function.ThrowingConsumer$1.accept(ThrowingConsumer.java:88) ~[spring-core-6.1.6.jar:6.1.6]
    at org.springframework.boot.SpringApplication.callRunner(SpringApplication.java:798) ~[spring-boot-3.3.0-RC1.jar:3.3.0-RC1]
    at org.springframework.boot.SpringApplication.callRunner(SpringApplication.java:789) ~[spring-boot-3.3.0-RC1.jar:3.3.0-RC1]
    at org.springframework.boot.SpringApplication.lambda$callRunners$3(SpringApplication.java:774) ~[spring-boot-3.3.0-RC1.jar:3.3.0-RC1]
    at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:183) ~[na:na]
    at java.base/java.util.stream.SortedOps$SizedRefSortingSink.end(SortedOps.java:357) ~[na:na]
    at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:510) ~[na:na]
    at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499) ~[na:na]
    at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:150) ~[na:na]
    at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:173) ~[na:na]
    at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234) ~[na:na]
    at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:596) ~[na:na]
    at org.springframework.boot.SpringApplication.callRunners(SpringApplication.java:774) ~[spring-boot-3.3.0-RC1.jar:3.3.0-RC1]
    at org.springframework.boot.SpringApplication.run(SpringApplication.java:341) ~[spring-boot-3.3.0-RC1.jar:3.3.0-RC1]
    at org.springframework.boot.SpringApplication.run(SpringApplication.java:1354) ~[spring-boot-3.3.0-RC1.jar:3.3.0-RC1]
    at org.springframework.boot.SpringApplication.run(SpringApplication.java:1343) ~[spring-boot-3.3.0-RC1.jar:3.3.0-RC1]
    at org.example.sb40635.Sb40635Application.main(Sb40635Application.java:10) ~[main/:na]

Comment From: mhalbritter

The ClientHttpRequestFactory we support have the following signatures:

HttpComponentsClientHttpRequestFactory - setConnectTimeout int - setConnectTimeout Duration - no setReadTimeout

JettyClientHttpRequestFactory - setConnectTimeout int - setConnectTimeout Duration - setReadTimeout Duration - setReadTimeout long

JdkClientHttpRequestFactory - no setConnectTimeout - setReadTimeout Duration - setReadTimeout int

SimpleClientHttpRequestFactory - setConnectTimeout int - setConnectTimeout Duration - setReadTimeout Duration - setReadTimeout int

OkHttp3ClientHttpRequestFactory - setConnectTimeout int - setConnectTimeout Duration - setReadTimeout Duration - setReadTimeout int

We can fix this bei either preferring the Duration accepting methods or by getting the setReadTimeout fixed in JettyClientHttpRequestFactory to also accept an integer.

We currently assume in the Reflective class that the int methods accept milliseconds:

int timeout = Math.toIntExact(readTimeout.toMillis());

I think preferring the Duration accepting methods would be the better fix.

Comment From: fml2

We currently assume in the Reflective class that the int methods accept milliseconds

I'd also search for a method with a long parameter since this type is often used for expressing periods in milliseconds.