Skip to content

Commit

Permalink
When not keeping edges, don't attempt to find the origin of an unexpe…
Browse files Browse the repository at this point in the history
…cted exception in `SkyframeErrorProcessor`.

Currently, builds without edges will crash if there's an unexpected exception.

PiperOrigin-RevId: 557521507
Change-Id: Ia68525c8d3c0d1a8dbdcbe1ad3aebab2bd042c18
  • Loading branch information
justinhorvitz authored and copybara-github committed Aug 16, 2023
1 parent 0589995 commit 96c08ae
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,7 @@ public SkyframeAnalysisResult configureTargets(
skyframeExecutor.getCyclesReporter(),
eventHandler,
keepGoing,
skyframeExecutor.tracksStateForIncrementality(),
eventBus,
bugReporter);

Expand Down Expand Up @@ -712,9 +713,10 @@ public SkyframeAnalysisResult analyzeAndExecuteTargets(
skyframeExecutor.getCyclesReporter(),
eventHandler,
keepGoing,
skyframeExecutor.tracksStateForIncrementality(),
eventBus,
bugReporter,
/*includeExecutionPhase=*/ true)
/* includeExecutionPhase= */ true)
.executionDetailedExitCode());
}
}
Expand All @@ -737,6 +739,7 @@ public SkyframeAnalysisResult analyzeAndExecuteTargets(
skyframeExecutor.getCyclesReporter(),
eventHandler,
keepGoing,
skyframeExecutor.tracksStateForIncrementality(),
eventBus,
bugReporter,
/* includeExecutionPhase= */ true)
Expand Down Expand Up @@ -781,9 +784,10 @@ public SkyframeAnalysisResult analyzeAndExecuteTargets(
skyframeExecutor.getCyclesReporter(),
eventHandler,
keepGoing,
skyframeExecutor.tracksStateForIncrementality(),
eventBus,
bugReporter,
/*includeExecutionPhase=*/ true);
/* includeExecutionPhase= */ true);
detailedExitCodes.add(errorProcessingResult.executionDetailedExitCode());

foundActionConflictInLatestCheck = !errorProcessingResult.actionConflicts().isEmpty();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ static ErrorProcessingResult processAnalysisErrors(
CyclesReporter cyclesReporter,
ExtendedEventHandler eventHandler,
boolean keepGoing,
boolean keepEdges,
@Nullable EventBus eventBus,
BugReporter bugReporter)
throws InterruptedException, ViewCreationFailedException {
Expand All @@ -194,9 +195,10 @@ static ErrorProcessingResult processAnalysisErrors(
cyclesReporter,
eventHandler,
keepGoing,
keepEdges,
eventBus,
bugReporter,
/*includeExecutionPhase=*/ false);
/* includeExecutionPhase= */ false);
} catch (BuildFailedException | TestExecException unexpected) {
throw new IllegalStateException("Unexpected execution phase exception: ", unexpected);
}
Expand Down Expand Up @@ -232,10 +234,13 @@ static ErrorProcessingResult processErrors(
CyclesReporter cyclesReporter,
ExtendedEventHandler eventHandler,
boolean keepGoing,
boolean keepEdges,
@Nullable EventBus eventBus,
@Nullable BugReporter bugReporter,
boolean includeExecutionPhase)
throws InterruptedException, ViewCreationFailedException, BuildFailedException,
throws InterruptedException,
ViewCreationFailedException,
BuildFailedException,
TestExecException {
boolean inBuildViewTest = eventBus == null;
ViewCreationFailedException noKeepGoingAnalysisExceptionAspect = null;
Expand All @@ -260,9 +265,10 @@ static ErrorProcessingResult processErrors(

SkyKey errorKey = getEffectiveErrorKey(errorEntry);
if (includeExecutionPhase) {
assertValidAnalysisOrExecutionException(errorInfo, errorKey, result.getWalkableGraph());
assertValidAnalysisOrExecutionException(
errorInfo, errorKey, result.getWalkableGraph(), keepEdges);
} else {
assertValidAnalysisException(errorInfo, errorKey, result.getWalkableGraph());
assertValidAnalysisException(errorInfo, errorKey, result.getWalkableGraph(), keepEdges);
}
Exception cause = errorInfo.getException();
Preconditions.checkState(cause != null || !errorInfo.getCycleInfo().isEmpty(), errorInfo);
Expand Down Expand Up @@ -663,7 +669,8 @@ private static Label maybeGetConfiguredTargetCycleCulprit(
}

private static void assertValidAnalysisException(
ErrorInfo errorInfo, SkyKey key, WalkableGraph walkableGraph) throws InterruptedException {
ErrorInfo errorInfo, SkyKey key, WalkableGraph walkableGraph, boolean keepEdges)
throws InterruptedException {
Throwable cause = errorInfo.getException();
if (cause == null) {
// Cycle.
Expand All @@ -675,11 +682,12 @@ private static void assertValidAnalysisException(
return;
}

logUnexpectedExceptionOrigin(errorInfo, key, walkableGraph, cause);
logUnexpectedExceptionOrigin(errorInfo, key, walkableGraph, cause, keepEdges);
}

private static void assertValidAnalysisOrExecutionException(
ErrorInfo errorInfo, SkyKey key, WalkableGraph walkableGraph) throws InterruptedException {
ErrorInfo errorInfo, SkyKey key, WalkableGraph walkableGraph, boolean keepEdges)
throws InterruptedException {
Throwable cause = errorInfo.getException();
if (cause == null) {
// Cycle.
Expand All @@ -693,16 +701,25 @@ private static void assertValidAnalysisOrExecutionException(
return;
}

logUnexpectedExceptionOrigin(errorInfo, key, walkableGraph, cause);
logUnexpectedExceptionOrigin(errorInfo, key, walkableGraph, cause, keepEdges);
}

/**
* Walk the graph to find a path to the lowest-level node that threw unexpected exception and log
* it.
*/
private static void logUnexpectedExceptionOrigin(
ErrorInfo errorInfo, SkyKey key, WalkableGraph walkableGraph, Throwable cause)
ErrorInfo errorInfo,
SkyKey key,
WalkableGraph walkableGraph,
Throwable cause,
boolean keepEdges)
throws InterruptedException {
if (!keepEdges) {
// Can't traverse the graph to find the origin.
logUnexpectedException(key, errorInfo, "direct deps not stored");
return;
}
List<SkyKey> path = new ArrayList<>();
try {
SkyKey currentKey = key;
Expand All @@ -728,10 +745,14 @@ private static void logUnexpectedExceptionOrigin(
}
} while (foundDep);
} finally {
BugReport.logUnexpected("Unexpected analysis error: %s -> %s, (%s)", key, errorInfo, path);
logUnexpectedException(key, errorInfo, path);
}
}

private static void logUnexpectedException(SkyKey key, ErrorInfo errorInfo, Object extraInfo) {
BugReport.logUnexpected("Unexpected analysis error: %s -> %s, (%s)", key, errorInfo, extraInfo);
}

@Nullable
private static DetailedException convertToAnalysisException(Throwable cause) {
// The cause may be NoSuch{Target,Package}Exception if we run the reduced loading phase and then
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3835,6 +3835,7 @@ public void acceptConfiguredTargetAndDataError(NoSuchThingException error) {}
cyclesReporter,
eventHandler,
/* keepGoing= */ true,
tracksStateForIncrementality(),
/* eventBus= */ null,
bugReporter);
} catch (ViewCreationFailedException ignored) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,12 @@ public void testProcessErrors_analysisErrorNoKeepGoing_throwsException(
() ->
SkyframeErrorProcessor.processErrors(
result,
/*cyclesReporter=*/ new CyclesReporter(),
/*eventHandler=*/ mock(ExtendedEventHandler.class),
/*keepGoing=*/ false,
/*eventBus=*/ null,
/*bugReporter=*/ null,
/* cyclesReporter= */ new CyclesReporter(),
/* eventHandler= */ mock(ExtendedEventHandler.class),
/* keepGoing= */ false,
/* keepEdges= */ true,
/* eventBus= */ null,
/* bugReporter= */ null,
includeExecutionPhase));
assertThat(thrown).hasCauseThat().isEqualTo(analysisException);
}
Expand Down

0 comments on commit 96c08ae

Please sign in to comment.