Skip to content

Commit

Permalink
Fix a bug with non-execution phase errors being considered
Browse files Browse the repository at this point in the history
UndetailedExecutionCause.

Also, send a bug report for actual undetailed execution errors, instead of just
logging.

PiperOrigin-RevId: 439253033
  • Loading branch information
joeleba authored and copybara-github committed Apr 4, 2022
1 parent 93677c6 commit 436d04d
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,11 @@
import com.google.devtools.build.lib.skyframe.AspectKeyCreator.AspectKey;
import com.google.devtools.build.lib.util.DetailedExitCode;
import com.google.devtools.build.skyframe.WalkableGraph;
import javax.annotation.Nullable;

/** Encapsulates the raw analysis result of top level targets and aspects coming from Skyframe. */
public final class SkyframeAnalysisAndExecutionResult extends SkyframeAnalysisResult {
private final DetailedExitCode representativeExecutionExitCode;
@Nullable private final DetailedExitCode representativeExecutionExitCode;

private SkyframeAnalysisAndExecutionResult(
boolean hasLoadingError,
Expand All @@ -48,6 +49,7 @@ private SkyframeAnalysisAndExecutionResult(
this.representativeExecutionExitCode = representativeExecutionExitCode;
}

@Nullable
public DetailedExitCode getRepresentativeExecutionExitCode() {
return representativeExecutionExitCode;
}
Expand Down Expand Up @@ -96,7 +98,7 @@ public static SkyframeAnalysisAndExecutionResult withErrors(
WalkableGraph walkableGraph,
ImmutableMap<AspectKey, ConfiguredAspect> aspects,
PackageRoots packageRoots,
DetailedExitCode representativeExecutionExitCode) {
@Nullable DetailedExitCode representativeExecutionExitCode) {
return new SkyframeAnalysisAndExecutionResult(
hasLoadingError,
hasAnalysisError,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,7 @@ static ErrorProcessingResult processErrors(
// turns out to be with execution.
boolean hasAnalysisError = true;
ViewCreationFailedException noKeepGoingAnalysisExceptionAspect = null;
DetailedExitCode detailedExitCode = null;
Throwable undetailedCause = null;
DetailedExitCode representativeExecutionDetailedExitCode = null;
Map<ActionAnalysisMetadata, ConflictException> actionConflicts = new HashMap<>();
for (Map.Entry<SkyKey, ErrorInfo> errorEntry : result.errorMap().entrySet()) {
SkyKey errorKey = getErrorKey(errorEntry);
Expand Down Expand Up @@ -275,17 +274,21 @@ static ErrorProcessingResult processErrors(
actionConflicts.putAll(tlce.getTransitiveActionConflicts());
continue;
} else if (isExecutionException(cause)) {
detailedExitCode =
DetailedExitCode detailedExitCode = DetailedException.getDetailedExitCode(cause);
if (detailedExitCode == null) {
detailedExitCode = createDetailedExitCodeForUndetailedExecutionCause(result, cause);
}
representativeExecutionDetailedExitCode =
DetailedExitCodeComparator.chooseMoreImportantWithFirstIfTie(
detailedExitCode, ((DetailedException) cause).getDetailedExitCode());
representativeExecutionDetailedExitCode, detailedExitCode);
rootCauses =
cause instanceof ActionExecutionException
? ((ActionExecutionException) cause).getRootCauses()
: NestedSetBuilder.emptySet(Order.STABLE_ORDER);
hasAnalysisError = false;
} else {
// TODO(ulfjack): Report something!
undetailedCause = cause;
BugReport.logUnexpected(
cause, "Unexpected cause encountered while evaluating: %s", errorKey);
}

if (!inTest) {
Expand Down Expand Up @@ -325,12 +328,11 @@ static ErrorProcessingResult processErrors(
throw noKeepGoingAnalysisExceptionAspect;
}

if (includeExecutionPhase && detailedExitCode == null) {
detailedExitCode = createDetailedExitCodeForUndetailedExecutionCause(result, undetailedCause);
}

return ErrorProcessingResult.create(
hasLoadingError, hasAnalysisError, ImmutableMap.copyOf(actionConflicts), detailedExitCode);
hasLoadingError,
hasAnalysisError,
ImmutableMap.copyOf(actionConflicts),
representativeExecutionDetailedExitCode);
}

private static SkyKey getErrorKey(Entry<SkyKey, ErrorInfo> errorEntry) {
Expand Down Expand Up @@ -631,14 +633,14 @@ static void rethrow(

private static DetailedExitCode createDetailedExitCodeForUndetailedExecutionCause(
EvaluationResult<?> result, Throwable undetailedCause) {
// TODO(b/227660368): These warning logs should be a bug report, but tests currently fail.
if (undetailedCause == null) {
logger.atWarning().log("No exceptions found despite error in %s", result);
BugReport.sendBugReport("No exceptions found despite error in %s", result);
return createDetailedExecutionExitCode(
"keep_going execution failed without an action failure",
Execution.Code.NON_ACTION_EXECUTION_FAILURE);
}
logger.atWarning().withCause(undetailedCause).log("No detailed exception found in %s", result);
BugReport.sendBugReport(
new IllegalStateException("No detailed exception found in " + result, undetailedCause));
return createDetailedExecutionExitCode(
"keep_going execution failed without an action failure: "
+ undetailedCause.getMessage()
Expand Down

0 comments on commit 436d04d

Please sign in to comment.