Set a bit in ActionExecutedEvent indicating if a failure occurred before the action actually executed.

PiperOrigin-RevId: 164917959

2 files changed, 54 insertions, 14 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutedEvent.java b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutedEvent.java
index f5a11a8ff5..1ce6bce6d0 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutedEvent.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutedEvent.java
@@ -14,6 +14,7 @@
 
 package com.google.devtools.build.lib.actions;
 
+import com.google.common.base.MoreObjects;
 import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.buildeventstream.BuildEvent;
 import com.google.devtools.build.lib.buildeventstream.BuildEventConverters;
@@ -23,6 +24,7 @@ import com.google.devtools.build.lib.buildeventstream.BuildEventWithConfiguratio
 import com.google.devtools.build.lib.buildeventstream.GenericBuildEvent;
 import com.google.devtools.build.lib.buildeventstream.NullConfiguration;
 import com.google.devtools.build.lib.buildeventstream.PathConverter;
+import com.google.devtools.build.lib.util.Preconditions;
 import com.google.devtools.build.lib.vfs.Path;
 import java.util.Collection;
 
@@ -35,13 +37,21 @@ public class ActionExecutedEvent implements BuildEventWithConfiguration {
   private final ActionExecutionException exception;
   private final Path stdout;
   private final Path stderr;
+  private final ErrorTiming timing;
 
-  public ActionExecutedEvent(Action action,
-      ActionExecutionException exception, Path stdout, Path stderr) {
+  public ActionExecutedEvent(
+      Action action,
+      ActionExecutionException exception,
+      Path stdout,
+      Path stderr,
+      ErrorTiming timing) {
     this.action = action;
     this.exception = exception;
     this.stdout = stdout;
     this.stderr = stderr;
+    this.timing = timing;
+    Preconditions.checkState(
+        (this.exception == null) == (this.timing == ErrorTiming.NO_ERROR), this);
   }
 
   public Action getAction() {
@@ -53,6 +63,10 @@ public class ActionExecutedEvent implements BuildEventWithConfiguration {
     return exception;
   }
 
+  public ErrorTiming errorTiming() {
+    return timing;
+  }
+
   public String getStdout() {
     if (stdout == null) {
       return null;
@@ -137,4 +151,22 @@ public class ActionExecutedEvent implements BuildEventWithConfiguration {
     }
     return GenericBuildEvent.protoChaining(this).setAction(actionBuilder.build()).build();
   }
+
+  @Override
+  public String toString() {
+    return MoreObjects.toStringHelper(this)
+        .add("exception", exception)
+        .add("timing", timing)
+        .add("stdout", stdout)
+        .add("stderr", stderr)
+        .add("action", action)
+        .toString();
+  }
+
+  /** When an error occurred that aborted action execution, if any. */
+  public enum ErrorTiming {
+    NO_ERROR,
+    BEFORE_EXECUTION,
+    AFTER_EXECUTION
+  }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
index d628a88ad7..be0ba39ae3 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
@@ -28,6 +28,7 @@ import com.google.devtools.build.lib.actions.ActionCacheChecker.Token;
 import com.google.devtools.build.lib.actions.ActionCompletionEvent;
 import com.google.devtools.build.lib.actions.ActionContext;
 import com.google.devtools.build.lib.actions.ActionExecutedEvent;
+import com.google.devtools.build.lib.actions.ActionExecutedEvent.ErrorTiming;
 import com.google.devtools.build.lib.actions.ActionExecutionContext;
 import com.google.devtools.build.lib.actions.ActionExecutionContextFactory;
 import com.google.devtools.build.lib.actions.ActionExecutionException;
@@ -581,7 +582,8 @@ public final class SkyframeActionExecutor implements ActionExecutionContextFacto
     try {
       return action.discoverInputs(actionExecutionContext);
     } catch (ActionExecutionException e) {
-      throw processAndThrow(e, action, actionExecutionContext.getFileOutErr());
+      throw processAndThrow(
+          e, action, actionExecutionContext.getFileOutErr(), ErrorTiming.BEFORE_EXECUTION);
     }
   }
 
@@ -796,10 +798,10 @@ public final class SkyframeActionExecutor implements ActionExecutionContextFacto
     }
   }
 
-  ActionExecutionException processAndThrow(
-      ActionExecutionException e, Action action, FileOutErr outErrBuffer)
+  private ActionExecutionException processAndThrow(
+      ActionExecutionException e, Action action, FileOutErr outErrBuffer, ErrorTiming errorTiming)
       throws ActionExecutionException {
-    reportActionExecution(action, e, outErrBuffer);
+    reportActionExecution(action, e, outErrBuffer, errorTiming);
     boolean reported = reportErrorIfNotAbortingMode(e, outErrBuffer);
 
     ActionExecutionException toThrow = e;
@@ -858,7 +860,7 @@ public final class SkyframeActionExecutor implements ActionExecutionContextFacto
       }
       // Defer reporting action success until outputs are checked
     } catch (ActionExecutionException e) {
-      processAndThrow(e, action, outErrBuffer);
+      throw processAndThrow(e, action, outErrBuffer, ErrorTiming.AFTER_EXECUTION);
     } finally {
       profiler.completeTask(ProfilerTask.ACTION_EXECUTE);
     }
@@ -889,15 +891,18 @@ public final class SkyframeActionExecutor implements ActionExecutionContextFacto
         }
       }
 
-      reportActionExecution(action, null, fileOutErr);
+      reportActionExecution(action, null, fileOutErr, ErrorTiming.NO_ERROR);
     } catch (ActionExecutionException actionException) {
       // Success in execution but failure in completion.
-      reportActionExecution(action, actionException, fileOutErr);
+      reportActionExecution(action, actionException, fileOutErr, ErrorTiming.AFTER_EXECUTION);
       throw actionException;
     } catch (IllegalStateException exception) {
       // More serious internal error, but failure still reported.
-      reportActionExecution(action,
-          new ActionExecutionException(exception, action, true), fileOutErr);
+      reportActionExecution(
+          action,
+          new ActionExecutionException(exception, action, true),
+          fileOutErr,
+          ErrorTiming.AFTER_EXECUTION);
       throw exception;
     }
   }
@@ -1074,8 +1079,11 @@ public final class SkyframeActionExecutor implements ActionExecutionContextFacto
     }
   }
 
-  private void reportActionExecution(Action action,
-      ActionExecutionException exception, FileOutErr outErr) {
+  private void reportActionExecution(
+      Action action,
+      ActionExecutionException exception,
+      FileOutErr outErr,
+      ErrorTiming errorTiming) {
     Path stdout = null;
     Path stderr = null;
 
@@ -1085,7 +1093,7 @@ public final class SkyframeActionExecutor implements ActionExecutionContextFacto
     if (outErr.hasRecordedStderr()) {
       stderr = outErr.getErrorPath();
     }
-    postEvent(new ActionExecutedEvent(action, exception, stdout, stderr));
+    postEvent(new ActionExecutedEvent(action, exception, stdout, stderr, errorTiming));
   }
 
   /**