improve remote execution error messages
If the error originated in the gRPC library (it usually does) then
display the error as "STATUS CODE: error message" directly to the
user.
Before this change:
"java.io.IOException: io.grpc.StatusRuntimeException: UNAVAILABLE:
failed to dns resolve hostname"
After this change:
"UNAVAILABLE: failed to dns resolve hostname"
Closes #8599.
PiperOrigin-RevId: 254617861
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java
index b09163c..5d10cee 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java
@@ -39,6 +39,7 @@
import com.google.devtools.build.lib.remote.options.RemoteOutputsMode;
import com.google.devtools.build.lib.remote.util.DigestUtil;
import com.google.devtools.build.lib.remote.util.TracingMetadataUtils;
+import com.google.devtools.build.lib.remote.util.Utils;
import com.google.devtools.build.lib.runtime.BlazeModule;
import com.google.devtools.build.lib.runtime.BuildEventArtifactUploaderFactory;
import com.google.devtools.build.lib.runtime.Command;
@@ -228,7 +229,7 @@
capabilities = rsc.get(buildRequestId, invocationId);
} catch (IOException e) {
throw new AbruptExitException(
- "Failed to query remote execution capabilities: " + e.getMessage(),
+ "Failed to query remote execution capabilities: " + Utils.grpcAwareErrorMessage(e),
ExitCode.REMOTE_ERROR,
e);
} catch (InterruptedException e) {
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java
index 72ba687..e711b27 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java
@@ -51,6 +51,7 @@
import com.google.devtools.build.lib.remote.util.DigestUtil;
import com.google.devtools.build.lib.remote.util.DigestUtil.ActionKey;
import com.google.devtools.build.lib.remote.util.TracingMetadataUtils;
+import com.google.devtools.build.lib.remote.util.Utils;
import com.google.devtools.build.lib.remote.util.Utils.InMemoryOutput;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
@@ -194,7 +195,7 @@
} catch (CacheNotFoundException e) {
// Intentionally left blank
} catch (IOException e) {
- String errorMsg = e.getMessage();
+ String errorMsg = Utils.grpcAwareErrorMessage(e);
if (isNullOrEmpty(errorMsg)) {
errorMsg = e.getClass().getSimpleName();
}
@@ -247,7 +248,7 @@
remoteCache.upload(
actionKey, action, command, execRoot, files, context.getFileOutErr());
} catch (IOException e) {
- String errorMsg = e.getMessage();
+ String errorMsg = Utils.grpcAwareErrorMessage(e);
if (isNullOrEmpty(errorMsg)) {
errorMsg = e.getClass().getSimpleName();
}
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 cb6463a..2a09f3c 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
@@ -55,7 +55,6 @@
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.Reporter;
import com.google.devtools.build.lib.exec.ExecutionOptions;
-import com.google.devtools.build.lib.exec.SpawnExecException;
import com.google.devtools.build.lib.exec.SpawnRunner;
import com.google.devtools.build.lib.profiler.Profiler;
import com.google.devtools.build.lib.profiler.ProfilerTask;
@@ -66,6 +65,7 @@
import com.google.devtools.build.lib.remote.util.DigestUtil;
import com.google.devtools.build.lib.remote.util.DigestUtil.ActionKey;
import com.google.devtools.build.lib.remote.util.TracingMetadataUtils;
+import com.google.devtools.build.lib.remote.util.Utils;
import com.google.devtools.build.lib.remote.util.Utils.InMemoryOutput;
import com.google.devtools.build.lib.util.ExitCode;
import com.google.devtools.build.lib.util.io.FileOutErr;
@@ -439,17 +439,24 @@
} else {
status = Status.EXECUTION_FAILED;
}
- throw new SpawnExecException(
- verboseFailures ? Throwables.getStackTraceAsString(exception) : exception.getMessage(),
- new SpawnResult.Builder()
- .setRunnerName(getName())
- .setStatus(status)
- .setExitCode(ExitCode.REMOTE_ERROR.getNumericExitCode())
- .setFailureMessage(exception.getMessage())
- .build(),
- /* forciblyRunRemotely= */ false);
+
+ final String errorMessage;
+ if (!verboseFailures) {
+ errorMessage = Utils.grpcAwareErrorMessage(exception);
+ } else {
+ // On --verbose_failures print the whole stack trace
+ errorMessage = Throwables.getStackTraceAsString(exception);
+ }
+
+ return new SpawnResult.Builder()
+ .setRunnerName(getName())
+ .setStatus(status)
+ .setExitCode(ExitCode.REMOTE_ERROR.getNumericExitCode())
+ .setFailureMessage(errorMessage)
+ .build();
}
+
static Action buildAction(Digest command, Digest inputRoot, Duration timeout, boolean cacheable) {
Action.Builder action = Action.newBuilder();
diff --git a/src/main/java/com/google/devtools/build/lib/remote/util/Utils.java b/src/main/java/com/google/devtools/build/lib/remote/util/Utils.java
index 1ab04da..818a4da 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/util/Utils.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/util/Utils.java
@@ -107,6 +107,16 @@
return !Collections.disjoint(outputs, topLevelOutputs);
}
+ public static String grpcAwareErrorMessage(IOException e) {
+ io.grpc.Status errStatus = io.grpc.Status.fromThrowable(e);
+ if (!errStatus.getCode().equals(io.grpc.Status.UNKNOWN.getCode())) {
+ // If the error originated in the gRPC library then display it as "STATUS: error message"
+ // to the user
+ return String.format("%s: %s", errStatus.getCode().name(), errStatus.getDescription());
+ }
+ return e.getMessage();
+ }
+
/** An in-memory output file. */
public static final class InMemoryOutput {
private final ActionInput output;
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 40e7074..157c3a3 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
@@ -14,7 +14,6 @@
package com.google.devtools.build.lib.remote;
import static com.google.common.truth.Truth.assertThat;
-import static com.google.devtools.build.lib.testutil.MoreAsserts.assertThrows;
import static java.nio.charset.StandardCharsets.UTF_8;
import static org.junit.Assert.fail;
import static org.mockito.AdditionalAnswers.answerVoid;
@@ -50,7 +49,6 @@
import com.google.common.util.concurrent.MoreExecutors;
import com.google.devtools.build.lib.actions.ActionInput;
import com.google.devtools.build.lib.actions.ActionInputHelper;
-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.actions.SpawnResult;
@@ -59,7 +57,6 @@
import com.google.devtools.build.lib.authandtls.GoogleAuthUtils;
import com.google.devtools.build.lib.clock.JavaClock;
import com.google.devtools.build.lib.exec.ExecutionOptions;
-import com.google.devtools.build.lib.exec.SpawnExecException;
import com.google.devtools.build.lib.exec.util.FakeOwner;
import com.google.devtools.build.lib.remote.RemoteRetrier.ExponentialBackoff;
import com.google.devtools.build.lib.remote.options.RemoteOptions;
@@ -222,7 +219,7 @@
remoteOptions,
Options.getDefaults(ExecutionOptions.class),
null,
- true,
+ /* verboseFailures= */ true,
/*cmdlineReporter=*/ null,
"build-req-id",
"command-id",
@@ -857,13 +854,10 @@
FakeSpawnExecutionContext policy =
new FakeSpawnExecutionContext(simpleSpawn, fakeFileCache, execRoot, outErr);
- SpawnExecException expected =
- assertThrows(SpawnExecException.class, () -> client.exec(simpleSpawn, policy));
- assertThat(expected.getSpawnResult().status())
- .isEqualTo(SpawnResult.Status.EXECUTION_FAILED_CATASTROPHICALLY);
- // Ensure we also got back the stack trace.
- assertThat(expected)
- .hasMessageThat()
+ SpawnResult result = client.exec(simpleSpawn, policy);
+ assertThat(result.status()).isEqualTo(SpawnResult.Status.EXECUTION_FAILED_CATASTROPHICALLY);
+ // Ensure we also got back the stack trace due to verboseFailures=true
+ assertThat(result.getFailureMessage())
.contains("GrpcRemoteExecutionClientTest.passUnavailableErrorWithStackTrace");
}
@@ -880,12 +874,10 @@
FakeSpawnExecutionContext policy =
new FakeSpawnExecutionContext(simpleSpawn, fakeFileCache, execRoot, outErr);
- ExecException expected =
- assertThrows(ExecException.class, () -> client.exec(simpleSpawn, policy));
- assertThat(expected).hasMessageThat().contains("whoa"); // Error details.
+ SpawnResult result = client.exec(simpleSpawn, policy);
+ assertThat(result.getFailureMessage()).contains("whoa"); // Error details.
// Ensure we also got back the stack trace.
- assertThat(expected)
- .hasMessageThat()
+ assertThat(result.getFailureMessage())
.contains("GrpcRemoteExecutionClientTest.passInternalErrorWithStackTrace");
}
@@ -941,14 +933,11 @@
FakeSpawnExecutionContext policy =
new FakeSpawnExecutionContext(simpleSpawn, fakeFileCache, execRoot, outErr);
- SpawnExecException expected =
- assertThrows(SpawnExecException.class, () -> client.exec(simpleSpawn, policy));
- assertThat(expected.getSpawnResult().status())
- .isEqualTo(SpawnResult.Status.REMOTE_CACHE_FAILED);
- assertThat(expected).hasMessageThat().contains(DIGEST_UTIL.toString(stdOutDigest));
+ SpawnResult result = client.exec(simpleSpawn, policy);
+ assertThat(result.status()).isEqualTo(SpawnResult.Status.REMOTE_CACHE_FAILED);
+ assertThat(result.getFailureMessage()).contains(DIGEST_UTIL.toString(stdOutDigest));
// Ensure we also got back the stack trace.
- assertThat(expected)
- .hasMessageThat()
+ assertThat(result.getFailureMessage())
.contains("GrpcRemoteExecutionClientTest.passCacheMissErrorWithStackTrace");
}
@@ -1004,14 +993,11 @@
FakeSpawnExecutionContext policy =
new FakeSpawnExecutionContext(simpleSpawn, fakeFileCache, execRoot, outErr);
- SpawnExecException expected =
- assertThrows(SpawnExecException.class, () -> client.exec(simpleSpawn, policy));
- assertThat(expected.getSpawnResult().status())
- .isEqualTo(SpawnResult.Status.REMOTE_CACHE_FAILED);
- assertThat(expected).hasMessageThat().contains(DIGEST_UTIL.toString(stdOutDigest));
- // Ensure we also got back the stack trace.
- assertThat(expected)
- .hasMessageThat()
+ SpawnResult result = client.exec(simpleSpawn, policy);
+ assertThat(result.status()).isEqualTo(SpawnResult.Status.REMOTE_CACHE_FAILED);
+ assertThat(result.getFailureMessage()).contains(DIGEST_UTIL.toString(stdOutDigest));
+ // Ensure we also got back the stack trace because verboseFailures=true
+ assertThat(result.getFailureMessage())
.contains("passRepeatedOrphanedCacheMissErrorWithStackTrace");
}
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 5dd3713..567629a 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
@@ -61,7 +61,6 @@
import com.google.devtools.build.lib.events.Reporter;
import com.google.devtools.build.lib.events.StoredEventHandler;
import com.google.devtools.build.lib.exec.ExecutionOptions;
-import com.google.devtools.build.lib.exec.SpawnExecException;
import com.google.devtools.build.lib.exec.SpawnRunner;
import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionContext;
import com.google.devtools.build.lib.exec.util.FakeOwner;
@@ -739,9 +738,9 @@
SpawnExecutionContext policy =
new FakeSpawnExecutionContext(spawn, fakeFileCache, execRoot, outErr);
- SpawnExecException e = assertThrows(SpawnExecException.class, () -> runner.exec(spawn, policy));
- assertThat(e.getSpawnResult().exitCode()).isEqualTo(ExitCode.REMOTE_ERROR.getNumericExitCode());
- assertThat(e.getSpawnResult().getDetailMessage("", "", false, false)).contains("reasons");
+ SpawnResult result = runner.exec(spawn, policy);
+ assertThat(result.exitCode()).isEqualTo(ExitCode.REMOTE_ERROR.getNumericExitCode());
+ assertThat(result.getDetailMessage("", "", false, false)).contains("reasons");
}
@Test
@@ -759,9 +758,9 @@
SpawnExecutionContext policy =
new FakeSpawnExecutionContext(spawn, fakeFileCache, execRoot, outErr);
- SpawnExecException e = assertThrows(SpawnExecException.class, () -> runner.exec(spawn, policy));
- assertThat(e.getSpawnResult().exitCode()).isEqualTo(ExitCode.REMOTE_ERROR.getNumericExitCode());
- assertThat(e.getSpawnResult().getDetailMessage("", "", false, false)).contains("reasons");
+ SpawnResult result = runner.exec(spawn, policy);
+ assertThat(result.exitCode()).isEqualTo(ExitCode.REMOTE_ERROR.getNumericExitCode());
+ assertThat(result.getDetailMessage("", "", false, false)).contains("reasons");
}
@Test
@@ -910,8 +909,8 @@
new FakeSpawnExecutionContext(spawn, fakeFileCache, execRoot, outErr);
// act
- SpawnExecException e = assertThrows(SpawnExecException.class, () -> runner.exec(spawn, policy));
- assertThat(e.getMessage()).isEqualTo(downloadFailure.getMessage());
+ SpawnResult result = runner.exec(spawn, policy);
+ assertThat(result.getFailureMessage()).isEqualTo(downloadFailure.getMessage());
// assert
verify(cache)
diff --git a/src/test/java/com/google/devtools/build/lib/remote/UtilsTest.java b/src/test/java/com/google/devtools/build/lib/remote/UtilsTest.java
new file mode 100644
index 0000000..ad1b4c0
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/remote/UtilsTest.java
@@ -0,0 +1,39 @@
+// Copyright 2019 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.remote;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.devtools.build.lib.remote.util.Utils;
+import io.grpc.Status;
+import java.io.IOException;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/** Tests for remote utility methods */
+@RunWith(JUnit4.class)
+public class UtilsTest {
+
+ @Test
+ public void testGrpcAwareErrorMessages() {
+ IOException ioError = new IOException("io error");
+ IOException wrappedGrpcError =
+ new IOException(
+ "wrapped error", Status.ABORTED.withDescription("grpc error").asRuntimeException());
+
+ assertThat(Utils.grpcAwareErrorMessage(ioError)).isEqualTo("io error");
+ assertThat(Utils.grpcAwareErrorMessage(wrappedGrpcError)).isEqualTo("ABORTED: grpc error");
+ }
+}
