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 Closes #7860. PiperOrigin-RevId: 240793745
bazelbuild · Mar 28, 2019 · 80bff99 · 80bff99
1 parent 7e3cfd8
commit 80bff99
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 15 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
@@ -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,17 @@ 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
@@ -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
@@ -1008,16 +1008,12 @@ public void read(ReadRequest request, StreamObserver<ReadResponse> responseObser
             responseObserver.onError(Status.DEADLINE_EXCEEDED.asException());
           }
         });
-    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();
   }
 }