On Wed, 15 Oct 2025 13:52:39 GMT, Daniel Fuchs <[email protected]> wrote:
>> Volkan Yazici has updated the pull request with a new target base due to a >> merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains five additional >> commits since the last revision: >> >> - Merge remote-tracking branch 'upstream/master' into sendAsyncExWrap >> - Use `Utils::getCompletionCause` >> >> Co-authored-by: Daniel Fuchs <[email protected]> >> - Document exception wrapping in `HttpClientImpl::send` >> - Fix test failures >> - Ensure `sendAsync` wraps execution failures in `IOException` > > src/java.net.http/share/classes/jdk/internal/net/http/HttpClientImpl.java > line 1112: > >> 1110: res = translateSendAsyncExecFailure(res); >> 1111: >> 1112: if (exchangeExecutor != null) { > > Suggestion: > > if (exchangeExecutor != null) { > // we're called by sendAsync() - make sure we translate > exceptions > res = translateSendAsyncExecFailure(res); Implemented in ec28a849112. This rendered the need to touch following tests unnecessary: - `AbstractThrowingPublishers` - `InvalidInputStreamSubscriptionRequest` - `http3/H3QuicTLSConnection` - `http3/StopSendingTest` Hence, reverted these changes. > src/java.net.http/share/classes/jdk/internal/net/http/HttpClientImpl.java > line 1145: > >> 1143: // Except `Error`s, wrap failures inside an >> `IOException`. >> 1144: // This is required to comply with the >> specification of `HttpClient::sendAsync`. >> 1145: var translatedException = unwrappedException >> instanceof Error > > Suggestion: > > var translatedException = unwrappedException > instanceof Error > || unwrappedException > instanceof CancelledException Implemented in ec28a849112. > test/jdk/java/net/httpclient/AbstractThrowingSubscribers.java line 683: > >> 681: public boolean test(Throwable throwable) { >> 682: // `UncheckedIOException` is peeled off by >> `HttpClientImpl::translateSendAsyncExecFailure` >> 683: return throwable instanceof CustomIOException; > > We want to preserve the UncheckedIOException here. We don't want to peel it > off. I'm not able to follow. `UncheckedIOE` is peeled off by `Utils::toIOException` in `translateSendAsyncExecFailure()`. Mind elaborating on what are you suggesting? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/27787#discussion_r2433723097 PR Review Comment: https://git.openjdk.org/jdk/pull/27787#discussion_r2433723572 PR Review Comment: https://git.openjdk.org/jdk/pull/27787#discussion_r2433729561
