From 9b0dcf5378699738acdc63f47b82d8dcddcc8e45 Mon Sep 17 00:00:00 2001
From: George Gensure <ggensure@uber.com>
Date: Wed, 27 Mar 2019 14:55:09 -0400
Subject: [PATCH] Wrap StatusRuntimeExceptions from GrpcRemoteCache

Exceptions that occur during remote interactions are expected to be
wrapped in IOException for observation by the RemoteSpawn{Runner,Cache}
layers.

Fixes #7856
---
 .../build/lib/remote/GrpcRemoteCache.java     | 23 +++++++++++++------
 .../devtools/build/lib/remote/Retrier.java    |  2 +-
 .../build/lib/remote/GrpcRemoteCacheTest.java |  9 +++-----
 3 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/src/main/java/com/google/devtools/build/lib/remote/GrpcRemoteCache.java b/src/main/java/com/google/devtools/build/lib/remote/GrpcRemoteCache.java
index 8e896bddeed439..78a83d4c1fb05d 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/GrpcRemoteCache.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/GrpcRemoteCache.java
@@ -160,7 +160,11 @@ public static boolean isRemoteCacheOptions(RemoteOptions options) {
   private ListenableFuture<FindMissingBlobsResponse> getMissingDigests(
       FindMissingBlobsRequest request) throws IOException, InterruptedException {
     Context ctx = Context.current();
-    return retrier.executeAsync(() -> ctx.call(() -> casFutureStub().findMissingBlobs(request)));
+    try {
+      return retrier.executeAsync(() -> ctx.call(() -> casFutureStub().findMissingBlobs(request)));
+    } catch (StatusRuntimeException e) {
+      throw new IOException(e);
+    }
   }
 
   private ImmutableSet<Digest> getMissingDigests(Iterable<Digest> digests)
@@ -274,6 +278,9 @@ public void onSuccess(Void result) {
 
           @Override
           public void onFailure(Throwable t) {
+            if (t instanceof StatusRuntimeException) {
+              t = new IOException(t);
+            }
             outerF.setException(t);
           }
         },
@@ -289,12 +296,14 @@ private ListenableFuture<Void> downloadBlob(
     Context ctx = Context.current();
     AtomicLong offset = new AtomicLong(0);
     ProgressiveBackoff progressiveBackoff = new ProgressiveBackoff(retrier::newBackoff);
-    return retrier.executeAsync(
-        () ->
-            ctx.call(
-                () ->
-                    requestRead(
-                        resourceName, offset, progressiveBackoff, digest, out, hashSupplier)));
+    return Futures.catchingAsync(
+        retrier.executeAsync(
+            () -> ctx.call(
+                () -> requestRead(resourceName, offset, progressiveBackoff, digest, out, hashSupplier)),
+            progressiveBackoff),
+        StatusRuntimeException.class,
+        (e) -> Futures.immediateFailedFuture(new IOException(e)),
+        MoreExecutors.directExecutor());
   }
 
   static class ProgressiveBackoff implements Backoff {
diff --git a/src/main/java/com/google/devtools/build/lib/remote/Retrier.java b/src/main/java/com/google/devtools/build/lib/remote/Retrier.java
index b3ccedb624cad3..949050d0e1919a 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/Retrier.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/Retrier.java
@@ -264,7 +264,7 @@ public <T> ListenableFuture<T> executeAsync(AsyncCallable<T> call) {
    * Executes an {@link AsyncCallable}, retrying execution in case of failure with the given
    * backoff.
    */
-  private <T> ListenableFuture<T> executeAsync(AsyncCallable<T> call, Backoff backoff) {
+  public <T> ListenableFuture<T> executeAsync(AsyncCallable<T> call, Backoff backoff) {
     try {
       return Futures.catchingAsync(
           call.call(),
diff --git a/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteCacheTest.java b/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteCacheTest.java
index 8676fb0a79fc08..70a0088a4c688d 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteCacheTest.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteCacheTest.java
@@ -1011,13 +1011,10 @@ public void read(ReadRequest request, StreamObserver<ReadResponse> responseObser
     boolean passedThroughDeadlineExceeded = false;
     try {
       getFromFuture(client.downloadBlob(digest));
-    } catch (RuntimeException e) {
+      fail("Should have thrown an exception.");
+    } catch (IOException e) {
       Status st = Status.fromThrowable(e);
-      if (st.getCode() != Status.Code.DEADLINE_EXCEEDED) {
-        throw e;
-      }
-      passedThroughDeadlineExceeded = true;
+      assertThat(st.getCode()).isEqualTo(Status.Code.DEADLINE_EXCEEDED);
     }
-    assertThat(passedThroughDeadlineExceeded).isTrue();
   }
 }