kbendick commented on code in PR #5190:
URL: https://github.com/apache/iceberg/pull/5190#discussion_r914164348


##########
api/src/main/java/org/apache/iceberg/util/ExceptionUtil.java:
##########
@@ -116,12 +116,17 @@ public static <R, E1 extends Exception, E2 extends 
Exception, E3 extends Excepti
           LOG.warn("Suppressing failure in finally block", e);
           if (failure != null) {
             failure.addSuppressed(e);
+            tryThrowAs(failure, e1Class);
+            tryThrowAs(failure, e2Class);
+            tryThrowAs(failure, e3Class);
+            tryThrowAs(failure, RuntimeException.class);
+            throw new RuntimeException("Unknown throwable", failure);

Review Comment:
   This seems like it will fix what the warning is about, that throwing from a 
finally block will swallow whatever is returned or thrown inside of the try 
block.
   
   So this repeats lines 105-109, where we attempt to throw `failure` after 
running the catch block. As the warning message from error prone says that it 
will get swallowed if we throw inside of the finally, but we did try to throw 
it inside of the catch block.
   
   Though I think this also changes the semantics of the function as intended. 
So maybe we should just add the suppression?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to