On Wed, 22 Oct 2025 12:56:03 GMT, Daniel Fuchs <[email protected]> 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
