RFR: 8368249: HttpClient: Translate exceptions thrown by sendAsync [v3]

[Jaikiran Pai]

On Wed, 22 Oct 2025 12:56:03 GMT, Daniel Fuchs <dfuchs at openjdk.org> wrote:

>> I think, not propagating the original `UncheckedIOException` would be a bug. Imagine this application code:
>> 
>> 
>> final Future<HttpResponse<String>> future = client.sendAsync(req, new BodyHandler<String>() {
>>     @Override
>>     public BodySubscriber<String> apply(ResponseInfo ri) {
>>         throw new UncheckedIOException("application specific message", new IOException("some other message"));
>>     }
>> });
>> final HttpResponse<String> resp;
>> try {
>>     resp = future.get();
>> } catch (ExecutionException e) {
>>     final Throwable cause = e.getCause();
>>     // cause must be UncheckedIOException and its message must be "application specific message"
>>     assert ...
>> }
>> 
>> I think the application should receive the `UncheckedIOException` as the top level cause of the `ExecutionException` and that `UncheckedIOException` must contain the original cause and the original message that was there when the application raised it. Not passing along that to the application, I think, would be wrong.
>
> Then we should wrap - but that could be strange. Especially in those cases where the application does:
> 
>     throw new UncheckedIOException(new IOException(msg));
> 
> 
> But yes - maybe you're right. Since send() does rewrap UncheckedIOException into IOException then maybe sendAsync should do so too. Utils.toIOException could be overriden with a method that takes a lambda to convert UncheckedIOException to IOException. Something like:
> 
> 
>      public static IOException toIOException(Throwable cause) {
> +        return toIOException(cause, UncheckedIOException::getCause);
> +    }
> +
> +    public static IOException toIOException(Throwable cause, Function<UncheckedIOException, IOException> uncheckedIOConverter) {
>          if (cause == null) return null;
>          if (cause instanceof CompletionException ce) {
>              cause = ce.getCause();
> @@ -522,7 +526,7 @@ public static IOException toIOException(Throwable cause) {
>          if (cause instanceof IOException io) {
>              return io;
>          } else if (cause instanceof UncheckedIOException uio) {
> -            return uio.getCause();
> +            return uncheckedIOConverter.apply(uio);
>          }
>          return new IOException(cause.getMessage(), cause);
>      }
> 
> 
> Then sendAsync could call `Utils.toIOException(throwable, IOException::new)`

> I think the application should receive the UncheckedIOException as the top level cause of the ExecutionException and that UncheckedIOException

Slight correction to that previous comment of mine. In my mind, for a moment I thought UncheckedIOException is a IOException and that's why I said it should be returned as a the instance from `ExecutionException.getCause()`. That's not the case and I think what should really happen is that we treat `UncheckedIOException` just like any other `RuntimeException` and wrap it into a `IOException`. I think we shouldn't be peeking into the `UncheckedIOException.getCause()` at all when constructing that top level `IOException`. That way we will correctly pass along the original exception that was raised by the application code. 

Very specifically, I think the `Utils.toIOException()` should look like:


public static IOException toIOException(Throwable cause) {
    if (cause == null) return null;
    if (cause instanceof CompletionException ce) {
        cause = ce.getCause();
    } else if (cause instanceof ExecutionException ee) {
        cause = ee.getCause();
    }
    if (cause instanceof IOException io) {
        return io;
    }
    return new IOException(cause.getMessage(), cause);
}

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

PR Review Comment: https://git.openjdk.org/jdk/pull/27787#discussion_r2454672474