Fix a couple of bugs related to error handling for top-level aspects.

--
MOS_MIGRATED_REVID=112561390

3 files changed, 21 insertions, 6 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java
index aafa94364d..032434d804 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java
@@ -41,6 +41,7 @@ import com.google.devtools.build.lib.rules.SkylarkRuleClassFunctions.SkylarkAspe
 import com.google.devtools.build.lib.skyframe.AspectValue.AspectKey;
 import com.google.devtools.build.lib.skyframe.ConfiguredTargetFunction.DependencyEvaluationException;
 import com.google.devtools.build.lib.skyframe.SkyframeExecutor.BuildViewProvider;
+import com.google.devtools.build.lib.skyframe.SkylarkImportLookupFunction.SkylarkImportFailedException;
 import com.google.devtools.build.lib.syntax.Type.ConversionException;
 import com.google.devtools.build.lib.util.Preconditions;
 import com.google.devtools.build.skyframe.SkyFunction;
@@ -72,11 +73,11 @@ public final class AspectFunction implements SkyFunction {
   @Nullable
   public static SkylarkAspect loadSkylarkAspect(
       Environment env, Label extensionLabel, String skylarkValueName)
-      throws ConversionException {
-    
+      throws ConversionException, SkylarkImportFailedException {
     SkyKey importFileKey = SkylarkImportLookupValue.key(extensionLabel, false);
     SkylarkImportLookupValue skylarkImportLookupValue =
-        (SkylarkImportLookupValue) env.getValue(importFileKey);
+        (SkylarkImportLookupValue) env.getValueOrThrow(
+            importFileKey, SkylarkImportFailedException.class);
     if (skylarkImportLookupValue == null) {
       return null;
     }
@@ -107,6 +108,8 @@ public final class AspectFunction implements SkyFunction {
         skylarkAspect =
             loadSkylarkAspect(
                 env, skylarkAspectClass.getExtensionLabel(), skylarkAspectClass.getExportedName());
+      } catch (SkylarkImportFailedException e) {
+        throw new AspectFunctionException(skyKey, e);
       } catch (ConversionException e) {
         throw new AspectFunctionException(skyKey, e);
       }
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 be7ff9bba3..af8997035b 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
@@ -61,6 +61,7 @@ import com.google.devtools.build.lib.skyframe.AspectValue.AspectValueKey;
 import com.google.devtools.build.lib.skyframe.BuildInfoCollectionValue.BuildInfoKeyAndConfig;
 import com.google.devtools.build.lib.skyframe.ConfiguredTargetFunction.ConfiguredValueCreationException;
 import com.google.devtools.build.lib.skyframe.SkyframeActionExecutor.ConflictException;
+import com.google.devtools.build.lib.skyframe.SkylarkImportLookupFunction.SkylarkImportFailedException;
 import com.google.devtools.build.lib.util.Preconditions;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.build.skyframe.CycleInfo;
@@ -425,8 +426,10 @@ public final class SkyframeBuildView {
       // that the only errors should be analysis errors.
       Preconditions.checkState(
           cause instanceof ConfiguredValueCreationException
-              || cause instanceof AspectCreationException // for top-level aspects
-              || cause instanceof ActionConflictException,
+              || cause instanceof ActionConflictException
+              // For top-level aspects
+              || cause instanceof AspectCreationException
+              || cause instanceof SkylarkImportFailedException,
           "%s -> %s",
           key,
           errorInfo);
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ToplevelSkylarkAspectFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ToplevelSkylarkAspectFunction.java
index cac4c02216..a883d60b30 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ToplevelSkylarkAspectFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ToplevelSkylarkAspectFunction.java
@@ -17,8 +17,10 @@ package com.google.devtools.build.lib.skyframe;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.packages.AspectParameters;
 import com.google.devtools.build.lib.rules.SkylarkRuleClassFunctions.SkylarkAspect;
+import com.google.devtools.build.lib.skyframe.AspectFunction.AspectCreationException;
 import com.google.devtools.build.lib.skyframe.AspectValue.SkylarkAspectLoadingKey;
 import com.google.devtools.build.lib.skyframe.SkylarkImportLookupFunction.SkylarkImportFailedException;
 import com.google.devtools.build.lib.syntax.Type.ConversionException;
@@ -53,6 +55,7 @@ public class ToplevelSkylarkAspectFunction implements SkyFunction {
       labelLookupMap =
           SkylarkImportLookupFunction.labelsForAbsoluteImports(ImmutableSet.of(extensionFile), env);
     } catch (SkylarkImportFailedException e) {
+      env.getListener().handle(Event.error(e.getMessage()));
       throw new LoadSkylarkAspectFunctionException(e, skyKey);
     }
     if (labelLookupMap == null) {
@@ -63,8 +66,14 @@ public class ToplevelSkylarkAspectFunction implements SkyFunction {
     try {
       skylarkAspect = AspectFunction.loadSkylarkAspect(
           env, labelLookupMap.get(extensionFile), skylarkValueName);
+    } catch (SkylarkImportFailedException e) {
+      env.getListener().handle(Event.error(e.getMessage()));
+      throw new LoadSkylarkAspectFunctionException(
+          new AspectCreationException(e.getMessage()), skyKey);
     } catch (ConversionException e) {
-      throw new LoadSkylarkAspectFunctionException(e, skyKey);
+      env.getListener().handle(Event.error(e.getMessage()));
+      throw new LoadSkylarkAspectFunctionException(
+          new AspectCreationException(e.getMessage()), skyKey);
     }
     if (skylarkAspect == null) {
       return null;