[GitHub] [calcite] vlsi commented on a change in pull request #2140: [CALCITE-4199] Add Util.throwAsRuntime and Util.causeOrSelf to simplify exception re-throwing

[GitBox <gi...@apache.org>]

vlsi commented on a change in pull request #2140:
URL: https://github.com/apache/calcite/pull/2140#discussion_r483926161



##########
File path: core/src/main/java/org/apache/calcite/util/Util.java
##########
@@ -911,6 +912,43 @@ public static void throwIfUnchecked(Throwable throwable) {
     }
   }
 
+  /**
+   * This method rethrows input throwable as is (if its unchecked) or
+   * wraps it with {@link RuntimeException} and throws.
+   * <p>The typical usage would be {@code throw throwAsRuntime(...)}, where {@code throw} statement
+   * is needed so Java compiler knows the execution stops at that line.</p>
+   *
+   * @param throwable input throwable
+   * @return the method never returns, it always throws an unchecked exception
+   */
+  @API(since = "1.26", status = API.Status.EXPERIMENTAL)
+  public static RuntimeException throwAsRuntime(Throwable throwable) {
+    throwIfUnchecked(throwable);
+    throw new RuntimeException(throwable);
+  }
+
+  /**
+   * This method rethrows input throwable as is (if its unchecked) with an extra message or
+   * wraps it with {@link RuntimeException} and throws.
+   * <p>The typical usage would be {@code throw throwAsRuntime(...)}, where {@code throw} statement
+   * is needed so Java compiler knows the execution stops at that line.</p>
+   *
+   * @param throwable input throwable
+   * @return the method never returns, it always throws an unchecked exception
+   */
+  @API(since = "1.26", status = API.Status.EXPERIMENTAL)
+  public static RuntimeException throwAsRuntime(String message, Throwable throwable) {
+    if (throwable instanceof RuntimeException) {
+      throwable.addSuppressed(new Throwable(message));
+      throw (RuntimeException) throwable;
+    }
+    if (throwable instanceof Error) {

Review comment:
       This block is a part of `throwIfUnchecked`, however, here I need to call `addSuppressed` before the throwing.
   
   An alternative implementation could be:
   
   ```java
   try {
     throwIfUnchecked(throwable);
   } catch (Throwable t) {
     // Ok, it is unchecked, so addSuppressed and rethrow
     t.addSuppressed(new Throwable(message));
     throwIfUnchecked(throwable);
   }
   ```
   
   Even though it looks like "code reuse", I don't like it




----------------------------------------------------------------
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:
users@infra.apache.org