Encode ViewCreation failures with FailureDetails
Bazel's analysis phase can fail with a ViewCreationFailedException,
encompassing a variety of loading, analysis, and post-analysis evaluation
problems. This change equips that exception with FailureDetails and
eliminates the final use of (non-success) DetailedExitCode without
FailureDetails.
Before this change, a ViewCreationFailedException (VCFE) always led to a
numerical exit code of 1. To ensure backwards compatibility, this change
uses the DetailedExitCode factory method which overrides the numerical
exit code metadata from failure_detail.proto. Most of the detailed failure
subcategories that can end up in a VCFE do have metadata specifying 1 as
their numerical exit code, but not all; e.g., post-analysis query
evaluation failures can cause a VCFE and, as query failures, have 2 or 7
for their metadata.
Ensuring the propagation of failure details from analysis to the VCFE
required the following work.
SkyframeBuildView translated analysis exceptions, designated via the
marker interface SaneAnalysisException, into VCFEs. Likewise, it
translated the loading-phase exceptions NoSuch{Package,Target}Exception
into VCFEs. By making SaneAnalysisException extend DetailedException,
that conversion now propagates any detailed failures described by those
three exception types.
SaneAnalysisException has six implementations. This change implements
DetailedException across them. Doing so with AspectCreationException and
ConfiguredValueCreationException involved moderately complex work. In
AspectFunction and ConfiguredTargetFunction, these two exception types
get instantiated when a nested set of "Cause"s is non-empty (and also in
several other more straightforward contexts).
"Cause" had already been equipped with a DetailedExitCode property, but
AnalysisFailedCause didn't support it (Cause's other implementations
did). This change implements the property for AnalysisFailedCause.
The nested set traversal in
ConfiguredTargetFunction.getPrioritizedDetailedExitCode does not involve
any deep traversals, though that's not obvious. If the nested set
builder ever gets another nested set added to it via addTransitive, then
it does so while handling a dependency's exception, which results in a
DependencyEvaluationException, whose handling avoids the call to
getPrioritizedDetailedExitCode.
This change does *not* detail analysis failures in maximum resolution. To
do so, a future change would need to enhance the error-event-sensitive
failure detection in AspectFunction and ConfiguredTargetFunction, using
e.g. the events-with-properties strategy used by
ErrorSensingEventHandler. It would also need to replace the non-detailed
AspectCreationException and ConfiguredValueCreationException
constructors.
RELNOTES: None.
PiperOrigin-RevId: 333827282
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BzlLoadFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/BzlLoadFunction.java
index c995d1f..cd64b5b 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/BzlLoadFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/BzlLoadFunction.java
@@ -41,7 +41,11 @@
 import com.google.devtools.build.lib.packages.StarlarkExportable;
 import com.google.devtools.build.lib.packages.WorkspaceFileValue;
 import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
+import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
+import com.google.devtools.build.lib.server.FailureDetails.StarlarkLoading;
+import com.google.devtools.build.lib.server.FailureDetails.StarlarkLoading.Code;
 import com.google.devtools.build.lib.skyframe.StarlarkBuiltinsFunction.BuiltinsFailedException;
+import com.google.devtools.build.lib.util.DetailedExitCode;
 import com.google.devtools.build.lib.util.Fingerprint;
 import com.google.devtools.build.lib.util.Pair;
 import com.google.devtools.build.lib.vfs.PathFragment;
@@ -438,7 +442,7 @@
       if (!loadStack.add(key)) {
         ImmutableList<BzlLoadValue.Key> cycle =
             CycleUtils.splitIntoPathAndChain(Predicates.equalTo(key), loadStack).second;
-        throw new BzlLoadFailedException("Starlark load cycle: " + cycle);
+        throw new BzlLoadFailedException("Starlark load cycle: " + cycle, Code.CYCLE);
       }
     }
 
@@ -514,7 +518,7 @@
                   BuildFileNotFoundException.class,
                   InconsistentFilesystemException.class);
     } catch (BuildFileNotFoundException | InconsistentFilesystemException e) {
-      throw BzlLoadFailedException.errorFindingPackage(label.toPathFragment(), e);
+      throw BzlLoadFailedException.errorFindingContainingPackage(label.toPathFragment(), e);
     }
     if (packageLookup == null) {
       return null;
@@ -560,7 +564,7 @@
     // Ensure the .bzl exists and passes static checks (parsing, resolving).
     // (A missing prelude file still returns a valid but empty BzlCompileValue.)
     if (!compileValue.lookupSuccessful()) {
-      throw new BzlLoadFailedException(compileValue.getError());
+      throw new BzlLoadFailedException(compileValue.getError(), Code.COMPILE_ERROR);
     }
     StarlarkFile file = compileValue.getAST();
     if (!file.ok()) {
@@ -1055,61 +1059,103 @@
 
   static final class BzlLoadFailedException extends Exception implements SaneAnalysisException {
     private final Transience transience;
+    private final DetailedExitCode detailedExitCode;
 
-    private BzlLoadFailedException(String errorMessage) {
+    private BzlLoadFailedException(
+        String errorMessage, DetailedExitCode detailedExitCode, Transience transience) {
       super(errorMessage);
-      this.transience = Transience.PERSISTENT;
+      this.transience = transience;
+      this.detailedExitCode = detailedExitCode;
     }
 
-    private BzlLoadFailedException(String errorMessage, Exception cause, Transience transience) {
+    private BzlLoadFailedException(String errorMessage, DetailedExitCode detailedExitCode) {
+      this(errorMessage, detailedExitCode, Transience.PERSISTENT);
+    }
+
+    private BzlLoadFailedException(
+        String errorMessage,
+        DetailedExitCode detailedExitCode,
+        Exception cause,
+        Transience transience) {
       super(errorMessage, cause);
       this.transience = transience;
+      this.detailedExitCode = detailedExitCode;
+    }
+
+    private BzlLoadFailedException(String errorMessage, Code code) {
+      this(errorMessage, createDetailedExitCode(errorMessage, code), Transience.PERSISTENT);
+    }
+
+    private BzlLoadFailedException(
+        String errorMessage, Code code, Exception cause, Transience transience) {
+      this(errorMessage, createDetailedExitCode(errorMessage, code), cause, transience);
     }
 
     Transience getTransience() {
       return transience;
     }
 
+    @Override
+    public DetailedExitCode getDetailedExitCode() {
+      return detailedExitCode;
+    }
+
+    private static DetailedExitCode createDetailedExitCode(String message, Code code) {
+      return DetailedExitCode.of(
+          FailureDetail.newBuilder()
+              .setMessage(message)
+              .setStarlarkLoading(StarlarkLoading.newBuilder().setCode(code))
+              .build());
+    }
+
     // TODO(bazel-team): This exception should hold a Location of the requesting file's load
     // statement, and code that catches it should use the location in the Event they create.
     static BzlLoadFailedException whileLoadingDep(
         String requestingFile, BzlLoadFailedException cause) {
       // Don't chain exception cause, just incorporate the message with a prefix.
-      return new BzlLoadFailedException("in " + requestingFile + ": " + cause.getMessage());
+      return new BzlLoadFailedException(
+          "in " + requestingFile + ": " + cause.getMessage(), cause.getDetailedExitCode());
     }
 
     static BzlLoadFailedException errors(PathFragment file) {
-      return new BzlLoadFailedException(String.format("Extension file '%s' has errors", file));
+      return new BzlLoadFailedException(
+          String.format("Extension file '%s' has errors", file), Code.EVAL_ERROR);
     }
 
-    static BzlLoadFailedException errorFindingPackage(PathFragment file, Exception cause) {
-      return new BzlLoadFailedException(
+    static BzlLoadFailedException errorFindingContainingPackage(
+        PathFragment file, Exception cause) {
+      String errorMessage =
           String.format(
-              "Encountered error while reading extension file '%s': %s", file, cause.getMessage()),
-          cause,
-          Transience.PERSISTENT);
+              "Encountered error while reading extension file '%s': %s", file, cause.getMessage());
+      DetailedExitCode detailedExitCode =
+          cause instanceof DetailedException
+              ? ((DetailedException) cause).getDetailedExitCode()
+              : createDetailedExitCode(errorMessage, Code.CONTAINING_PACKAGE_NOT_FOUND);
+      return new BzlLoadFailedException(
+          errorMessage, detailedExitCode, cause, Transience.PERSISTENT);
     }
 
     static BzlLoadFailedException errorReadingBzl(
         PathFragment file, BzlCompileFunction.FailedIOException cause) {
-      return new BzlLoadFailedException(
+      String errorMessage =
           String.format(
-              "Encountered error while reading extension file '%s': %s", file, cause.getMessage()),
-          cause,
-          cause.getTransience());
+              "Encountered error while reading extension file '%s': %s", file, cause.getMessage());
+      return new BzlLoadFailedException(errorMessage, Code.IO_ERROR, cause, cause.getTransience());
     }
 
     static BzlLoadFailedException noBuildFile(Label file, @Nullable String reason) {
       if (reason != null) {
         return new BzlLoadFailedException(
-            String.format("Unable to find package for %s: %s.", file, reason));
+            String.format("Unable to find package for %s: %s.", file, reason),
+            Code.PACKAGE_NOT_FOUND);
       }
       return new BzlLoadFailedException(
           String.format(
               "Every .bzl file must have a corresponding package, but '%s' does not have one."
                   + " Please create a BUILD file in the same or any parent directory. Note that"
                   + " this BUILD file does not need to do anything except exist.",
-              file));
+              file),
+          Code.PACKAGE_NOT_FOUND);
     }
 
     static BzlLoadFailedException labelCrossesPackageBoundary(
@@ -1122,11 +1168,13 @@
               // message for the user.
               containingPackageLookupValue.getContainingPackageRoot(),
               label,
-              containingPackageLookupValue));
+              containingPackageLookupValue),
+          Code.LABEL_CROSSES_PACKAGE_BOUNDARY);
     }
 
     static BzlLoadFailedException starlarkErrors(PathFragment file) {
-      return new BzlLoadFailedException(String.format("Extension '%s' has errors", file));
+      return new BzlLoadFailedException(
+          String.format("Extension '%s' has errors", file), Code.PARSE_ERROR);
     }
 
     static BzlLoadFailedException builtinsFailed(Label file, BuiltinsFailedException cause) {
@@ -1134,6 +1182,7 @@
           String.format(
               "Internal error while loading Starlark builtins for %s: %s",
               file, cause.getMessage()),
+          Code.BUILTINS_ERROR,
           cause,
           cause.getTransience());
     }