On Mon, 14 Apr 2025 03:57:41 GMT, Kim Barrett <[email protected]> wrote:
>> test/lib/jdk/test/whitebox/WhiteBox.java line 609:
>>
>>> 607: throw e;
>>> 608: } catch (Throwable t) {
>>> 609: throw new RuntimeException(t);
>>
>> The only checked exceptions that are thrown by the try-body are
>> `InvocationTargetException` and
>> `IllegalAccessException` (both from `Method::invoke`). The way
>> `InvocationTargetException` is being
>> handled here seems fine. But I think it would be better to explicitly
>> handle `IllegalAccessException` and
>> transform it into a `RuntimeException`, and let everything else just
>> propagate out untouched, e.g. no
>> handlers for `RuntimeException` or `Throwable`.
>
> Thinking about this some more, if we're going to extract and (re)throw
> InterruptedException directly, then I prefer my earlier version:
> https://github.com/openjdk/jdk/pull/24527#pullrequestreview-2755384786
The latest update catches `IllegalAccessException`, and therefore doesn't need
to catch `Throwable`, which I agree is nicer.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24527#discussion_r2055011473
