remote: Provide a clear error message if the remote cache is in an in…

…valid state.

A remote cache must never serve a failed action. However, if it did
Bazel would not detect this and simply fail and display an error message
that's hard to distinquish from a local execution failure.

Bazel now displays a clear error message stating what went wrong.

RELNOTES: None.
PiperOrigin-RevId: 164975631

src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java

@@ -118,6 +118,11 @@ public SpawnResult exec(Spawn spawn, SpawnExecutionPolicy policy)
               ? remoteCache.getCachedActionResult(actionKey)
               : null;
       if (cachedResult != null) {
+        if (cachedResult.getExitCode() != 0) {
+          // The remote cache must never serve a failed action.
+          throw new EnvironmentalExecException("The remote cache is in an invalid state as it"
+              + " served a failed action. Hash of the action: " + actionKey.getDigest());
+        }
         try {
           return downloadRemoteResults(cachedResult, policy.getFileOutErr());
         } catch (CacheNotFoundException e) {

src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java

@@ -14,6 +14,7 @@
 package com.google.devtools.build.lib.remote;
 
 import static com.google.common.truth.Truth.assertThat;
+import static org.junit.Assert.fail;
 import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.never;
@@ -27,6 +28,7 @@
 import com.google.devtools.build.lib.actions.ActionInput;
 import com.google.devtools.build.lib.actions.ActionInputFileCache;
 import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander;
+import com.google.devtools.build.lib.actions.EnvironmentalExecException;
 import com.google.devtools.build.lib.actions.ExecutionRequirements;
 import com.google.devtools.build.lib.actions.ResourceSet;
 import com.google.devtools.build.lib.actions.SimpleSpawn;
@@ -214,6 +216,36 @@ public void failedActionShouldNotBeUploaded() throws Exception {
         any(FileOutErr.class));
   }
 
+  @Test
+  public void dontAcceptFailedCachedAction() throws Exception {
+    // Test that bazel fails if the remote cache serves a failed action.
+
+    RemoteOptions options = Options.getDefaults(RemoteOptions.class);
+
+    ActionResult failedAction = ActionResult.newBuilder().setExitCode(1).build();
+    when(cache.getCachedActionResult(any(ActionKey.class))).thenReturn(failedAction);
+
+    Spawn spawn = new SimpleSpawn(
+        new FakeOwner("foo", "bar"),
+        /*arguments=*/ ImmutableList.of(),
+        /*environment=*/ ImmutableMap.of(),
+        /*executionInfo=*/ ImmutableMap.of(),
+        /*inputs=*/ ImmutableList.of(),
+        /*outputs=*/ ImmutableList.<ActionInput>of(),
+        ResourceSet.ZERO);
+    SpawnExecutionPolicy policy = new FakeSpawnExecutionPolicy(spawn);
+
+    RemoteSpawnRunner runner =
+        spy(new RemoteSpawnRunner(execRoot, options, localRunner, true, cache, null));
+
+    try {
+      runner.exec(spawn, policy);
+      fail("Expected exception");
+    } catch (EnvironmentalExecException expected) {
+      // Intentionally left empty.
+    }
+  }
+
   // TODO(buchgr): Extract a common class to be used for testing.
   class FakeSpawnExecutionPolicy implements SpawnExecutionPolicy {