RFR: 8208693: HttpClient: Extend the request timeout's scope to cover the response body [v8]

Daniel Fuchs dfuchs at openjdk.org
Tue Nov 4 18:01:36 UTC 2025


On Mon, 3 Nov 2025 14:36:26 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!)
>
> Volkan Yazici has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 31 commits:
> 
>  - Shorten `TimeoutResponseTestSupport` using imports
>  - Make `MultiExchange::cancelTimer` package-private
>  - Merge remote-tracking branch 'upstream/master' into respBodyTime
>  - Simplify Javadoc
>  - Merge remote-tracking branch 'upstream/master' into respBodyTime
>  - Improve `HttpBodySubscriberWrapper::onTermination` JavaDoc
>  - Mark `SubscriptionWrapper` as `final`
>  - Replace wrapper's `preTerminationCallback` argument with a method to be extended
>  - Fix `HttpClient` doc typo
>  - Invoke `preTerminationCallback` at `cancel()` too
>  - ... and 21 more: https://git.openjdk.org/jdk/compare/ef464d69...55396846

src/java.net.http/share/classes/java/net/http/WebSocket.java line 151:

> 149:          * moment a connection is requested until it is established. The elapsed
> 150:          * time includes any SSL/TLS handshake.
> 151:          *

This change should probably be  ammended. 
The connectionTimeout for websocket is set as the request timeout for the HTTP upgrade request to websocket.
So it includes both the handshake and the 101 response.
Now I wonder if we are missing something that will cancel the timer when handing the connection off to WebSocket. It would be good to double check. We had no failing tests so maybe this is properly handled.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/27469#discussion_r2491576541


More information about the net-dev mailing list