Don't crash if there are unexpected exceptions in analysis, try to find a path to the exception and just send a bug report. We know there are lurking bugs in how we propagate exceptions, no need to punish users.
If this detection code itself crashes, we'll still send some kind of report.
Also add a toString to SkylarkAspectLoadingKey, because why not.
PiperOrigin-RevId: 295224092
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java
index ecc1a4e..d3b133f 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java
@@ -14,6 +14,7 @@
package com.google.devtools.build.lib.skyframe;
+import com.google.common.base.MoreObjects;
import com.google.common.base.Objects;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
@@ -389,6 +390,17 @@
&& Objects.equal(skylarkValueName, that.skylarkValueName);
}
+ @Override
+ public String toString() {
+ return MoreObjects.toStringHelper(this)
+ .add("targetLabel", targetLabel)
+ .add("aspectConfigurationKey", aspectConfigurationKey)
+ .add("baseConfiguredTargetKey", baseConfiguredTargetKey)
+ .add("skylarkFileLabel", skylarkFileLabel)
+ .add("skylarkValueName", skylarkValueName)
+ .toString();
+ }
+
AspectKey toAspectKey(AspectClass aspectClass) {
return AspectKey.createAspectKey(
targetLabel,
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
index a21ca77..8a2b4e7 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
@@ -52,6 +52,7 @@
import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider;
import com.google.devtools.build.lib.analysis.config.CoreOptions;
import com.google.devtools.build.lib.analysis.config.FragmentClassSet;
+import com.google.devtools.build.lib.bugreport.BugReport;
import com.google.devtools.build.lib.buildtool.BuildRequestOptions;
import com.google.devtools.build.lib.causes.Cause;
import com.google.devtools.build.lib.causes.LabelCause;
@@ -87,7 +88,9 @@
import com.google.devtools.build.skyframe.SkyFunction.Environment;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
+import com.google.devtools.build.skyframe.WalkableGraph;
import com.google.devtools.common.options.OptionDefinition;
+import java.util.ArrayList;
import java.util.Collection;
import java.util.HashSet;
import java.util.List;
@@ -602,14 +605,15 @@
SkyframeExecutor skyframeExecutor,
ExtendedEventHandler eventHandler,
boolean keepGoing,
- @Nullable EventBus eventBus) {
+ @Nullable EventBus eventBus)
+ throws InterruptedException {
boolean inTest = eventBus == null;
boolean hasLoadingError = false;
ViewCreationFailedException noKeepGoingException = null;
for (Map.Entry<SkyKey, ErrorInfo> errorEntry : result.errorMap().entrySet()) {
SkyKey errorKey = errorEntry.getKey();
ErrorInfo errorInfo = errorEntry.getValue();
- assertSaneAnalysisError(errorInfo, errorKey);
+ assertSaneAnalysisError(errorInfo, errorKey, result.getWalkableGraph());
skyframeExecutor
.getCyclesReporter().reportCycles(errorInfo.getCycleInfo(), errorKey, eventHandler);
Exception cause = errorInfo.getException();
@@ -752,29 +756,50 @@
return null;
}
- private static void assertSaneAnalysisError(ErrorInfo errorInfo, SkyKey key) {
+ private static void assertSaneAnalysisError(
+ ErrorInfo errorInfo, SkyKey key, WalkableGraph walkableGraph) throws InterruptedException {
Throwable cause = errorInfo.getException();
- if (cause != null) {
- // We should only be trying to configure targets when the loading phase succeeds, meaning
- // that the only errors should be analysis errors.
- Preconditions.checkState(
- cause instanceof ConfiguredValueCreationException
- || cause instanceof ActionConflictException
- || cause instanceof CcCrosstoolException
- // For top-level aspects
- || cause instanceof AspectCreationException
- || cause instanceof SkylarkImportFailedException
- // Only if we run the reduced loading phase and then analyze with --nokeep_going.
- || cause instanceof NoSuchTargetException
- || cause instanceof NoSuchPackageException
- // Platform-related:
- || cause instanceof InvalidExecutionPlatformLabelException,
- "%s -> %s",
- key,
- errorInfo);
+ // We should only be trying to configure targets when the loading phase succeeds, meaning
+ // that the only errors should be analysis errors.
+ if (cause != null && !isSaneAnalysisError(cause)) {
+ // Walk the graph to find a path to the lowest-level node that threw unexpected exception.
+ List<SkyKey> path = new ArrayList<>();
+ try {
+ SkyKey currentKey = key;
+ boolean foundDep;
+ do {
+ path.add(currentKey);
+ foundDep = false;
+ for (SkyKey dep : walkableGraph.getDirectDeps(currentKey)) {
+ if (cause.equals(walkableGraph.getException(dep))) {
+ currentKey = dep;
+ foundDep = true;
+ break;
+ }
+ }
+ } while (foundDep);
+ } finally {
+ BugReport.sendBugReport(
+ new IllegalStateException(
+ "Unexpected analysis error: " + key + " -> " + errorInfo + ", (" + path + ")"));
+ }
}
}
+ private static boolean isSaneAnalysisError(Throwable cause) {
+ return cause instanceof ConfiguredValueCreationException
+ || cause instanceof ActionConflictException
+ || cause instanceof CcCrosstoolException
+ // For top-level aspects
+ || cause instanceof AspectCreationException
+ || cause instanceof SkylarkImportFailedException
+ // Only if we run the reduced loading phase and then analyze with --nokeep_going.
+ || cause instanceof NoSuchTargetException
+ || cause instanceof NoSuchPackageException
+ // Platform-related:
+ || cause instanceof InvalidExecutionPlatformLabelException;
+ }
+
/** Special flake for error cases when loading CROSSTOOL for C++ rules */
// TODO(b/110087561): Remove when CROSSTOOL file is not loaded anymore
public static class CcCrosstoolException extends Exception {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
index fe51536..b9ebe6f 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
@@ -1664,7 +1664,7 @@
ExtendedEventHandler eventHandler,
BuildConfiguration originalConfig,
Iterable<Dependency> keys)
- throws TransitionException, InvalidConfigurationException {
+ throws TransitionException, InvalidConfigurationException, InterruptedException {
return getConfiguredTargetMapForTesting(eventHandler, originalConfig, keys).values().asList();
}
@@ -1681,7 +1681,7 @@
ExtendedEventHandler eventHandler,
BuildConfigurationValue.Key originalConfig,
Iterable<Dependency> keys)
- throws TransitionException, InvalidConfigurationException {
+ throws InvalidConfigurationException, InterruptedException {
return getConfiguredTargetMapForTesting(eventHandler, originalConfig, keys).values().asList();
}
@@ -1699,7 +1699,7 @@
ExtendedEventHandler eventHandler,
BuildConfigurationValue.Key originalConfig,
Iterable<Dependency> keys)
- throws TransitionException, InvalidConfigurationException {
+ throws InvalidConfigurationException, InterruptedException {
return getConfiguredTargetMapForTesting(
eventHandler, getConfiguration(eventHandler, originalConfig), keys);
}
@@ -1718,7 +1718,7 @@
ExtendedEventHandler eventHandler,
BuildConfiguration originalConfig,
Iterable<Dependency> keys)
- throws InvalidConfigurationException {
+ throws InvalidConfigurationException, InterruptedException {
checkActive();
Multimap<Dependency, BuildConfiguration> configs;
@@ -2294,7 +2294,7 @@
@Nullable
public ConfiguredTarget getConfiguredTargetForTesting(
ExtendedEventHandler eventHandler, Label label, BuildConfiguration configuration)
- throws TransitionException, InvalidConfigurationException {
+ throws TransitionException, InvalidConfigurationException, InterruptedException {
return getConfiguredTargetForTesting(eventHandler, label, configuration, NoTransition.INSTANCE);
}
@@ -2306,7 +2306,7 @@
Label label,
BuildConfiguration configuration,
ConfigurationTransition transition)
- throws TransitionException, InvalidConfigurationException {
+ throws TransitionException, InvalidConfigurationException, InterruptedException {
ConfiguredTargetAndData configuredTargetAndData =
getConfiguredTargetAndDataForTesting(eventHandler, label, configuration, transition);
return configuredTargetAndData == null ? null : configuredTargetAndData.getConfiguredTarget();
@@ -2319,7 +2319,7 @@
Label label,
BuildConfiguration configuration,
ConfigurationTransition transition)
- throws TransitionException, InvalidConfigurationException {
+ throws TransitionException, InvalidConfigurationException, InterruptedException {
return Iterables.getFirst(
getConfiguredTargetsForTesting(
eventHandler,
@@ -2336,7 +2336,7 @@
@Nullable
public ConfiguredTargetAndData getConfiguredTargetAndDataForTesting(
ExtendedEventHandler eventHandler, Label label, BuildConfiguration configuration)
- throws TransitionException, InvalidConfigurationException {
+ throws TransitionException, InvalidConfigurationException, InterruptedException {
return getConfiguredTargetAndDataForTesting(
eventHandler, label, configuration, NoTransition.INSTANCE);
}
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 a7ca224..c658486 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
@@ -446,7 +446,9 @@
}
try {
return skyframeExecutor.getConfiguredTargetAndDataForTesting(reporter, parsedLabel, config);
- } catch (StarlarkTransition.TransitionException | InvalidConfigurationException e) {
+ } catch (StarlarkTransition.TransitionException
+ | InvalidConfigurationException
+ | InterruptedException e) {
throw new AssertionError(e);
}
}
@@ -496,7 +498,9 @@
try {
return skyframeExecutor.getConfiguredTargetAndDataForTesting(
reporter, parsedLabel, configuration);
- } catch (StarlarkTransition.TransitionException | InvalidConfigurationException e) {
+ } catch (StarlarkTransition.TransitionException
+ | InvalidConfigurationException
+ | InterruptedException e) {
throw new AssertionError(e);
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewForTesting.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewForTesting.java
index f81268f..7bb75a7 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewForTesting.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewForTesting.java
@@ -342,7 +342,8 @@
*/
private ImmutableMap<Label, ConfigMatchingProvider> getConfigurableAttributeKeysForTesting(
ExtendedEventHandler eventHandler, TargetAndConfiguration ctg)
- throws StarlarkTransition.TransitionException, InvalidConfigurationException {
+ throws StarlarkTransition.TransitionException, InvalidConfigurationException,
+ InterruptedException {
if (!(ctg.getTarget() instanceof Rule)) {
return ImmutableMap.of();
}
@@ -419,7 +420,8 @@
@VisibleForTesting
public ConfiguredTarget getConfiguredTargetForTesting(
ExtendedEventHandler eventHandler, Label label, BuildConfiguration config)
- throws StarlarkTransition.TransitionException, InvalidConfigurationException {
+ throws StarlarkTransition.TransitionException, InvalidConfigurationException,
+ InterruptedException {
ConfigurationTransition transition =
getTopLevelTransitionForTarget(label, config, eventHandler);
if (transition == null) {
@@ -429,9 +431,10 @@
}
@VisibleForTesting
- public ConfiguredTargetAndData getConfiguredTargetAndDataForTesting(
+ ConfiguredTargetAndData getConfiguredTargetAndDataForTesting(
ExtendedEventHandler eventHandler, Label label, BuildConfiguration config)
- throws StarlarkTransition.TransitionException, InvalidConfigurationException {
+ throws StarlarkTransition.TransitionException, InvalidConfigurationException,
+ InterruptedException {
ConfigurationTransition transition =
getTopLevelTransitionForTarget(label, config, eventHandler);
if (transition == null) {
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
index 18fe94c..b62317d 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
@@ -903,7 +903,9 @@
try {
return view.getConfiguredTargetForTesting(
reporter, BlazeTestUtils.convertLabel(label), config);
- } catch (InvalidConfigurationException | StarlarkTransition.TransitionException e) {
+ } catch (InvalidConfigurationException
+ | StarlarkTransition.TransitionException
+ | InterruptedException e) {
throw new AssertionError(e);
}
}
@@ -913,7 +915,8 @@
*/
protected ConfiguredTargetAndData getConfiguredTargetAndData(
Label label, BuildConfiguration config)
- throws StarlarkTransition.TransitionException, InvalidConfigurationException {
+ throws StarlarkTransition.TransitionException, InvalidConfigurationException,
+ InterruptedException {
return view.getConfiguredTargetAndDataForTesting(reporter, label, config);
}
@@ -924,7 +927,7 @@
*/
public ConfiguredTargetAndData getConfiguredTargetAndData(String label)
throws LabelSyntaxException, StarlarkTransition.TransitionException,
- InvalidConfigurationException {
+ InvalidConfigurationException, InterruptedException {
return getConfiguredTargetAndData(Label.parseAbsolute(label, ImmutableMap.of()), targetConfig);
}
@@ -1788,7 +1791,8 @@
ctad = getConfiguredTargetAndData(ct.getLabel().toString());
} catch (LabelSyntaxException
| StarlarkTransition.TransitionException
- | InvalidConfigurationException e) {
+ | InvalidConfigurationException
+ | InterruptedException e) {
throw new RuntimeException(e);
}
return getMapperFromConfiguredTargetAndTarget(ctad);
diff --git a/src/test/java/com/google/devtools/build/lib/buildtool/util/BuildIntegrationTestCase.java b/src/test/java/com/google/devtools/build/lib/buildtool/util/BuildIntegrationTestCase.java
index 80707f5..43952d3 100644
--- a/src/test/java/com/google/devtools/build/lib/buildtool/util/BuildIntegrationTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/buildtool/util/BuildIntegrationTestCase.java
@@ -465,7 +465,7 @@
protected ConfiguredTarget getConfiguredTarget(
ExtendedEventHandler eventHandler, Label label, BuildConfiguration config)
- throws TransitionException, InvalidConfigurationException {
+ throws TransitionException, InvalidConfigurationException, InterruptedException {
return getSkyframeExecutor().getConfiguredTargetForTesting(eventHandler, label, config);
}
@@ -498,7 +498,7 @@
* If they used multiple different configurations, or if none of them had a configuration, then
* falls back to the base top-level configuration.
*/
- protected BuildConfiguration getTargetConfiguration() throws InterruptedException {
+ protected BuildConfiguration getTargetConfiguration() {
BuildConfiguration baseConfiguration =
Iterables.getOnlyElement(getConfigurationCollection().getTargetConfigurations());
BuildResult result = getResult();
@@ -540,14 +540,6 @@
}
/**
- * Create a BuildRequest for the specified list of targets, using the
- * currently-installed request options.
- */
- protected BuildRequest createRequest(String... targets) throws Exception {
- return createNewRequest("BuildIntegrationTestCase", targets);
- }
-
- /**
* Creates a BuildRequest for either blaze build or blaze analyze, using the
* currently-installed request options.
* @param commandName blaze build or analyze command
@@ -784,7 +776,7 @@
protected ConfiguredTargetAndData getConfiguredTargetAndTarget(
ExtendedEventHandler eventHandler, Label label, BuildConfiguration config)
- throws TransitionException, InvalidConfigurationException {
+ throws TransitionException, InvalidConfigurationException, InterruptedException {
return getSkyframeExecutor().getConfiguredTargetAndDataForTesting(eventHandler, label, config);
}
