From 6102d33bf0b72dc0fe9ada4c71113cbee3eb8187 Mon Sep 17 00:00:00 2001 From: kshyanashree <109167932+kshyanashree@users.noreply.github.com> Date: Tue, 2 Aug 2022 20:52:56 -0700 Subject: [PATCH] Propagate the error message when a credential helper fails. (#16030) Closes #16012. PiperOrigin-RevId: 464732834 Change-Id: If51ce914098ff17ffe23fa4614947e7f4a5088dc Co-authored-by: Tiago Quelhas --- .../credentialhelper/CredentialHelper.java | 8 +++--- .../CredentialHelperException.java | 28 +++++++++++++++++++ .../devtools/build/lib/remote/util/BUILD | 1 + .../devtools/build/lib/remote/util/Utils.java | 19 +++++++++++-- .../CredentialHelperTest.java | 22 ++++++++------- 5 files changed, 61 insertions(+), 17 deletions(-) create mode 100644 src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper/CredentialHelperException.java diff --git a/src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper/CredentialHelper.java b/src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper/CredentialHelper.java index e410c0b52bc771..db9b43b1aef925 100644 --- a/src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper/CredentialHelper.java +++ b/src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper/CredentialHelper.java @@ -77,7 +77,7 @@ 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", @@ -85,7 +85,7 @@ public GetCredentialsResponse getCredentials(CredentialHelperEnvironment environ 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/src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper/CredentialHelperException.java b/src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper/CredentialHelperException.java new file mode 100644 index 00000000000000..7d8718f0edfd7f --- /dev/null +++ b/src/main/java/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 index 9afc158cfbad38..13d4514d68b76a 100644 --- 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 index 8c4099a5931bd6..c7750404352206 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 @@ -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/src/test/java/com/google/devtools/build/lib/authandtls/credentialhelper/CredentialHelperTest.java b/src/test/java/com/google/devtools/build/lib/authandtls/credentialhelper/CredentialHelperTest.java index 4aeb4595b084d0..0db1127fb55c3c 100644 --- a/src/test/java/com/google/devtools/build/lib/authandtls/credentialhelper/CredentialHelperTest.java +++ b/src/test/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"); } }