Skip to content

Commit

Permalink
Propagate the error message when a credential helper fails.
Browse files Browse the repository at this point in the history
Closes #16012.

PiperOrigin-RevId: 464732834
Change-Id: If51ce914098ff17ffe23fa4614947e7f4a5088dc
  • Loading branch information
tjgq authored and copybara-github committed Aug 2, 2022
1 parent 591ecbe commit 24c16fc
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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."
Expand All @@ -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"
Expand All @@ -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:"
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
19 changes: 16 additions & 3 deletions src/main/java/com/google/devtools/build/lib/remote/util/Utils.java
Original file line number Diff line number Diff line change
Expand Up @@ -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.BulkTransferException;
import com.google.devtools.build.lib.remote.common.CacheNotFoundException;
Expand Down Expand Up @@ -387,6 +388,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());
Expand All @@ -398,9 +400,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();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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");
}
}

0 comments on commit 24c16fc

Please sign in to comment.