From 27830077e6cad6b8345d2dc8f0fc1470e49a005a Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Thu, 16 Nov 2017 08:48:59 -0800 Subject: [PATCH 1/2] auth: Treat IOExceptions as UNAVAILABLE Fixes #3267 --- .../GoogleAuthLibraryCallCredentials.java | 10 ++++++++- .../GoogleAuthLibraryCallCredentialsTest.java | 21 +++++++++++++++++-- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/auth/src/main/java/io/grpc/auth/GoogleAuthLibraryCallCredentials.java b/auth/src/main/java/io/grpc/auth/GoogleAuthLibraryCallCredentials.java index 0f4b3e964ba..470471443c3 100644 --- a/auth/src/main/java/io/grpc/auth/GoogleAuthLibraryCallCredentials.java +++ b/auth/src/main/java/io/grpc/auth/GoogleAuthLibraryCallCredentials.java @@ -27,6 +27,7 @@ import io.grpc.MethodDescriptor; import io.grpc.Status; import io.grpc.StatusException; +import java.io.IOException; import java.lang.reflect.Constructor; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; @@ -96,7 +97,14 @@ public void run() { // https://github.com/google/google-auth-library-java/issues/3 is resolved. // // Some implementations may return null here. - Map> metadata = creds.getRequestMetadata(uri); + Map> metadata; + try { + metadata = creds.getRequestMetadata(uri); + } catch (IOException e) { + // Since it's an I/O failure, let the call be retried with UNAVAILABLE. + applier.fail(Status.UNAVAILABLE.withCause(e)); + return; + } // Re-use the headers if getRequestMetadata() returns the same map. It may return a // different map based on the provided URI, i.e., for JWT. However, today it does not // cache JWT and so we won't bother tring to save its return value based on the URI. diff --git a/auth/src/test/java/io/grpc/auth/GoogleAuthLibraryCallCredentialsTest.java b/auth/src/test/java/io/grpc/auth/GoogleAuthLibraryCallCredentialsTest.java index 9522b6d2846..1bf595b1e12 100644 --- a/auth/src/test/java/io/grpc/auth/GoogleAuthLibraryCallCredentialsTest.java +++ b/auth/src/test/java/io/grpc/auth/GoogleAuthLibraryCallCredentialsTest.java @@ -171,8 +171,25 @@ public void invalidBase64() throws Exception { } @Test - public void credentialsThrows() throws Exception { - IOException exception = new IOException("Broken"); + public void credentialsThrowsIoException() throws Exception { + Exception exception = new IOException("Broken"); + when(credentials.getRequestMetadata(eq(expectedUri))).thenThrow(exception); + + GoogleAuthLibraryCallCredentials callCredentials = + new GoogleAuthLibraryCallCredentials(credentials); + callCredentials.applyRequestMetadata(method, attrs, executor, applier); + assertEquals(1, runPendingRunnables()); + + verify(credentials).getRequestMetadata(eq(expectedUri)); + verify(applier).fail(statusCaptor.capture()); + Status status = statusCaptor.getValue(); + assertEquals(Status.Code.UNAVAILABLE, status.getCode()); + assertEquals(exception, status.getCause()); + } + + @Test + public void credentialsThrowsRuntimeException() throws Exception { + Exception exception = new RuntimeException("Broken"); when(credentials.getRequestMetadata(eq(expectedUri))).thenThrow(exception); GoogleAuthLibraryCallCredentials callCredentials = From 8008b2cdccb30121e7684afb3894354b4dae07d9 Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Fri, 17 Nov 2017 08:05:05 -0800 Subject: [PATCH 2/2] include description in status --- .../io/grpc/auth/GoogleAuthLibraryCallCredentials.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/auth/src/main/java/io/grpc/auth/GoogleAuthLibraryCallCredentials.java b/auth/src/main/java/io/grpc/auth/GoogleAuthLibraryCallCredentials.java index 470471443c3..d8035d3ac3b 100644 --- a/auth/src/main/java/io/grpc/auth/GoogleAuthLibraryCallCredentials.java +++ b/auth/src/main/java/io/grpc/auth/GoogleAuthLibraryCallCredentials.java @@ -102,7 +102,9 @@ public void run() { metadata = creds.getRequestMetadata(uri); } catch (IOException e) { // Since it's an I/O failure, let the call be retried with UNAVAILABLE. - applier.fail(Status.UNAVAILABLE.withCause(e)); + applier.fail(Status.UNAVAILABLE + .withDescription("Credentials failed to obtain metadata") + .withCause(e)); return; } // Re-use the headers if getRequestMetadata() returns the same map. It may return a @@ -118,7 +120,9 @@ public void run() { } applier.apply(headers); } catch (Throwable e) { - applier.fail(Status.UNAUTHENTICATED.withCause(e)); + applier.fail(Status.UNAUTHENTICATED + .withDescription("Failed computing credential metadata") + .withCause(e)); } } });