RFR: 8208693: HttpClient: Extend the request timeout's scope to cover the response body [v5]
Daniel Fuchs
dfuchs at openjdk.org
Thu Oct 30 14:48:00 UTC 2025
On Wed, 29 Oct 2025 14:27:43 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 incrementally with one additional commit since the last revision:
>
> Replace wrapper's `preTerminationCallback` argument with a method to be extended
src/java.net.http/share/classes/jdk/internal/net/http/common/HttpBodySubscriberWrapper.java line 79:
> 77: * A callback to be invoked before termination, whether due to the
> 78: * completion or failure of the subscriber, or cancellation of the
> 79: * subscription.
I'd suggest to also say that when a subscription is cancelled, onTermination() is called before onCancel().
src/java.net.http/share/classes/jdk/internal/net/http/common/HttpBodySubscriberWrapper.java line 102:
> 100: } finally {
> 101: if (markCancelled()) {
> 102: onTermination();
I was wondering if it would be better to have the subclasses call the appropriate method from `onCancel()` rather than have both `cancel()` and `complete()` call `onTermination()`.
However - thinking about it again, I believe that calling onTermination() from cancel() is the right call.
FWIW - we should also make the private class `SubscriptionWrapper` final.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/27469#discussion_r2478386175
PR Review Comment: https://git.openjdk.org/jdk/pull/27469#discussion_r2478212917
More information about the net-dev
mailing list