Propagate the error message when a credential helper fails. (bazelbui…

…ld#16030) Closes bazelbuild#16012. PiperOrigin-RevId: 464732834 Change-Id: If51ce914098ff17ffe23fa4614947e7f4a5088dc Co-authored-by: Tiago Quelhas <[email protected]>
EngFlow · Aug 3, 2022 · 6102d33 · 6102d33
1 parent 0109031
commit 6102d33
Show file tree

Hide file tree

Showing 5 changed files with 61 additions and 17 deletions.
diff --git a/...main/java/com/google/devtools/build/lib/authandtls/credentialhelper/CredentialHelper.java b/...main/java/com/google/devtools/build/lib/authandtls/credentialhelper/CredentialHelper.java
@@ -77,15 +77,15 @@ public GetCredentialsResponse getCredentials(CredentialHelperEnvironment environ
 
       process.waitFor();
       if (process.timedout()) {
-        throw new IOException(
+        throw new CredentialHelperException(
             String.format(
                 Locale.US,
                 "Failed to get credentials for '%s' from helper '%s': process timed out",
                 uri,
                 path));
       }
       if (process.exitValue() != 0) {
-        throw new IOException(
+        throw new CredentialHelperException(
             String.format(
                 Locale.US,
                 "Failed to get credentials for '%s' from helper '%s': process exited with code %d."
@@ -99,7 +99,7 @@ public GetCredentialsResponse getCredentials(CredentialHelperEnvironment environ
       try {
         GetCredentialsResponse response = GSON.fromJson(stdout, GetCredentialsResponse.class);
         if (response == null) {
-          throw new IOException(
+          throw new CredentialHelperException(
               String.format(
                   Locale.US,
                   "Failed to get credentials for '%s' from helper '%s': process exited without"
@@ -110,7 +110,7 @@ public GetCredentialsResponse getCredentials(CredentialHelperEnvironment environ
         }
         return response;
       } catch (JsonSyntaxException e) {
-        throw new IOException(
+        throw new CredentialHelperException(
             String.format(
                 Locale.US,
                 "Failed to get credentials for '%s' from helper '%s': error parsing output. stderr:"

diff --git a/.../com/google/devtools/build/lib/authandtls/credentialhelper/CredentialHelperException.java b/.../com/google/devtools/build/lib/authandtls/credentialhelper/CredentialHelperException.java
@@ -0,0 +1,28 @@
+// Copyright 2022 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.authandtls.credentialhelper;
+
+import java.io.IOException;
+
+/** An {@link Exception} thrown while invoking a credential helper. */
+public class CredentialHelperException extends IOException {
+  public CredentialHelperException(String message) {
+    super(message);
+  }
+
+  public CredentialHelperException(String message, Throwable cause) {
+    super(message, cause);
+  }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/remote/util/BUILD b/src/main/java/com/google/devtools/build/lib/remote/util/BUILD
@@ -19,6 +19,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/actions:execution_requirements",
         "//src/main/java/com/google/devtools/build/lib/analysis:blaze_version_info",
         "//src/main/java/com/google/devtools/build/lib/authandtls",
+        "//src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper",
         "//src/main/java/com/google/devtools/build/lib/cmdline",
         "//src/main/java/com/google/devtools/build/lib/remote:ExecutionStatusException",
         "//src/main/java/com/google/devtools/build/lib/remote/common",

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
@@ -39,6 +39,7 @@
 import com.google.devtools.build.lib.actions.SpawnResult;
 import com.google.devtools.build.lib.actions.Spawns;
 import com.google.devtools.build.lib.authandtls.CallCredentialsProvider;
+import com.google.devtools.build.lib.authandtls.credentialhelper.CredentialHelperException;
 import com.google.devtools.build.lib.remote.ExecutionStatusException;
 import com.google.devtools.build.lib.remote.common.CacheNotFoundException;
 import com.google.devtools.build.lib.remote.common.OutputDigestMismatchException;
@@ -367,6 +368,7 @@ private static String executionStatusExceptionErrorMessage(ExecutionStatusExcept
   public static String grpcAwareErrorMessage(IOException e) {
     io.grpc.Status errStatus = io.grpc.Status.fromThrowable(e);
     if (e.getCause() instanceof ExecutionStatusException) {
+      // Display error message returned by the remote service.
       try {
         return "Remote Execution Failure:\n"
             + executionStatusExceptionErrorMessage((ExecutionStatusException) e.getCause());
@@ -378,9 +380,20 @@ public static String grpcAwareErrorMessage(IOException 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());
+      // Display error message returned by the gRPC library, prefixed by the status code.
+      StringBuilder sb = new StringBuilder();
+      sb.append(errStatus.getCode().name());
+      sb.append(": ");
+      sb.append(errStatus.getDescription());
+      // If the error originated from a credential helper, print additional debugging information.
+      for (Throwable t = errStatus.getCause(); t != null; t = t.getCause()) {
+        if (t instanceof CredentialHelperException) {
+          sb.append(": ");
+          sb.append(t.getMessage());
+          break;
+        }
+      }
+      return sb.toString();
     }
     return e.getMessage();
   }

diff --git a/.../java/com/google/devtools/build/lib/authandtls/credentialhelper/CredentialHelperTest.java b/.../java/com/google/devtools/build/lib/authandtls/credentialhelper/CredentialHelperTest.java
@@ -28,7 +28,6 @@
 import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem;
 import com.google.devtools.build.runfiles.Runfiles;
-import java.io.IOException;
 import java.net.URI;
 import java.time.Duration;
 import org.junit.Test;
@@ -95,18 +94,20 @@ public void knownUriWithMultipleHeaders() throws Exception {
 
   @Test
   public void unknownUri() {
-    IOException ioException =
+    CredentialHelperException e =
         assertThrows(
-            IOException.class, () -> getCredentialsFromHelper("https://unknown.example.com"));
-    assertThat(ioException).hasMessageThat().contains("Unknown uri 'https://unknown.example.com'");
+            CredentialHelperException.class,
+            () -> getCredentialsFromHelper("https://unknown.example.com"));
+    assertThat(e).hasMessageThat().contains("Unknown uri 'https://unknown.example.com'");
   }
 
   @Test
   public void credentialHelperOutputsNothing() throws Exception {
-    IOException ioException =
+    CredentialHelperException e =
         assertThrows(
-            IOException.class, () -> getCredentialsFromHelper("https://printnothing.example.com"));
-    assertThat(ioException).hasMessageThat().contains("exited without output");
+            CredentialHelperException.class,
+            () -> getCredentialsFromHelper("https://printnothing.example.com"));
+    assertThat(e).hasMessageThat().contains("exited without output");
   }
 
   @Test
@@ -135,9 +136,10 @@ public void helperGetEnvironment() throws Exception {
 
   @Test
   public void helperTimeout() throws Exception {
-    IOException ioException =
+    CredentialHelperException e =
         assertThrows(
-            IOException.class, () -> getCredentialsFromHelper("https://timeout.example.com"));
-    assertThat(ioException).hasMessageThat().contains("process timed out");
+            CredentialHelperException.class,
+            () -> getCredentialsFromHelper("https://timeout.example.com"));
+    assertThat(e).hasMessageThat().contains("process timed out");
   }
 }