remote: Return exit code 34 for remote caching/execution errors.

For any errors that are due to failures in the remote caching /
execution layers Bazel now returns exit code 34 (ExitCode.REMOTE_ERROR).
This includes errors where the remote cache / executor is unreachable or
crashes. It does not include errors if the test / build failure is due
to user errors i.e. compilation or test failures.

PiperOrigin-RevId: 167259236

3 files changed, 86 insertions, 71 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/BUILD b/src/test/java/com/google/devtools/build/lib/BUILD
index ba7d4e318a..a457eac130 100644
--- a/src/test/java/com/google/devtools/build/lib/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/BUILD
@@ -1082,6 +1082,7 @@ java_test(
         "//src/main/java/com/google/devtools/build/lib:auth_and_tls_options",
         "//src/main/java/com/google/devtools/build/lib:build-base",
         "//src/main/java/com/google/devtools/build/lib:events",
+        "//src/main/java/com/google/devtools/build/lib:exitcode-external",
         "//src/main/java/com/google/devtools/build/lib:inmemoryfs",
         "//src/main/java/com/google/devtools/build/lib:io",
         "//src/main/java/com/google/devtools/build/lib:preconditions",
diff --git a/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteExecutionClientTest.java b/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteExecutionClientTest.java
index 1db31cc372..c5d5b66f62 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteExecutionClientTest.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteExecutionClientTest.java
@@ -31,11 +31,12 @@ import com.google.devtools.build.lib.actions.ActionInputFileCache;
 import com.google.devtools.build.lib.actions.ActionInputHelper;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander;
-import com.google.devtools.build.lib.actions.EnvironmentalExecException;
+import com.google.devtools.build.lib.actions.ExecException;
 import com.google.devtools.build.lib.actions.ResourceSet;
 import com.google.devtools.build.lib.actions.SimpleSpawn;
 import com.google.devtools.build.lib.authandtls.AuthAndTLSOptions;
 import com.google.devtools.build.lib.authandtls.GrpcUtils;
+import com.google.devtools.build.lib.exec.SpawnExecException;
 import com.google.devtools.build.lib.exec.SpawnInputExpander;
 import com.google.devtools.build.lib.exec.SpawnResult;
 import com.google.devtools.build.lib.exec.SpawnRunner.ProgressStatus;
@@ -636,13 +637,12 @@ public class GrpcRemoteExecutionClientTest {
     try {
       client.exec(simpleSpawn, simplePolicy);
       fail("Expected an exception");
-    } catch (EnvironmentalExecException expected) {
-      assertThat(expected).hasMessageThat().contains("The remote executor/cache is unavailable");
+    } catch (SpawnExecException expected) {
+      assertThat(expected.getSpawnResult().status())
+          .isEqualTo(SpawnResult.Status.CONNECTION_FAILED);
       // Ensure we also got back the stack trace.
       assertThat(expected).hasMessageThat()
           .contains("GrpcRemoteExecutionClientTest.passUnavailableErrorWithStackTrace");
-      Throwable t = expected.getCause();
-      assertThat(t).isInstanceOf(RetryException.class);
     }
   }
 
@@ -660,14 +660,11 @@ public class GrpcRemoteExecutionClientTest {
     try {
       client.exec(simpleSpawn, simplePolicy);
       fail("Expected an exception");
-    } catch (EnvironmentalExecException expected) {
-      assertThat(expected).hasMessageThat().contains("Error in remote cache/executor");
+    } catch (ExecException expected) {
       assertThat(expected).hasMessageThat().contains("whoa"); // Error details.
       // Ensure we also got back the stack trace.
       assertThat(expected).hasMessageThat()
           .contains("GrpcRemoteExecutionClientTest.passInternalErrorWithStackTrace");
-      Throwable t = expected.getCause();
-      assertThat(t).isInstanceOf(RetryException.class);
     }
   }
 
@@ -719,14 +716,13 @@ public class GrpcRemoteExecutionClientTest {
     try {
       client.exec(simpleSpawn, simplePolicy);
       fail("Expected an exception");
-    } catch (EnvironmentalExecException expected) {
-      assertThat(expected).hasMessageThat().contains("Failed to download from remote cache");
+    } catch (SpawnExecException expected) {
+      assertThat(expected.getSpawnResult().status())
+          .isEqualTo(SpawnResult.Status.REMOTE_CACHE_FAILED);
+      assertThat(expected).hasMessageThat().contains(stdOutDigest.getHash());
       // Ensure we also got back the stack trace.
       assertThat(expected).hasMessageThat()
           .contains("GrpcRemoteExecutionClientTest.passCacheMissErrorWithStackTrace");
-      Throwable t = expected.getCause();
-      assertThat(t).isInstanceOf(CacheNotFoundException.class);
-      assertThat(((CacheNotFoundException) t).getMissingDigest()).isEqualTo(stdOutDigest);
     }
   }
 
@@ -800,11 +796,9 @@ public class GrpcRemoteExecutionClientTest {
     try {
       client.exec(simpleSpawn, simplePolicy);
       fail("Expected an exception");
-    } catch (EnvironmentalExecException expected) {
-      assertThat(expected).hasMessageThat().contains("Failed to download from remote cache");
-      Throwable t = expected.getCause();
-      assertThat(t).isInstanceOf(CacheNotFoundException.class);
-      assertThat(((CacheNotFoundException) t).getMissingDigest()).isEqualTo(stdOutDigest);
+    } catch (ExecException expected) {
+      assertThat(expected).hasMessageThat().contains("Missing digest");
+      assertThat(expected).hasMessageThat().contains("476d9ec701e2de6a6c37ab5211117a7cb8333a27");
     }
   }
 }
diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java
index ea1ac727df..a6c8218f01 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java
@@ -40,6 +40,7 @@ import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.EventKind;
 import com.google.devtools.build.lib.events.Reporter;
 import com.google.devtools.build.lib.events.StoredEventHandler;
+import com.google.devtools.build.lib.exec.SpawnExecException;
 import com.google.devtools.build.lib.exec.SpawnInputExpander;
 import com.google.devtools.build.lib.exec.SpawnResult;
 import com.google.devtools.build.lib.exec.SpawnResult.Status;
@@ -48,6 +49,7 @@ import com.google.devtools.build.lib.exec.SpawnRunner.ProgressStatus;
 import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionPolicy;
 import com.google.devtools.build.lib.exec.util.FakeOwner;
 import com.google.devtools.build.lib.remote.Digests.ActionKey;
+import com.google.devtools.build.lib.util.ExitCode;
 import com.google.devtools.build.lib.util.io.FileOutErr;
 import com.google.devtools.build.lib.vfs.FileSystem;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
@@ -202,14 +204,7 @@ public class RemoteSpawnRunnerTest {
         spy(new RemoteSpawnRunner(execRoot, options, localRunner, true, /*cmdlineReporter=*/null,
             cache, null));
 
-    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);
+    Spawn spawn = newSimpleSpawn();
     SpawnExecutionPolicy policy = new FakeSpawnExecutionPolicy(spawn);
 
     SpawnResult res = Mockito.mock(SpawnResult.class);
@@ -235,14 +230,7 @@ public class RemoteSpawnRunnerTest {
     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);
+    Spawn spawn = newSimpleSpawn();
     SpawnExecutionPolicy policy = new FakeSpawnExecutionPolicy(spawn);
 
     RemoteSpawnRunner runner =
@@ -273,15 +261,7 @@ public class RemoteSpawnRunnerTest {
     RemoteSpawnRunner runner =
         new RemoteSpawnRunner(execRoot, options, localRunner, false, reporter, cache, null);
 
-    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);
+    Spawn spawn = newSimpleSpawn();
     SpawnExecutionPolicy policy = new FakeSpawnExecutionPolicy(spawn);
 
     when(cache.getCachedActionResult(any(ActionKey.class)))
@@ -318,15 +298,7 @@ public class RemoteSpawnRunnerTest {
         new RemoteSpawnRunner(execRoot, options, localRunner, true, /*cmdlineReporter=*/null,
             cache, null);
 
-    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);
+    Spawn spawn = newSimpleSpawn();
     SpawnExecutionPolicy policy = new FakeSpawnExecutionPolicy(spawn);
 
     when(cache.getCachedActionResult(any(ActionKey.class))).thenReturn(null);
@@ -364,15 +336,7 @@ public class RemoteSpawnRunnerTest {
     when(executor.executeRemotely(any(ExecuteRequest.class))).thenReturn(succeeded);
     doNothing().when(cache).download(eq(execResult), any(Path.class), any(FileOutErr.class));
 
-    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);
+    Spawn spawn = newSimpleSpawn();
 
     SpawnExecutionPolicy policy = new FakeSpawnExecutionPolicy(spawn);
 
@@ -398,15 +362,7 @@ public class RemoteSpawnRunnerTest {
     when(cache.getCachedActionResult(any(ActionKey.class))).thenReturn(null);
     when(executor.executeRemotely(any(ExecuteRequest.class))).thenThrow(new TimeoutException());
 
-    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);
+    Spawn spawn = newSimpleSpawn();
 
     SpawnExecutionPolicy policy = new FakeSpawnExecutionPolicy(spawn);
 
@@ -417,6 +373,70 @@ public class RemoteSpawnRunnerTest {
     verify(cache, never()).download(eq(cachedResult), eq(execRoot), any(FileOutErr.class));
   }
 
+  @Test
+  public void testExitCode_executorfailure() throws Exception {
+    // If we get a failure due to the remote cache not working, the exit code should be
+    // ExitCode.REMOTE_ERROR.
+
+    RemoteOptions options = Options.getDefaults(RemoteOptions.class);
+    options.remoteLocalFallback = false;
+
+    RemoteSpawnRunner runner =
+        new RemoteSpawnRunner(execRoot, options, localRunner, true, /*cmdlineReporter=*/null,
+            cache, executor);
+
+    when(cache.getCachedActionResult(any(ActionKey.class))).thenReturn(null);
+    when(executor.executeRemotely(any(ExecuteRequest.class))).thenThrow(new IOException());
+
+    Spawn spawn = newSimpleSpawn();
+    SpawnExecutionPolicy policy = new FakeSpawnExecutionPolicy(spawn);
+
+    try {
+      runner.exec(spawn, policy);
+      fail("Exception expected");
+    } catch (SpawnExecException e) {
+      assertThat(e.getSpawnResult().exitCode())
+          .isEqualTo(ExitCode.REMOTE_ERROR.getNumericExitCode());
+    }
+  }
+
+  @Test
+  public void testExitCode_executionfailure() throws Exception {
+    // If we get a failure due to the remote executor not working, the exit code should be
+    // ExitCode.REMOTE_ERROR.
+
+    RemoteOptions options = Options.getDefaults(RemoteOptions.class);
+    options.remoteLocalFallback = false;
+
+    RemoteSpawnRunner runner =
+        new RemoteSpawnRunner(execRoot, options, localRunner, true, /*cmdlineReporter=*/null,
+            cache, executor);
+
+    when(cache.getCachedActionResult(any(ActionKey.class))).thenThrow(new IOException());
+
+    Spawn spawn = newSimpleSpawn();
+    SpawnExecutionPolicy policy = new FakeSpawnExecutionPolicy(spawn);
+
+    try {
+      runner.exec(spawn, policy);
+      fail("Exception expected");
+    } catch (SpawnExecException e) {
+      assertThat(e.getSpawnResult().exitCode())
+          .isEqualTo(ExitCode.REMOTE_ERROR.getNumericExitCode());
+    }
+  }
+
+  private static Spawn newSimpleSpawn() {
+    return new SimpleSpawn(
+        new FakeOwner("foo", "bar"),
+            /*arguments=*/ ImmutableList.of(),
+            /*environment=*/ ImmutableMap.of(),
+            /*executionInfo=*/ ImmutableMap.of(),
+            /*inputs=*/ ImmutableList.of(),
+            /*outputs=*/ ImmutableList.<ActionInput>of(),
+        ResourceSet.ZERO);
+  }
+
   // TODO(buchgr): Extract a common class to be used for testing.
   class FakeSpawnExecutionPolicy implements SpawnExecutionPolicy {