On Wed, 24 Sep 2025 13:24:35 GMT, Volkan Yazici <[email protected]> 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/Http3ExchangeImpl.java
line 1341:
> 1339: if (!responseReceived && resetError ==
> Http3Error.H3_REQUEST_REJECTED) {
> 1340: exchange.markUnprocessedByPeer();
> 1341: }
Without this, server handler's
`exchange.resetStream(Http3Error.H3_REQUEST_REJECTED.code())` results in client
to fail the request, instead of retrying it. I am not sure if
`H3_REQUEST_REJECTED` is the only error code we should guard against.
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-).
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/27469#discussion_r2392359322
PR Review Comment: https://git.openjdk.org/jdk/pull/27469#discussion_r2408186745
