Improve SkyframeExecutor.createBuildConfigurationKey error handling t…

…o report cycles.

With the previous code, the error handling crashes with an NPE.

Analysis of the skyframe dependencies show that it is not possible now to create a cycle from this code path, but upcoming changes for platform-based flags make it a possibility.

Work towards platform-based flags: #19409.

PiperOrigin-RevId: 607075106
Change-Id: Ic10d1c377ffd37c40b574d2b3e0324602b671cc7

src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java

@@ -241,6 +241,7 @@
 import com.google.devtools.build.skyframe.NodeEntry.DirtyType;
 import com.google.devtools.build.skyframe.RecordingDifferencer;
 import com.google.devtools.build.skyframe.SkyFunction;
+import com.google.devtools.build.skyframe.SkyFunction.Environment;
 import com.google.devtools.build.skyframe.SkyFunctionName;
 import com.google.devtools.build.skyframe.SkyKey;
 import com.google.devtools.build.skyframe.SkyValue;
@@ -1911,8 +1912,35 @@ private BuildConfigurationKey createBuildConfigurationKey(
         evaluateSkyKeys(eventHandler, ImmutableSet.of(key));
     // Handle all possible errors by reporting them to the user.
     if (evaluationResult.hasError()) {
-      throw new InvalidConfigurationException(
-          Code.PLATFORM_MAPPING_EVALUATION_FAILURE, evaluationResult.getError().getException());
+      Map.Entry<SkyKey, ErrorInfo> firstError =
+          Iterables.get(evaluationResult.errorMap().entrySet(), 0);
+      SkyKey errorKey = firstError.getKey();
+      ErrorInfo error = firstError.getValue();
+      Throwable e = error.getException();
+
+      if (e != null) {
+        // Wrap exceptions related to loading
+        if (e instanceof NoSuchThingException) {
+          throw new InvalidConfigurationException(
+              ((NoSuchThingException) e).getDetailedExitCode(), e);
+        }
+        Throwables.throwIfInstanceOf(e, InvalidConfigurationException.class);
+        // If we get here, e is non-null but not an InvalidConfigurationException, so wrap it and
+        // throw.
+        throw new InvalidConfigurationException(Code.PLATFORM_MAPPING_EVALUATION_FAILURE, e);
+      } else if (!error.getCycleInfo().isEmpty()) {
+        // This should not ever happen: there should not be a way for BuildConfigurationKeyValue.Key
+        // to produce a skyframe cycle. Produce a basic error message for developers
+        // to use to track down and fix the problem.
+        // Unfortunately, there's no way to express this as an invariant, so manual inspection of
+        // skyfunctions is the only way to prevent this.
+        cyclesReporter.reportCycles(error.getCycleInfo(), errorKey, eventHandler);
+        throw new InvalidConfigurationException(
+            "cannot load build configuration key because of this cycle", Code.CYCLE);
+      }
+
+      // Unclear what could have happened if the exception is null and there isn't a cycle.
+      throw new IllegalStateException("Unknown error during configuration creation evaluation", e);
     }
     BuildConfigurationKeyValue buildConfigurationKeyValue =
         (BuildConfigurationKeyValue) evaluationResult.get(key);