I'd like for WebClient and/or ClientRequest to have first-class support for an HTTP request timeout. (i.e. the time it takes to receive a response after sending a request). Something similar to the java http client's request timeout.
The currently documented use of netty's ReadTimeoutHandler / WriteTimeoutHandler is insufficient to use as an HTTP request timeout. Specifically they apply at the TCP level, which leads to problems like this:
1. They apply during the SSL handshake, which might take longer than a typical HTTP response, due to the cryptography involved. Therefore, they would need to be set higher than desired for HTTP responses.
2. They apply even when an HTTP request is not being processed. For example, they could cause a connection sitting in the connection pool to be closed, even though it might be able to be used a split-second later by another request.
The .timeout operator on the reactive stream is insufficient to use as an HTTP request timeout as well. Specifically, it operates on the reactive stream, which includes things like obtaining a connection from the connection pool and potentially creating a new connection, in addition to the time it takes the client to receive a response. This leads to having to use a .timeout value this is greater than the connection timeout plus the time to obtain the connection from the connection pool. So, the .timeout operator cannot be used as an HTTP request timeout. I'm willing to "pay the price" of establishing connections occasionally (leading to waiting longer occasionally for the stream to emit), but I'd still like to set a lower HTTP request timeout, to ensure the stream emits values fast when a new connection is not established.
Comment From: rstoyanchev
This seems more like something to be exposed at the HTTP client library level. In the WebClient we could insert a .timeout(..) at the point of receiving the response but that would include obtaining the connection. We could also add a .timeout(..) after the writing of the request but there is still a time period after the request is written and before the response is received. So I don't think we even have the option to provide this.
@violetagg is there anything at the level of Reactor Netty that could be used to achieve this?
Comment From: violetagg
@rstoyanchev @philsttr No there is no such timeout, but I think that it is better this functionality to be exposed by Reactor Netty
Comment From: philsttr
I opened https://github.com/reactor/reactor-netty/issues/1159.
Feel free to close this issue, or leave it open if there are any changes needed at WebClient level (e.g. for specifying request-specific timeouts, as opposed to a default timeout for the http client)
Comment From: rstoyanchev
We can re-open if needed.
Comment From: rstoyanchev
Now that https://github.com/reactor/reactor-netty/pull/1216 provides an option at the level of the Reactor Netty request, there needs to be some way for applications to access it.
Comment From: rstoyanchev
Both this and #25493 would benefit from having some way to customze the request from the underlying HTTP library. An alternative is to have a timeout property in the connector but that can only apply to all requests.
@philsttr would it be sufficient for your needs if the connector could be configured with Consumer<HttpClientRequest> registrations similar to HttpClient#doOnRequest in which case you would have to check the request details and decide what timeout to apply. Or otherwise we could expose a similar hook in WebClient which would be slightly more complicated for us to implement but possible.
Comment From: philsttr
I think in a perfect world:
* I could specify the default response timeout for the WebClient, when building the WebClient (e.g. WebClient.builder().defaultResponseTimeout(...)
* I could specify a request-specific response timeout when building the request (e.g. webClient.get().responseTimeout(...))
* I could specify a request-specific response timeout on a ClientRequest in an ExchangeFilterFunction (e.g. (request, next) -> next.exchange(ClientRequest.from(request).responseTimeout(...).build()))
However, I think I can probably get by with only having a Consumer<HttpClientRequest> specified on the connector IF it had access to the request attributes that were specified on webflux's ClientRequest. This would allow me to
1. set a request attribute when building a request (webClient.get().attribute(...)), or within an ExchangeFilterFunction ((request, next) -> next.exchange(ClientRequest.from(request).attribute(...).build())), and
2. have the Consumer read the attribute and set the response timeout on the HttpClientRequest. (maybe webflux could define a well known attribute name for response timeout, and do this step for me (?))
Comment From: rstoyanchev
Such specific properties might not be possible across all client connectors some of which may not even be maintained by us. Furthermore any other config option would be fair game but finding a common abstraction across all libraries would not be feasible. This is why typically we leave this to be configured at the HTTP library level.
I was thinking more about where to expose the native request. It makes sense to be able to check attributes. We could make it a BiConsumer then with access to both the Spring ClientRequest and the native request which would allow you to check attributes. Depending on the use case it could also work to mutate the WebClient and create one or two replicas each with slightly different configuration and then just use the right one for a given scenario.
Comment From: rstoyanchev
BiConsumer of ClientRequest and the native request does not work actually. Or at least can't be done at the connector level which is at a lower level and has no visibility of ClientRequest. That means we have to do it at the level of the ClientRequest and if we do it there we might as well expose it per request.
Long story short, ClientRequest will accept Consumer<ClientHttpRequest> and the same will also be exposed in WebClient per request. In turn ClientHttpRequest will have a new getNativeRequest() method so that from the consumer you can access the underlying request. This will be invoked after the request is populated but before the body is written. That should be quite convenient I think.
Comment From: philsttr
Ok, so I could do something like this ExchangeFilterFunction?
(request, next) -> next.exchange(ClientRequest.from(request)
.clientHttpRequest(clientHttpRequest -> clientHttpRequest.getNativeRequest().responseTimeout(...))
.build())
Comment From: rstoyanchev
Yes that's the idea.
Comment From: rstoyanchev
I've exposed this on WebClient so you could do:
WebClient.create()
.get().uri("/path")
.httpRequest(httpRequest -> {
HttpClientRequest clientRequest = httpRequest.getNativeRequest();
clientRequest.responseTimeout(Duration.ofSeconds(5));
})
.retrieve()
...
Comment From: philsttr
Thanks @rstoyanchev ! Looks great!
Not sure if this is planned for later, but the timeout section of the docs could use an update.
Comment From: rstoyanchev
Sure, I'll have a look at updating the section.