Reactor-netty: HTTP client request timeout

[philsttr]

I'd like the http client to allow specifying a 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.

This is for use in WebFlux's WebClient, originally requested in https://github.com/spring-projects/spring-framework/issues/25115

Motivation

To be able to specify an HTTP-level request timeout, that does not end up closing idle connections or take into account the time it takes to establish connections (see Considered Alternatives below)

Desired solution

The ability to specify:

• the default request timeout for an http client.
• a specific timeout for a specific request (i.e. override the default)

I'd like to easily be able to use these features in WebFlux's WebClient. This might or might not require changes to WebClient, but I'd like it to be considered as part of a "complete" solution.

Considered alternatives

The 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.

[violetagg]

@philsttr I'm looking at this and even as of today you can use ReadTimeoutHandler. Depending on when you add this handler, you can achieve different behaviour.
The behaviour that you described here as a desired behaviour can be achieved with adding this handler with doAfterRequest event. Check this PR #1216 that add a test in order to demonstrate it.

[philsttr]

Thanks for pointing me in that direction @violetagg !

Indeed, your solution will handle the default request timeout for an HttpClient. And this works with WebFlux's WebClient, since I can pass a custom HttpClient to a WebClient's ReactorClientHttpConnector.

However, I didn't see an simple way to have request-specific timeouts (e.g. override the default) via WebClient (since the fact that reactor-netty is being used is hidden after the WebClient is constructed). Perhaps that could be accomplished with a custom handler that could inspect some metadata in the request or context?

Would you consider making request timeouts (default and request-specific) a first class feature of HttpClient? This seems like a common use case, and is a common feature of other HTTP clients.

[violetagg]

@philsttr @simonbasle PTAL #1216
@rstoyanchev May be an API for the request setting will be needed on WebClient level.

[violetagg]

Thank you all for the review

[smaldini]

I would rather call it a responseTimeout - the java client has it on the http request builder but its documentation mentions it is a response timeout. Request timeout in our http client might be open to different reading, write timeout or read timeout IMO.
Good report @philsttr tho, we are also starting work around webclient here and we came across the same issue.

[violetagg]

@philsttr @simonbasle @rstoyanchev I agree with @smaldini that responseTimeout is more accurate name. Wdyt?

We still do not have released version with this API so we can change it.

[violetagg]

I create this PR #1246

[simplynaveen20]

Thank you @violetagg for the PR to fix this. Do we know when is 0.9.11 release schedule which have the responseTimeout api, as it will be better alternative then using ReadTimeoutHandler / WriteTimeoutHandle , which have problem as mentioned in this issue _"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."_

[violetagg]

@simplynaveen20 the current plan for 0.9.11 release is to be available before 15.09

[violetagg]

@simplynaveen20 We are going to make a release next Monday (10.08)

[simplynaveen20]

@simplynaveen20 We are going to make a release next Monday (10.08)

Thank you very much for this. I will try it out and share feedback if any .