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
diff --git a/WORKSPACE b/WORKSPACE
index 0f5e2a6..de582a7 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -74,14 +74,14 @@
new_local_repository(
name = "com_google_protobuf",
- build_file = "./third_party/protobuf/3.4.0/BUILD",
- path = "./third_party/protobuf/3.4.0/",
+ build_file = "./third_party/protobuf/3.2.0/BUILD",
+ path = "./third_party/protobuf/3.2.0/",
)
new_local_repository(
name = "com_google_protobuf_java",
- build_file = "./third_party/protobuf/3.4.0/com_google_protobuf_java.BUILD",
- path = "./third_party/protobuf/3.4.0/",
+ build_file = "./third_party/protobuf/3.2.0/com_google_protobuf_java.BUILD",
+ path = "./third_party/protobuf/3.2.0/",
)
new_local_repository(
diff --git a/src/main/java/com/google/devtools/build/lib/exec/SpawnResult.java b/src/main/java/com/google/devtools/build/lib/exec/SpawnResult.java
index b3a0383..d2d3ee8 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/SpawnResult.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/SpawnResult.java
@@ -94,6 +94,12 @@
REMOTE_EXECUTOR_OVERLOADED,
/**
+ * The result of the remotely executed Spawn could not be retrieved due to errors in the remote
+ * caching layer.
+ */
+ REMOTE_CACHE_FAILED,
+
+ /**
* The remote execution system did not allow the request due to missing authorization or
* authentication.
*/
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java
index 69b751c..bd53704 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java
@@ -27,12 +27,14 @@
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.Reporter;
+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;
import com.google.devtools.build.lib.exec.SpawnRunner;
import com.google.devtools.build.lib.remote.Digests.ActionKey;
import com.google.devtools.build.lib.remote.TreeNodeRepository.TreeNode;
+import com.google.devtools.build.lib.util.ExitCode;
import com.google.devtools.build.lib.util.io.FileOutErr;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
@@ -209,40 +211,40 @@
if (options.remoteLocalFallback) {
return execLocally(spawn, policy, inputMap, uploadLocalResults, remoteCache, actionKey);
}
+ return handleError(cause, policy.getFileOutErr());
+ }
+
+ private SpawnResult handleError(IOException cause, FileOutErr outErr) throws IOException,
+ ExecException {
+ final Status status;
if (cause instanceof TimeoutException) {
- return handleTimeout(policy.getFileOutErr());
- }
- throw new EnvironmentalExecException(errorMessage(cause), cause, true);
- }
-
- private SpawnResult handleTimeout(FileOutErr outErr) throws IOException {
- // TODO(buchgr): Once the remote execution protocol allows it, also provide stdout/stderr
- // from the action that timed out.
- try (OutputStream out = outErr.getOutputStream()) {
- // This is a hack to ensure that the test.log file gets created, as else the action
- // will complain that one of its outputs has not been created.
- String msg = "Log output for timeouts is not yet supported in remote execution.";
- out.write(msg.getBytes(StandardCharsets.UTF_8));
- out.flush();
- }
- return new SpawnResult.Builder().setStatus(Status.TIMEOUT).build();
- }
-
- private String errorMessage(IOException e) {
- String message = "";
- if (e instanceof RetryException
- && ((RetryException) e).causedByStatusCode(Code.UNAVAILABLE)) {
- message = "The remote executor/cache is unavailable";
- } else if (e instanceof CacheNotFoundException) {
- message = "Failed to download from remote cache";
+ status = Status.TIMEOUT;
+ // TODO(buchgr): Once the remote execution protocol allows it, also provide stdout/stderr
+ // from the action that timed out.
+ try (OutputStream out = outErr.getOutputStream()) {
+ // This is a hack to ensure that the test.log file gets created, as else the action
+ // will complain that one of its outputs has not been created.
+ String msg = "Log output for timeouts is not yet supported in remote execution.";
+ out.write(msg.getBytes(StandardCharsets.UTF_8));
+ out.flush();
+ }
+ return new SpawnResult.Builder().setStatus(status).build();
+ } else if (cause instanceof RetryException
+ && ((RetryException) cause).causedByStatusCode(Code.UNAVAILABLE)) {
+ status = Status.CONNECTION_FAILED;
+ } else if (cause instanceof CacheNotFoundException) {
+ status = Status.REMOTE_CACHE_FAILED;
} else {
- message = "Error in remote cache/executor";
+ status = Status.EXECUTION_FAILED;
}
- // TODO(olaola): reuse the ErrorMessage class for these errors.
- if (verboseFailures) {
- message += "\n" + Throwables.getStackTraceAsString(e);
- }
- return message;
+ throw new SpawnExecException(
+ Throwables.getStackTraceAsString(cause),
+ new SpawnResult
+ .Builder()
+ .setExitCode(ExitCode.REMOTE_ERROR.getNumericExitCode())
+ .setStatus(status)
+ .build(),
+ /*catastrophic=*/true);
}
static Action buildAction(
diff --git a/src/main/protobuf/BUILD b/src/main/protobuf/BUILD
index 11d019c..afbfc5b 100644
--- a/src/main/protobuf/BUILD
+++ b/src/main/protobuf/BUILD
@@ -2,7 +2,7 @@
load("//tools/build_rules:genproto.bzl", "cc_grpc_library")
load("//tools/build_rules:utilities.bzl", "java_library_srcs")
-load("//third_party/protobuf/3.4.0:protobuf.bzl", "cc_proto_library", "py_proto_library")
+load("//third_party/protobuf/3.2.0:protobuf.bzl", "cc_proto_library", "py_proto_library")
load("//third_party/grpc:build_defs.bzl", "java_grpc_library")
FILES = [
diff --git a/src/test/java/com/google/devtools/build/lib/BUILD b/src/test/java/com/google/devtools/build/lib/BUILD
index ba7d4e3..a457eac 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 @@
"//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 1db31cc..c5d5b66 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.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 @@
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 @@
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 @@
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 @@
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 ea1ac72..a6c8218 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.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.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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 {
diff --git a/src/test/shell/bazel/remote_execution_test.sh b/src/test/shell/bazel/remote_execution_test.sh
index 5346377..5dbbc90 100755
--- a/src/test/shell/bazel/remote_execution_test.sh
+++ b/src/test/shell/bazel/remote_execution_test.sh
@@ -380,6 +380,25 @@
expect_log "TIMEOUT"
}
+function test_exitcode() {
+ mkdir -p a
+ cat > a/BUILD <<'EOF'
+genrule(
+ name = "foo",
+ srcs = [],
+ outs = ["foo.txt"],
+ cmd = "echo \"hello world\" > \"$@\"",
+)
+EOF
+
+ (set +e
+ bazel --host_jvm_args=-Dbazel.DigestFunction=SHA1 build \
+ --genrule_strategy=remote \
+ --remote_executor=bazel-test-does-not-exist \
+ //a:foo >& $TEST_log
+ [ $? -eq 34 ]) || fail "Test failed due to wrong exit code"
+}
+
# TODO(alpha): Add a test that fails remote execution when remote worker
# supports sandbox.
