When StateMachine looks up a SkyKey and is hooked up with a seque…

…ntial `StateMachine runAfter`. If looking up the `SkyKey` throws an exception, the next `StateMachine runAfter` will not be invoked.

This is caused by mistakenly calling `tryHandleException(exception)` in `Lookup#ValueOrExceptionXLookup#doLookup()` method.

In order to fix this bug, I did the following work in this change:
* Rename relevant function names to `acceptValueInternal()` and `tryHandleExceptionInternal()`, and mark their visibility as `protected`. This will clearly indicate these two methods should not be reached outside `Lookup` class.
* Correct to call `tryHandleExceptionInternal(skyKey, exception)` in `Lookup#ValueOrExceptionXLookup#doLookup()`.
* Add more unit tests to cover calling `Lookup#ValueOrExceptionXLookup#doLookup()`. This method is only invoked when current `StateMachine` looks up **one** newly added `SkyKey`. `X` number of `Exception` types, can be 2 or 3.
* Correct a wrong error message assertion in `ConstraintsTest#invalidSelectKeyError`.
* Fix a minor bug in `TargetAndConfigurationProducer#RuleTransitionApplier#computeTransition()` State Machine method. Directly `return DONE` if getting `configConditions` fails.

PiperOrigin-RevId: 571124557
Change-Id: I5fd9697808d7ff567bc866b5e5c61b662a66a31c

src/main/java/com/google/devtools/build/lib/analysis/producers/TargetAndConfigurationProducer.java

@@ -444,12 +444,26 @@ public StateMachine computeConfigConditions(Tasks tasks) {
       return DONE;
     }
 
+    @Override
+    public void acceptConfigConditions(ConfigConditions configConditions) {
+      this.configConditions = configConditions;
+    }
+
+    @Override
+    public void acceptConfigConditionsError(ConfiguredValueCreationException e) {
+      emitTransitionErrorMessage(e.getMessage());
+    }
+
     public StateMachine computeTransition(Tasks tasks) {
+      if (configConditions == null) {
+        return DONE;
+      }
+
       // logic to compute transition with ConfigConditions
       var transitionData =
           RuleTransitionData.create(
               target.getAssociatedRule(),
-              configConditions != null ? configConditions.asProviders() : null,
+              configConditions.asProviders(),
               preRuleTransitionKey.getConfigurationKey().getOptionsChecksum());
 
       ConfigurationTransition transition = null;
@@ -506,16 +520,6 @@ public void acceptTransitionError(OptionsParsingException e) {
       emitTransitionErrorMessage(e.getMessage());
     }
 
-    @Override
-    public void acceptConfigConditionsError(ConfiguredValueCreationException e) {
-      emitTransitionErrorMessage(e.getMessage());
-    }
-
-    @Override
-    public void acceptConfigConditions(ConfigConditions configConditions) {
-      this.configConditions = configConditions;
-    }
-
     @Override
     public void acceptPlatformInfo(PlatformInfo info) {
       this.platformInfo = info;

src/main/java/com/google/devtools/build/skyframe/state/Lookup.java

@@ -19,6 +19,7 @@
 import com.google.devtools.build.skyframe.SkyKey;
 import com.google.devtools.build.skyframe.SkyValue;
 import com.google.devtools.build.skyframe.SkyframeLookupResult;
+import com.google.errorprone.annotations.ForOverride;
 import java.util.function.Consumer;
 
 /** Captures information about a lookup requested by a state machine. */
@@ -48,22 +49,24 @@ final SkyKey key() {
 
   @Override
   public final void acceptValue(SkyKey unusedKey, SkyValue value) {
-    acceptValue(value);
+    acceptValueInternal(value);
     parent.signalChildDoneAndEnqueueIfReady();
   }
 
-  abstract void acceptValue(SkyValue value);
+  @ForOverride
+  protected abstract void acceptValueInternal(SkyValue value);
 
   @Override
   public final boolean tryHandleException(SkyKey unusedKey, Exception exception) {
-    boolean handled = tryHandleException(exception);
+    boolean handled = tryHandleExceptionInternal(exception);
     if (handled) {
       parent.signalChildDoneAndEnqueueIfReady();
     }
     return handled;
   }
 
-  abstract boolean tryHandleException(Exception exception);
+  @ForOverride
+  protected abstract boolean tryHandleExceptionInternal(Exception exception);
 
   static final class ConsumerLookup extends Lookup {
     private final Consumer<SkyValue> sink;
@@ -84,12 +87,12 @@ boolean doLookup(LookupEnvironment env) throws InterruptedException {
     }
 
     @Override
-    void acceptValue(SkyValue value) {
+    protected void acceptValueInternal(SkyValue value) {
       sink.accept(value);
     }
 
     @Override
-    boolean tryHandleException(Exception unusedException) {
+    protected boolean tryHandleExceptionInternal(Exception unusedException) {
       return false;
     }
   }
@@ -120,20 +123,20 @@ boolean doLookup(LookupEnvironment env) throws InterruptedException {
         if (e instanceof InterruptedException) {
           throw (InterruptedException) e;
         }
-        if (!tryHandleException(e)) {
+        if (!tryHandleException(key, e)) {
           throw new IllegalArgumentException("Unexpected exception for " + key(), e);
         }
       }
       return true;
     }
 
     @Override
-    void acceptValue(SkyValue value) {
+    protected void acceptValueInternal(SkyValue value) {
       sink.acceptValueOrException(value, /* exception= */ null);
     }
 
     @Override
-    boolean tryHandleException(Exception exception) {
+    protected boolean tryHandleExceptionInternal(Exception exception) {
       if (exceptionClass.isInstance(exception)) {
         sink.acceptValueOrException(/* value= */ null, exceptionClass.cast(exception));
         return true;
@@ -172,20 +175,20 @@ boolean doLookup(LookupEnvironment env) throws InterruptedException {
         if (e instanceof InterruptedException) {
           throw (InterruptedException) e;
         }
-        if (!tryHandleException(e)) {
+        if (!tryHandleException(key, e)) {
           throw new IllegalArgumentException("Unexpected exception for " + key(), e);
         }
       }
       return true;
     }
 
     @Override
-    void acceptValue(SkyValue value) {
+    protected void acceptValueInternal(SkyValue value) {
       sink.acceptValueOrException2(value, /* e1= */ null, /* e2= */ null);
     }
 
     @Override
-    boolean tryHandleException(Exception exception) {
+    protected boolean tryHandleExceptionInternal(Exception exception) {
       if (exceptionClass1.isInstance(exception)) {
         sink.acceptValueOrException2(
             /* value= */ null, exceptionClass1.cast(exception), /* e2= */ null);
@@ -235,20 +238,20 @@ boolean doLookup(LookupEnvironment env) throws InterruptedException {
         if (e instanceof InterruptedException) {
           throw (InterruptedException) e;
         }
-        if (!tryHandleException(e)) {
+        if (!tryHandleException(key, e)) {
           throw new IllegalArgumentException("Unexpected exception for " + key(), e);
         }
       }
       return true;
     }
 
     @Override
-    void acceptValue(SkyValue value) {
+    protected void acceptValueInternal(SkyValue value) {
       sink.acceptValueOrException3(value, /* e1= */ null, /* e2= */ null, /* e3= */ null);
     }
 
     @Override
-    boolean tryHandleException(Exception exception) {
+    protected boolean tryHandleExceptionInternal(Exception exception) {
       if (exceptionClass1.isInstance(exception)) {
         sink.acceptValueOrException3(
             /* value= */ null, exceptionClass1.cast(exception), /* e2= */ null, /* e3= */ null);