On Wed, 22 Oct 2025 11:35:04 GMT, Jaikiran Pai <[email protected]> wrote:
>> Volkan Yazici has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Revert `AbstractThrowingPublishers` changes
>
> src/java.net.http/share/classes/jdk/internal/net/http/HttpClientImpl.java
> line 981:
>
>> 979: // Exceptions are often thrown from asynchronous code,
>> making it
>> 980: // difficult to trace back to the user's original entry
>> point.
>> 981: // Wrap exceptions explicitly to preserve that context.
>
> Nit - Would it better to reword this as:
>
>> // Exceptions are often thrown from asynchronous code, and the
> // stacktrace may not always contain the application classes.
> // That makes it difficult to trace back to the application code which
> // invoked the HttpClient. Here we instantiate/recreate the
> // exceptions to capture the application's calling code in the
> // stacktrace of the thrown exception.
Changed as suggested in 85501942a64.
> test/jdk/java/net/httpclient/HttpClientSendAsyncExceptionTest.java line 72:
>
>> 70: var request =
>> HttpRequest.newBuilder(URI.create("https://example.com";)).GET().build();
>> 71: var responseBodyHandler =
>> HttpResponse.BodyHandlers.discarding();
>> 72: client.sendAsync(request, responseBodyHandler).get();
>
> Nit - it doesn't matter much in this code, but it's usually better to limit
> the code to the specific call that is expected to throw the expected
> exception type. That would mean moving out everything except the
> `Future.get()` call from the lambda.
>
> Same comment for the code in the other test methods too.
Very true. Implemented in 3a7f0aa181c.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/27787#discussion_r2460809351
PR Review Comment: https://git.openjdk.org/jdk/pull/27787#discussion_r2460809975
