On Thu, 23 Oct 2025 11:13:19 GMT, Daniel Fuchs <[email protected]> wrote:
>>> 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);
>> }
>
> Ok - let's do that. To my surprise there's only one place where toIOException
> is called. Other places that need an IO simply create a new IO. We could
> revisit those places and see if they should also call toIOException but
> that's another story best handled separately.
Pushed 5621d911944; it avoids peeling off `UncheckedIOException`. I inlined
this one-liner to `HttpClientImpl::translateSendAsyncExecFailure`, and stopped
using `Utils::toIOException`. I refrained from adapting `Utils::toIOException`
since it is used by another component (i.e., push manager) and I did not want
to introduce a behavioral change there.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/27787#discussion_r2460808920
