Allow non-ActionExecutionException DetailedExceptions to populate the detailed exit code on a failed keep-going execution phase, and enrich the message when possible to try to pin down any remaining issues.
Not changing the name of the code/deprecating because we don't yet know what the issue is: perhaps it's random other DetailedExceptions, or exceptions that are not properly bubbling up, etc.
PiperOrigin-RevId: 334455888
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionException.java b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionException.java
index 8fb293e..0e32500 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionException.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionException.java
@@ -21,7 +21,7 @@
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.collect.nestedset.Order;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
-import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
+import com.google.devtools.build.lib.skyframe.DetailedException;
import com.google.devtools.build.lib.util.DetailedExitCode;
import com.google.devtools.build.lib.util.ExitCode;
import net.starlark.java.syntax.Location;
@@ -31,7 +31,7 @@
* Typically these are re-raised ExecException throwables.
*/
@ThreadSafe
-public class ActionExecutionException extends Exception {
+public class ActionExecutionException extends Exception implements DetailedException {
private final ActionAnalysisMetadata action;
private final NestedSet<Cause> rootCauses;
@@ -150,10 +150,7 @@
return detailedExitCode.getExitCode();
}
- /**
- * Returns the pair of {@link ExitCode} and optional {@link FailureDetail} to return from this
- * Bazel invocation because of this action execution failure.
- */
+ @Override
public DetailedExitCode getDetailedExitCode() {
return detailedExitCode;
}
diff --git a/src/main/java/com/google/devtools/build/lib/actions/BUILD b/src/main/java/com/google/devtools/build/lib/actions/BUILD
index a645223..4470dea 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/actions/BUILD
@@ -80,6 +80,7 @@
"//src/main/java/com/google/devtools/build/lib/profiler",
"//src/main/java/com/google/devtools/build/lib/profiler:google-auto-profiler-utils",
"//src/main/java/com/google/devtools/build/lib/shell",
+ "//src/main/java/com/google/devtools/build/lib/skyframe:detailed_exceptions",
"//src/main/java/com/google/devtools/build/lib/skyframe:sane_analysis_exception",
"//src/main/java/com/google/devtools/build/lib/skyframe:sky_functions",
"//src/main/java/com/google/devtools/build/lib/skyframe:skyframe_aware_action",
diff --git a/src/main/java/com/google/devtools/build/lib/actions/BuildFailedException.java b/src/main/java/com/google/devtools/build/lib/actions/BuildFailedException.java
index effbe9a..d31e005 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/BuildFailedException.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/BuildFailedException.java
@@ -21,7 +21,7 @@
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.collect.nestedset.Order;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
-import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
+import com.google.devtools.build.lib.skyframe.DetailedException;
import com.google.devtools.build.lib.util.DetailedExitCode;
import com.google.devtools.build.lib.util.ExitCode;
@@ -39,7 +39,7 @@
* propagated by specifying the exit code to the constructor using a {@link DetailedExitCode}.
*/
@ThreadSafe
-public class BuildFailedException extends Exception {
+public class BuildFailedException extends Exception implements DetailedException {
private final boolean catastrophic;
private final NestedSet<Cause> rootCauses;
private final boolean errorAlreadyShown;
@@ -79,10 +79,7 @@
return errorAlreadyShown || getMessage() == null;
}
- /**
- * Returns the pair of {@link ExitCode} and optional {@link FailureDetail} to return from this
- * Bazel invocation.
- */
+ @Override
public DetailedExitCode getDetailedExitCode() {
return detailedExitCode;
}
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/SkyframeBuilder.java b/src/main/java/com/google/devtools/build/lib/buildtool/SkyframeBuilder.java
index c31c653..247ff7a 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/SkyframeBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/SkyframeBuilder.java
@@ -20,6 +20,7 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Range;
import com.google.common.collect.Sets;
+import com.google.common.flogger.GoogleLogger;
import com.google.devtools.build.lib.actions.ActionCacheChecker;
import com.google.devtools.build.lib.actions.ActionExecutionException;
import com.google.devtools.build.lib.actions.ActionExecutionStatusReporter;
@@ -50,6 +51,7 @@
import com.google.devtools.build.lib.skyframe.AspectValueKey.AspectKey;
import com.google.devtools.build.lib.skyframe.Builder;
import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey;
+import com.google.devtools.build.lib.skyframe.DetailedException;
import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
import com.google.devtools.build.lib.skyframe.TopDownActionCache;
import com.google.devtools.build.lib.util.AbruptExitException;
@@ -76,6 +78,7 @@
*/
@VisibleForTesting
public class SkyframeBuilder implements Builder {
+ private static final GoogleLogger logger = GoogleLogger.forEnclosingClass();
private final ResourceManager resourceManager;
private final SkyframeExecutor skyframeExecutor;
@@ -276,22 +279,42 @@
// 2. First non-infrastructure error with non-null exit code
// 3. If the build fails but no interpretable error is specified, BUILD_FAILURE.
DetailedExitCode detailedExitCode = null;
+ Throwable undetailedCause = null;
for (Map.Entry<SkyKey, ErrorInfo> error : result.errorMap().entrySet()) {
Throwable cause = error.getValue().getException();
- if (cause instanceof ActionExecutionException) {
+ if (cause instanceof DetailedException) {
// Update global exit code when current exit code is not null and global exit code has
// a lower 'reporting' priority.
detailedExitCode =
DetailedExitCodeComparator.chooseMoreImportantWithFirstIfTie(
- detailedExitCode, ((ActionExecutionException) cause).getDetailedExitCode());
+ detailedExitCode, ((DetailedException) cause).getDetailedExitCode());
+ if (!(cause instanceof ActionExecutionException)
+ && !(cause instanceof MissingInputFileException)) {
+ logger.atWarning().withCause(cause).log(
+ "Non-action-execution/missing-input exception for %s", error);
+ }
+ } else {
+ undetailedCause = cause;
}
}
-
- return detailedExitCode != null
- ? detailedExitCode
- : createDetailedExitCode(
- "keep_going execution failed without an action failure",
- Code.NON_ACTION_EXECUTION_FAILURE);
+ if (detailedExitCode != null) {
+ return detailedExitCode;
+ }
+ if (undetailedCause == null) {
+ logger.atWarning().log("No exceptions found despite error in %s", result);
+ return createDetailedExitCode(
+ "keep_going execution failed without an action failure",
+ Code.NON_ACTION_EXECUTION_FAILURE);
+ }
+ logger.atWarning().withCause(undetailedCause).log(
+ "No detailed exception found in %s", result);
+ return createDetailedExitCode(
+ "keep_going execution failed without an action failure: "
+ + undetailedCause.getMessage()
+ + " ("
+ + undetailedCause.getClass().getSimpleName()
+ + ")",
+ Code.NON_ACTION_EXECUTION_FAILURE);
}
ErrorInfo errorInfo = Preconditions.checkNotNull(result.getError(), result);
Exception exception = errorInfo.getException();
diff --git a/src/test/java/com/google/devtools/build/lib/buildtool/BUILD b/src/test/java/com/google/devtools/build/lib/buildtool/BUILD
index 8fb3804..f6b7d73 100644
--- a/src/test/java/com/google/devtools/build/lib/buildtool/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/buildtool/BUILD
@@ -362,6 +362,7 @@
"//src/main/java/com/google/devtools/build/lib/bazel:modules",
"//src/main/java/com/google/devtools/build/lib/causes",
"//src/main/java/com/google/devtools/build/lib/vfs",
+ "//src/main/protobuf:failure_details_java_proto",
"//src/test/java/com/google/devtools/build/lib/buildtool/util",
"//src/test/java/com/google/devtools/build/lib/packages:testutil",
"//src/test/java/com/google/devtools/build/lib/testutil",
diff --git a/src/test/java/com/google/devtools/build/lib/buildtool/MissingInputActionTest.java b/src/test/java/com/google/devtools/build/lib/buildtool/MissingInputActionTest.java
index bc5ea49..e9477f2 100644
--- a/src/test/java/com/google/devtools/build/lib/buildtool/MissingInputActionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/buildtool/MissingInputActionTest.java
@@ -23,6 +23,8 @@
import com.google.devtools.build.lib.causes.Cause;
import com.google.devtools.build.lib.packages.util.MockGenruleSupport;
import com.google.devtools.build.lib.runtime.BlazeModule;
+import com.google.devtools.build.lib.server.FailureDetails.Execution.Code;
+import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
import com.google.devtools.build.lib.testutil.Suite;
import com.google.devtools.build.lib.testutil.TestSpec;
import com.google.devtools.build.lib.vfs.Path;
@@ -113,8 +115,33 @@
}
}
+ @Test
+ public void testMissingTopLevelInput() throws Exception {
+ // Create a rule that exports a missing source file as a top-level artifact so that the missing
+ // file will be detected by the TargetCompletion function, not an ActionExecution function.
+ write(
+ "foo/missing.bzl",
+ "def _missing_impl(ctx):",
+ " return DefaultInfo(files = depset(ctx.files.srcs))",
+ "",
+ "missing = rule(",
+ " implementation = _missing_impl,",
+ " attrs = { 'srcs': attr.label_list(allow_files = True) }",
+ ")");
+ write(
+ "foo/BUILD",
+ "load('missing.bzl', 'missing')",
+ "missing(name = 'foo', srcs = ['missing.sh'])");
+ addOptions("--keep_going");
+ assertMissingInputOnBuild("//foo:foo", 0);
+ events.assertContainsError("foo/BUILD:2:8: //foo:foo: missing input file '//foo:missing.sh'");
+ events.assertContainsEventWithFrequency("missing input file", 1);
+ }
+
private void assertMissingInputOnBuild(String label, int numMissing) {
BuildFailedException e = assertThrows(BuildFailedException.class, () -> buildTarget(label));
+ FailureDetail failureDetail = e.getDetailedExitCode().getFailureDetail();
+ assertThat(failureDetail.getExecution().getCode()).isEqualTo(Code.SOURCE_INPUT_MISSING);
if (numMissing > 0) {
String expected = numMissing + " input file(s) do not exist";
assertThat(e).hasMessageThat().contains(expected);
