On Thu, 23 Oct 2025 10:34:48 GMT, Jaikiran Pai <[email protected]> wrote:
>> 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);
> }
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.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/27787#discussion_r2454766162
