Stop catching RuntimeException. This made the errors in issue #1234 more confusing. And we should never catch RuntimeException unless we're about to crash.

--
MOS_MIGRATED_REVID=122031190

3 files changed, 10 insertions, 13 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/syntax/ASTNode.java b/src/main/java/com/google/devtools/build/lib/syntax/ASTNode.java
index f1d10bfcb9..84d859fd0c 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/ASTNode.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/ASTNode.java
@@ -35,10 +35,8 @@ public abstract class ASTNode implements Serializable {
     return false;
   }
 
-  /**
-   * Returns an exception which should be thrown instead of the original one.
-   */
-  protected final EvalException handleException(Exception original) {
+  /** Returns an exception which should be thrown instead of the original one. */
+  protected final EvalException maybeTransformException(EvalException original) {
     // If there is already a non-empty stack trace, we only add this node iff it describes a
     // new scope (e.g. FuncallExpression).
     if (original instanceof EvalExceptionWithStackTrace) {
@@ -49,12 +47,11 @@ public abstract class ASTNode implements Serializable {
       return real;
     }
 
-    // Returns the original exception if it cannot be attached to a stack trace.
-    if (original instanceof EvalException && !((EvalException) original).canBeAddedToStackTrace()) {
-      return (EvalException) original;
+    if (original.canBeAddedToStackTrace()) {
+      return new EvalExceptionWithStackTrace(original, this);
+    } else {
+      return original;
     }
-
-    return new EvalExceptionWithStackTrace(original, this);
   }
 
   @VisibleForTesting  // productionVisibility = Visibility.PACKAGE_PRIVATE
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Expression.java b/src/main/java/com/google/devtools/build/lib/syntax/Expression.java
index d6090255d1..295db4a9ea 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Expression.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Expression.java
@@ -46,8 +46,8 @@ public abstract class Expression extends ASTNode {
   final Object eval(Environment env) throws EvalException, InterruptedException {
     try {
       return doEval(env);
-    } catch (EvalException | RuntimeException ex) {
-      throw handleException(ex);
+    } catch (EvalException ex) {
+      throw maybeTransformException(ex);
     }
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Statement.java b/src/main/java/com/google/devtools/build/lib/syntax/Statement.java
index 93f2a3a6ef..66f2256402 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Statement.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Statement.java
@@ -35,8 +35,8 @@ public abstract class Statement extends ASTNode {
   final void exec(Environment env) throws EvalException, InterruptedException   {
     try {
       doExec(env);
-    } catch (EvalException | RuntimeException ex)  {
-      throw handleException(ex);
+    } catch (EvalException ex) {
+      throw maybeTransformException(ex);
     }
   }