RFR: 8208693: HttpClient: Extend the request timeout's scope to cover the response body
Michael McMahon
michaelm at openjdk.org
Wed Oct 22 11:54:39 UTC 2025
On Mon, 6 Oct 2025 19:41:07 GMT, Volkan Yazici <vyazici at openjdk.org> wrote:
>> Currently `HttpRequest::timeout` only applies until the response headers are received. Extend its scope to also cover the consumption of the response body.
>>
>> ### Review guidelines
>>
>> 1. Read _"the fix"_ in `MultiExchange`
>> 2. Skim through the test server *handler* in `TimeoutResponseTestSupport`
>> 3. Review first `TimeoutResponseHeaderTest`, and then `TimeoutResponseBodyTest` (Mind the multiple `@test` blocks!)
>
> src/java.net.http/share/classes/jdk/internal/net/http/MultiExchange.java line 537:
>
>> 535: if (currentreq.timeout().isPresent()) {
>> 536: // Retried/Forwarded requests should reset the timer, if present
>> 537: cancelTimer();
>
> In `master` (i.e., without this change), retried (due to some failure) and forwarded (due to 3XX responses with a `Location` header) requests will reset the timer at each retry and forwarding, respectively. This PR preserves this behavior, while additionally extending the request timeout's scope to cover _the complete retrieval_ of the response body too. But I would like to open this to discussion:
>
> 1. Shall we preserve the existing behavior and reset the timer on retries/forwarding?
> 2. Shall we ensure an `HttpClient::send` never exceeds the configured timeout even when there are retries/forwarding?
> 4. Something else?
>
> Note that the 1<sup>st</sup> option (i.e., the current behavior) implies that, if I configure a client to take max. 30 seconds for a request, I can be unfortunate enough to learn at production that it can actually take up to `5*30=150` seconds, where 5 is the default retry limit. That said, this behavior matches with Apache HTTP Client's [setResponseTimeout](https://hc.apache.org/httpcomponents-client-5.5.x/current/httpclient5/apidocs/org/apache/hc/client5/http/config/RequestConfig.Builder.html#setResponseTimeout-org.apache.hc.core5.util.Timeout-).
It would be very unusual for a server to wait 29 seconds before redirecting. I think you can assume that redirects will come in a reasonably timely fashion. The idea behind the timeout is really to allow for potentially large response bodies being downloaded and that's only going to happen once. So, a more likely scenario would be 1 or 2 seconds per redirect and then however long it takes to download the resource. So, in my opinion, the current behavior is okay.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/27469#discussion_r2409632252
More information about the net-dev
mailing list