Prefer catastrophic action execution exceptions to non-catastrophic ones. This is both probably what the user most cares about, and also restores a Skyframe invariant, that as a catastrophic exception bubbles up, it stays catastrophic.

There is actually no hard Skyframe requirement for that, so we could consider relaxing it, but I'm willing to keep it for now.

PiperOrigin-RevId: 174759976

src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java
index 46238c4..25f74ca 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java
@@ -586,7 +586,6 @@
       throws ActionExecutionException {
     int missingCount = 0;
     int actionFailures = 0;
-    boolean catastrophe = false;
     // Only populate input data if we have the input values, otherwise they'll just go unused.
     // We still want to loop through the inputs to collect missing deps errors. During the
     // evaluator "error bubbling", we may get one last chance at reporting errors even though
@@ -637,10 +636,11 @@
         }
       } catch (ActionExecutionException e) {
         actionFailures++;
-        if (firstActionExecutionException == null) {
+        // Prefer a catastrophic exception as the one we propagate.
+        if (firstActionExecutionException == null
+            || !firstActionExecutionException.isCatastrophe() && e.isCatastrophe()) {
           firstActionExecutionException = e;
         }
-        catastrophe = catastrophe || e.isCatastrophe();
         rootCauses.addTransitive(e.getRootCauses());
       }
     }
@@ -651,8 +651,12 @@
         // having to copy the root causes to the upwards transitive closure.
         throw firstActionExecutionException;
       }
-      throw new ActionExecutionException(firstActionExecutionException.getMessage(),
-          firstActionExecutionException.getCause(), action, rootCauses.build(), catastrophe,
+      throw new ActionExecutionException(
+          firstActionExecutionException.getMessage(),
+          firstActionExecutionException.getCause(),
+          action,
+          rootCauses.build(),
+          firstActionExecutionException.isCatastrophe(),
           firstActionExecutionException.getExitCode());
     }
 

src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java
index 8744f1a..6853398 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java
@@ -267,7 +267,9 @@
         }
       } catch (ActionExecutionException e) {
         rootCausesBuilder.addTransitive(e.getRootCauses());
-        if (firstActionExecutionException == null) {
+        // Prefer a catastrophic exception as the one we propagate.
+        if (firstActionExecutionException == null
+            || !firstActionExecutionException.isCatastrophe() && e.isCatastrophe()) {
           firstActionExecutionException = e;
         }
       }