In keep-going mode, don't fail-fast on the evaluation of an individual SkyFunction if it throws an exception but still has missing deps. Instead, pretend it didn't throw, and restart it when its known deps are all done, presumably to throw the same exception. This removes a basic source of non-determinism.

--
MOS_MIGRATED_REVID=127129202

1 files changed, 6 insertions, 1 deletions

diff --git a/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java b/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java
index e7674cd622..8cd502dd45 100644
--- a/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java
+++ b/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java
@@ -1017,8 +1017,13 @@ public final class ParallelEvaluator implements Evaluator {
       } catch (final SkyFunctionException builderException) {
         ReifiedSkyFunctionException reifiedBuilderException =
             new ReifiedSkyFunctionException(builderException, skyKey);
+        // In keep-going mode, we do not let SkyFunctions throw errors with missing deps -- we will
+        // restart them when their deps are done, so we can have a definitive error and definitive
+        // graph structure, thus avoiding non-determinism. It's completely reasonable for
+        // SkyFunctions to throw eagerly because they do not know if they are in keep-going mode.
         // Propagated transitive errors are treated the same as missing deps.
-        if (reifiedBuilderException.getRootCauseSkyKey().equals(skyKey)) {
+        if ((!keepGoing || !env.valuesMissing())
+            && reifiedBuilderException.getRootCauseSkyKey().equals(skyKey)) {
           boolean shouldFailFast = !keepGoing || builderException.isCatastrophic();
           if (shouldFailFast) {
             // After we commit this error to the graph but before the eval call completes with the