On Sat, 2 Mar 2024 01:40:39 GMT, Serguei Spitsyn <sspit...@openjdk.org> wrote:
>>> Q2: Can we use Files.deleteIfExists(path) in the finally block of the
>>> run() method instead?
>>
>> I don't think that changes anything. If run() throws an exception and then
>> Files.deleteIfExists() throws an exception from the finally block, the
>> original exception is lost.
>>
>>> One more approach is to wrap run() call into the runHelper() method:
>>
>> Same issue with potentially losing the original exception.
>
> Two variants:
>
> public void runHelper(CommandExecutor executor, String cmd, Path path) {
> try {
> run(executor, cmd, path);
> } catch (Throwable t) {
> t.printStackTrace(System.out);
> throw t;
> } finally {
> Files.deleteIfExists(path);
> }
> }
>
> or
>
> public void runHelper(CommandExecutor executor, String cmd, Path path) {
> try {
> run(executor, cmd, path);
> } finally {
> try {
> Files.deleteIfExists(path);
> } catch (Throwable t) {
> <print message about exception thrown from deleteIfExists>
> t.printStackTrace(System.out);
> }
> }
> }
The one issue with the first variant is that when run() throws an exception, it
will be printed twice, assuming deleteIfExists() does not throw an exception.
Basically you either get the exception printed twice, or you get the exception
printed once plus whatever exception deleteIfExists() throws also printed
(which is ok).
I think your 2nd example will work, but it's kind of confusing because if you
got into the finally block via an exception, you then might throw another
exception, which is handled in the finally block, and then (implicitly) the
original exception is rethrown as you exit the finally block. However, if there
was no exception from run() but deleteIfExists() throws an exception, the
exception is printed, but does not cause the test to fail. I don't think we
want that.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/17992#discussion_r1509883828
