rdblue commented on a change in pull request #1909:
URL: https://github.com/apache/iceberg/pull/1909#discussion_r545312393
##########
File path: core/src/main/java/org/apache/iceberg/util/ExceptionUtil.java
##########
@@ -36,4 +40,96 @@ private ExceptionUtil() {
}
throw new RuntimeException(exception);
}
+
+ interface Block<R, E1 extends Exception, E2 extends Exception, E3 extends
Exception> {
+ R run() throws E1, E2, E3;
+ }
+
+ interface CatchBlock {
+ void run(Throwable failure) throws Exception;
+ }
+
+ interface FinallyBlock {
+ void run() throws Exception;
+ }
+
+ public static <R> R runSafely(
+ Block<R, RuntimeException, RuntimeException, RuntimeException> block,
+ CatchBlock catchBlock,
+ FinallyBlock finallyBlock) {
+ return runSafely(block, catchBlock, finallyBlock,
+ RuntimeException.class, RuntimeException.class,
RuntimeException.class);
+ }
+
+ public static <R, E1 extends Exception> R runSafely(
+ Block<R, E1, RuntimeException, RuntimeException> block,
+ CatchBlock catchBlock,
+ FinallyBlock finallyBlock,
+ Class<? extends E1> e1Class) throws E1 {
+ return runSafely(block, catchBlock, finallyBlock, e1Class,
RuntimeException.class, RuntimeException.class);
+ }
+
+ public static <R, E1 extends Exception, E2 extends Exception> R runSafely(
+ Block<R, E1, E2, RuntimeException> block,
+ CatchBlock catchBlock,
+ FinallyBlock finallyBlock,
+ Class<? extends E1> e1Class,
+ Class<? extends E2> e2Class) throws E1, E2 {
+ return runSafely(block, catchBlock, finallyBlock, e1Class, e2Class,
RuntimeException.class);
+ }
+
+ public static <R, E1 extends Exception, E2 extends Exception, E3 extends
Exception> R runSafely(
+ Block<R, E1, E2, E3> block,
+ CatchBlock catchBlock,
+ FinallyBlock finallyBlock,
+ Class<? extends E1> e1Class,
+ Class<? extends E2> e2Class,
+ Class<? extends E3> e3Class) throws E1, E2, E3 {
+ Exception failure = null;
+ try {
+ return block.run();
+
+ } catch (Exception t) {
+ failure = t;
+
+ if (catchBlock != null) {
+ try {
+ catchBlock.run(failure);
+ } catch (Exception e) {
+ LOG.warn("Suppressing failure in catch block", e);
+ failure.addSuppressed(e);
+ }
+ }
+
+ tryThrowAs(failure, e1Class);
+ tryThrowAs(failure, e2Class);
+ tryThrowAs(failure, e3Class);
+ tryThrowAs(failure, RuntimeException.class);
+ throw new RuntimeException("Unknown throwable", failure);
Review comment:
Originally, this block caught `Throwable`, but I changed it to
`Exception` thinking that we should not catch `Error` and run the `catch`
block. In that case, this line was just for safety.
But I've changed it back to `Throwable` after looking at this comment. I
thought more about it and this block _does_ need to run if `Error` is thrown
instead of `Exception`. If `Error` is thrown but not caught here, then the
finally block will run without the `failure` set and would throw any exception
from that block instead of suppressing it.
I think if we want to do no work when `Error` is thrown, then this should
specifically check for `Error`, but I think that it is correct to run
everything on `Error` because that's the superclass of things like
`AssertionError`. If an `assert` fails, we still want to run everything like
normal.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
