Remove remaining undetailed BuildFailedException constructor Some loading-phase-related failure modes have placeholder detailed codes to be improved when loading-phase failures are detailed. RELNOTES: None. PiperOrigin-RevId: 318366815
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 8969094..effbe9a 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
@@ -45,15 +45,6 @@ private final boolean errorAlreadyShown; private final DetailedExitCode detailedExitCode; - public BuildFailedException(String message) { - this( - message, - /*catastrophic=*/ false, - NestedSetBuilder.emptySet(Order.STABLE_ORDER), - /*errorAlreadyShown=*/ false, - DetailedExitCode.justExitCode(ExitCode.BUILD_FAILURE)); - } - public BuildFailedException(String message, DetailedExitCode detailedExitCode) { this( message,
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/AnalysisResult.java b/src/main/java/com/google/devtools/build/lib/analysis/AnalysisResult.java index ae03826..3c63b34 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/AnalysisResult.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/AnalysisResult.java
@@ -22,6 +22,7 @@ import com.google.devtools.build.lib.actions.ActionGraph; import com.google.devtools.build.lib.actions.PackageRoots; import com.google.devtools.build.lib.analysis.config.BuildConfigurationCollection; +import com.google.devtools.build.lib.server.FailureDetails.FailureDetail; import com.google.devtools.build.lib.skyframe.AspectValueKey.AspectKey; import java.util.Collection; import javax.annotation.Nullable; @@ -34,7 +35,7 @@ private final ImmutableSet<ConfiguredTarget> targetsToBuild; @Nullable private final ImmutableList<ConfiguredTarget> targetsToTest; private final ImmutableSet<ConfiguredTarget> targetsToSkip; - @Nullable private final String error; + @Nullable private final FailureDetail failureDetail; private final ActionGraph actionGraph; private final ArtifactsToOwnerLabels topLevelArtifactsToOwnerLabels; private final ImmutableSet<ConfiguredTarget> parallelTests; @@ -52,7 +53,7 @@ ImmutableMap<AspectKey, ConfiguredAspect> aspects, @Nullable ImmutableList<ConfiguredTarget> targetsToTest, ImmutableSet<ConfiguredTarget> targetsToSkip, - @Nullable String error, + @Nullable FailureDetail failureDetail, ActionGraph actionGraph, ArtifactsToOwnerLabels topLevelArtifactsToOwnerLabels, ImmutableSet<ConfiguredTarget> parallelTests, @@ -67,7 +68,7 @@ this.aspects = aspects; this.targetsToTest = targetsToTest; this.targetsToSkip = targetsToSkip; - this.error = error; + this.failureDetail = failureDetail; this.actionGraph = actionGraph; this.topLevelArtifactsToOwnerLabels = topLevelArtifactsToOwnerLabels; this.parallelTests = parallelTests; @@ -131,15 +132,14 @@ return parallelTests; } - /** - * Returns an error description (if any). - */ - @Nullable public String getError() { - return error; + /** Returns a {@link FailureDetail}, if any failures occurred. */ + @Nullable + public FailureDetail getFailureDetail() { + return failureDetail; } public boolean hasError() { - return error != null; + return failureDetail != null; } /** @@ -176,7 +176,7 @@ aspects, targetsToTest, targetsToSkip, - error, + failureDetail, actionGraph, topLevelArtifactsToOwnerLabels, Sets.union(parallelTests, exclusiveTests).immutableCopy(),
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BUILD b/src/main/java/com/google/devtools/build/lib/analysis/BUILD index b9598c2..afc5ee7 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/BUILD +++ b/src/main/java/com/google/devtools/build/lib/analysis/BUILD
@@ -620,6 +620,7 @@ "//src/main/java/com/google/devtools/build/lib/util", "//src/main/java/com/google/devtools/build/skyframe", "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects", + "//src/main/protobuf:failure_details_java_proto", "//third_party:flogger", "//third_party:guava", "//third_party:jsr305",
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java index c042a1c..a8b8df5 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
@@ -66,6 +66,11 @@ import com.google.devtools.build.lib.pkgcache.PackageManager.PackageManagerStatistics; import com.google.devtools.build.lib.profiler.Profiler; import com.google.devtools.build.lib.profiler.SilentCloseable; +import com.google.devtools.build.lib.server.FailureDetails; +import com.google.devtools.build.lib.server.FailureDetails.Analysis; +import com.google.devtools.build.lib.server.FailureDetails.FailureDetail; +import com.google.devtools.build.lib.server.FailureDetails.TargetPatterns; +import com.google.devtools.build.lib.server.FailureDetails.TargetPatterns.Code; import com.google.devtools.build.lib.skyframe.AspectValueKey; import com.google.devtools.build.lib.skyframe.AspectValueKey.AspectKey; import com.google.devtools.build.lib.skyframe.BuildConfigurationValue; @@ -523,8 +528,8 @@ ImmutableSet<ConfiguredTarget> parallelTests = testsPair.first; ImmutableSet<ConfiguredTarget> exclusiveTests = testsPair.second; - String error = - createErrorMessage(loadingResult, skyframeAnalysisResult, topLevelTargetsWithConfigs); + FailureDetail failureDetail = + createFailureDetail(loadingResult, skyframeAnalysisResult, topLevelTargetsWithConfigs); final WalkableGraph graph = skyframeAnalysisResult.getWalkableGraph(); final ActionGraph actionGraph = @@ -558,7 +563,7 @@ aspects, allTargetsToTest == null ? null : ImmutableList.copyOf(allTargetsToTest), ImmutableSet.copyOf(targetsToSkip), - error, + failureDetail, actionGraph, artifactsToOwnerLabelsBuilder.build(), parallelTests, @@ -575,22 +580,38 @@ * interleaved, but sequential on the single target scale). */ @Nullable - public static String createErrorMessage( + public static FailureDetail createFailureDetail( TargetPatternPhaseValue loadingResult, @Nullable SkyframeAnalysisResult skyframeAnalysisResult, @Nullable TopLevelTargetsAndConfigsResult topLevelTargetsAndConfigs) { if (loadingResult.hasError()) { - return "command succeeded, but there were errors parsing the target pattern"; + return FailureDetail.newBuilder() + .setMessage("command succeeded, but there were errors parsing the target pattern") + .setTargetPatterns(TargetPatterns.newBuilder().setCode(Code.TARGET_PATTERN_PARSE_FAILURE)) + .build(); } if (loadingResult.hasPostExpansionError() || (skyframeAnalysisResult != null && skyframeAnalysisResult.hasLoadingError())) { - return "command succeeded, but there were loading phase errors"; + return FailureDetail.newBuilder() + .setMessage("command succeeded, but there were loading phase errors") + .setAnalysis(Analysis.newBuilder().setCode(Analysis.Code.GENERIC_LOADING_PHASE_FAILURE)) + .build(); } if (topLevelTargetsAndConfigs != null && topLevelTargetsAndConfigs.hasError()) { - return "command succeeded, but top level configurations could not be created"; + return FailureDetail.newBuilder() + .setMessage("command succeeded, but top level configurations could not be created") + .setBuildConfiguration( + FailureDetails.BuildConfiguration.newBuilder() + .setCode( + FailureDetails.BuildConfiguration.Code + .TOP_LEVEL_CONFIGURATION_CREATION_FAILURE)) + .build(); } if (skyframeAnalysisResult != null && skyframeAnalysisResult.hasAnalysisError()) { - return "command succeeded, but not all targets were analyzed"; + return FailureDetail.newBuilder() + .setMessage("command succeeded, but not all targets were analyzed") + .setAnalysis(Analysis.newBuilder().setCode(Analysis.Code.NOT_ALL_TARGETS_ANALYZED)) + .build(); } return null; }
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/AnalysisPhaseRunner.java b/src/main/java/com/google/devtools/build/lib/buildtool/AnalysisPhaseRunner.java index 4772624..5118fe4 100644 --- a/src/main/java/com/google/devtools/build/lib/buildtool/AnalysisPhaseRunner.java +++ b/src/main/java/com/google/devtools/build/lib/buildtool/AnalysisPhaseRunner.java
@@ -42,11 +42,13 @@ import com.google.devtools.build.lib.profiler.SilentCloseable; import com.google.devtools.build.lib.runtime.BlazeModule; import com.google.devtools.build.lib.runtime.CommandEnvironment; +import com.google.devtools.build.lib.server.FailureDetails.FailureDetail; import com.google.devtools.build.lib.skyframe.BuildConfigurationValue; import com.google.devtools.build.lib.skyframe.BuildInfoCollectionFunction; import com.google.devtools.build.lib.skyframe.PrecomputedValue; import com.google.devtools.build.lib.skyframe.TargetPatternPhaseValue; import com.google.devtools.build.lib.util.AbruptExitException; +import com.google.devtools.build.lib.util.DetailedExitCode; import com.google.devtools.build.lib.util.Pair; import com.google.devtools.build.lib.util.RegexFilter; import com.google.devtools.common.options.OptionsParsingException; @@ -148,9 +150,10 @@ env.getReporter().handle(Event.progress("Loading complete.")); env.getReporter().post(new NoAnalyzeEvent()); logger.atInfo().log("No analysis requested, so finished"); - String errorMessage = BuildView.createErrorMessage(loadingResult, null, null); - if (errorMessage != null) { - throw new BuildFailedException(errorMessage); + FailureDetail failureDetail = BuildView.createFailureDetail(loadingResult, null, null); + if (failureDetail != null) { + throw new BuildFailedException( + failureDetail.getMessage(), DetailedExitCode.of(failureDetail)); } }
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java index 5eaa996..9252143 100644 --- a/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java +++ b/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java
@@ -45,6 +45,7 @@ import com.google.devtools.build.lib.profiler.SilentCloseable; import com.google.devtools.build.lib.runtime.BlazeRuntime; import com.google.devtools.build.lib.runtime.CommandEnvironment; +import com.google.devtools.build.lib.server.FailureDetails.FailureDetail; import com.google.devtools.build.lib.server.FailureDetails.Interrupted.Code; import com.google.devtools.build.lib.util.AbruptExitException; import com.google.devtools.build.lib.util.CrashFailureDetails; @@ -189,9 +190,10 @@ } else { env.getReporter().post(new NoExecutionEvent()); } - String delayedErrorMsg = analysisResult.getError(); - if (delayedErrorMsg != null) { - throw new BuildFailedException(delayedErrorMsg); + FailureDetail delayedFailureDetail = analysisResult.getFailureDetail(); + if (delayedFailureDetail != null) { + throw new BuildFailedException( + delayedFailureDetail.getMessage(), DetailedExitCode.of(delayedFailureDetail)); } } Profiler.instance().markPhase(ProfilePhase.FINISH);
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 3d99f98..c31c653 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
@@ -15,6 +15,7 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; +import com.google.common.base.Strings; import com.google.common.base.Throwables; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Range; @@ -44,6 +45,7 @@ import com.google.devtools.build.lib.server.FailureDetails.Execution; 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.server.FailureDetails.IncludeScanning; import com.google.devtools.build.lib.skyframe.ActionExecutionInactivityWatchdog; import com.google.devtools.build.lib.skyframe.AspectValueKey.AspectKey; import com.google.devtools.build.lib.skyframe.Builder; @@ -330,22 +332,38 @@ actionExecutionCause.getRootCauses(), /*errorAlreadyShown=*/ !actionExecutionCause.showError(), actionExecutionCause.getDetailedExitCode()); - } else if (cause instanceof MissingInputFileException) { - throw new BuildFailedException(cause.getMessage()); - } else if (cause instanceof BuildFileNotFoundException) { + } + if (cause instanceof MissingInputFileException) { + throw (MissingInputFileException) cause; + } + if (cause instanceof BuildFileNotFoundException) { // Sadly, this can happen because we may load new packages during input discovery. Any // failures reading those packages shouldn't terminate the build, but in Skyframe they do. LoggingUtil.logToRemote(Level.WARNING, "undesirable loading exception", cause); - throw new BuildFailedException(cause.getMessage()); - } else { - // We encountered an exception we don't think we should have encountered. This can indicate - // an exception-processing bug in our code, such as lower level exceptions not being properly - // handled, or in our expectations in this method. - BugReport.sendBugReport( - new IllegalStateException("action terminated with unexpected exception", cause)); throw new BuildFailedException( - "Unexpected exception, please file an issue with the Bazel team: " + cause.getMessage()); + cause.getMessage(), + DetailedExitCode.of( + FailureDetail.newBuilder() + .setMessage(Strings.nullToEmpty(cause.getMessage())) + .setIncludeScanning( + IncludeScanning.newBuilder() + .setCode(IncludeScanning.Code.PACKAGE_LOAD_FAILURE)) + .build())); } + // We encountered an exception we don't think we should have encountered. This can indicate + // an exception-processing bug in our code, such as lower level exceptions not being properly + // handled, or in our expectations in this method. + BugReport.sendBugReport( + new IllegalStateException("action terminated with unexpected exception", cause)); + String message = + "Unexpected exception, please file an issue with the Bazel team: " + cause.getMessage(); + throw new BuildFailedException( + message, + DetailedExitCode.of( + FailureDetail.newBuilder() + .setMessage(message) + .setExecution(Execution.newBuilder().setCode(Code.UNEXPECTED_EXCEPTION)) + .build())); } private static int countTestActions(Iterable<ConfiguredTarget> testTargets) {
diff --git a/src/main/protobuf/failure_details.proto b/src/main/protobuf/failure_details.proto index 23fa1b7..7beafc6 100644 --- a/src/main/protobuf/failure_details.proto +++ b/src/main/protobuf/failure_details.proto
@@ -141,6 +141,7 @@ LtoAction lto_action = 169; TestAction test_action = 172; Worker worker = 173; + Analysis analysis = 174; } reserved 102; // For internal use @@ -408,6 +409,7 @@ NON_ACTION_EXECUTION_FAILURE = 34 [(metadata) = { exit_code: 1 }]; CYCLE = 35 [(metadata) = { exit_code: 1 }]; SOURCE_INPUT_MISSING = 36 [(metadata) = { exit_code: 1 }]; + UNEXPECTED_EXCEPTION = 37 [(metadata) = { exit_code: 1 }]; } Code code = 1; @@ -544,6 +546,8 @@ PLATFORM_MAPPING_EVALUATION_FAILURE = 1 [(metadata) = { exit_code: 2 }]; PLATFORM_MAPPINGS_FILE_IS_DIRECTORY = 2 [(metadata) = { exit_code: 1 }]; PLATFORM_MAPPINGS_FILE_NOT_FOUND = 3 [(metadata) = { exit_code: 1 }]; + TOP_LEVEL_CONFIGURATION_CREATION_FAILURE = 4 + [(metadata) = { exit_code: 1 }]; } Code code = 1; @@ -662,6 +666,9 @@ INCLUDE_HINTS_FILE_NOT_IN_PACKAGE = 3 [(metadata) = { exit_code: 36 }]; INCLUDE_HINTS_READ_FAILURE = 4 [(metadata) = { exit_code: 36 }]; ILLEGAL_ABSOLUTE_PATH = 5 [(metadata) = { exit_code: 1 }]; + // TODO(b/138456686): this code should be deprecated in favor of more finely + // resolved loading-phase codes. + PACKAGE_LOAD_FAILURE = 6 [(metadata) = { exit_code: 1 }]; } Code code = 1; @@ -712,6 +719,7 @@ TARGET_PATTERN_FILE_WITH_COMMAND_LINE_PATTERN = 1 [(metadata) = { exit_code: 2 }]; TARGET_PATTERN_FILE_READ_FAILURE = 2 [(metadata) = { exit_code: 2 }]; + TARGET_PATTERN_PARSE_FAILURE = 3 [(metadata) = { exit_code: 1 }]; } Code code = 1; @@ -1007,4 +1015,17 @@ } Code code = 1; -} \ No newline at end of file +} + +message Analysis { + enum Code { + ANALYSIS_UNKNOWN = 0 [(metadata) = { exit_code: 37 }]; + LOAD_FAILURE = 1 [(metadata) = { exit_code: 1 }]; + // TODO(b/138456686): this code should be deprecated in favor of more finely + // resolved loading-phase codes. + GENERIC_LOADING_PHASE_FAILURE = 2 [(metadata) = { exit_code: 1 }]; + NOT_ALL_TARGETS_ANALYZED = 3 [(metadata) = { exit_code: 1 }]; + } + + Code code = 1; +}
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java b/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java index 0e29f01..63f3e65 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java
@@ -1208,7 +1208,8 @@ reporter.removeHandler(failFastHandler); AnalysisResult result = update(defaultFlags().with(Flag.KEEP_GOING), "//a", "//b"); assertThat(result.hasError()).isTrue(); - assertThat(result.getError()).contains("command succeeded, but not all targets were analyzed"); + assertThat(result.getFailureDetail().getMessage()) + .contains("command succeeded, but not all targets were analyzed"); } @Test
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java index 46ee6d3..179e1bc 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java
@@ -584,11 +584,6 @@ assertThat(actual).containsExactlyEntriesIn(expected); } - protected String getAnalysisError() { - ensureUpdateWasCalled(); - return analysisResult.getError(); - } - protected BuildViewForTesting getView() { return buildView; }