From 5599f299018cb363d600d4e39e35d2657b74f5bc Mon Sep 17 00:00:00 2001
From: Dmitry <58846611+dmitry-fa@users.noreply.github.com>
Date: Thu, 2 Apr 2020 10:59:22 +0300
Subject: [PATCH] fix: Blob.downloadTo() methods do not wrap
 RetryHelper$RetryHelperException (#218)

fix: wrap exception
---
 .../java/com/google/cloud/storage/Blob.java   | 35 ++++++++------
 .../com/google/cloud/storage/BlobTest.java    | 46 +++++++++++++------
 2 files changed, 53 insertions(+), 28 deletions(-)

diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/Blob.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/Blob.java
index 27e626264..7efc24c96 100644
--- a/google-cloud-storage/src/main/java/com/google/cloud/storage/Blob.java
+++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/Blob.java
@@ -26,6 +26,7 @@
 import com.google.auth.ServiceAccountSigner;
 import com.google.auth.ServiceAccountSigner.SigningException;
 import com.google.cloud.ReadChannel;
+import com.google.cloud.RetryHelper;
 import com.google.cloud.Tuple;
 import com.google.cloud.WriteChannel;
 import com.google.cloud.storage.Acl.Entity;
@@ -230,21 +231,25 @@ public void downloadTo(OutputStream outputStream, BlobSourceOption... options) {
     final CountingOutputStream countingOutputStream = new CountingOutputStream(outputStream);
     final StorageRpc storageRpc = this.options.getStorageRpcV1();
     final Map<StorageRpc.Option, ?> requestOptions = StorageImpl.optionMap(getBlobId(), options);
-    runWithRetries(
-        callable(
-            new Runnable() {
-              @Override
-              public void run() {
-                storageRpc.read(
-                    getBlobId().toPb(),
-                    requestOptions,
-                    countingOutputStream.getCount(),
-                    countingOutputStream);
-              }
-            }),
-        this.options.getRetrySettings(),
-        StorageImpl.EXCEPTION_HANDLER,
-        this.options.getClock());
+    try {
+      runWithRetries(
+          callable(
+              new Runnable() {
+                @Override
+                public void run() {
+                  storageRpc.read(
+                      getBlobId().toPb(),
+                      requestOptions,
+                      countingOutputStream.getCount(),
+                      countingOutputStream);
+                }
+              }),
+          this.options.getRetrySettings(),
+          StorageImpl.EXCEPTION_HANDLER,
+          this.options.getClock());
+    } catch (RetryHelper.RetryHelperException e) {
+      StorageException.translateAndThrow(e);
+    }
   }
 
   /**
diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/BlobTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/BlobTest.java
index 6f91f968a..51d606c2b 100644
--- a/google-cloud-storage/src/test/java/com/google/cloud/storage/BlobTest.java
+++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/BlobTest.java
@@ -32,6 +32,7 @@
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 import com.google.api.core.ApiClock;
 import com.google.api.gax.retrying.RetrySettings;
@@ -585,17 +586,22 @@ public void testBuilder() {
     assertTrue(blob.isDirectory());
   }
 
-  @Test
-  public void testDownload() throws Exception {
-    final byte[] expected = {1, 2};
+  private StorageRpc prepareForDownload() {
     StorageRpc mockStorageRpc = createNiceMock(StorageRpc.class);
-    expect(storage.getOptions()).andReturn(mockOptions).times(1);
+    expect(storage.getOptions()).andReturn(mockOptions);
     replay(storage);
     expect(mockOptions.getStorageRpcV1()).andReturn(mockStorageRpc);
     expect(mockOptions.getRetrySettings()).andReturn(RETRY_SETTINGS);
     expect(mockOptions.getClock()).andReturn(API_CLOCK);
     replay(mockOptions);
     blob = new Blob(storage, new BlobInfo.BuilderImpl(BLOB_INFO));
+    return mockStorageRpc;
+  }
+
+  @Test
+  public void testDownloadTo() throws Exception {
+    final byte[] expected = {1, 2};
+    StorageRpc mockStorageRpc = prepareForDownload();
     expect(
             mockStorageRpc.read(
                 anyObject(StorageObject.class),
@@ -618,16 +624,9 @@ public Long answer() throws Throwable {
   }
 
   @Test
-  public void testDownloadWithRetries() throws Exception {
+  public void testDownloadToWithRetries() throws Exception {
     final byte[] expected = {1, 2};
-    StorageRpc mockStorageRpc = createNiceMock(StorageRpc.class);
-    expect(storage.getOptions()).andReturn(mockOptions);
-    replay(storage);
-    expect(mockOptions.getStorageRpcV1()).andReturn(mockStorageRpc);
-    expect(mockOptions.getRetrySettings()).andReturn(RETRY_SETTINGS);
-    expect(mockOptions.getClock()).andReturn(API_CLOCK);
-    replay(mockOptions);
-    blob = new Blob(storage, new BlobInfo.BuilderImpl(BLOB_INFO));
+    StorageRpc mockStorageRpc = prepareForDownload();
     expect(
             mockStorageRpc.read(
                 anyObject(StorageObject.class),
@@ -662,4 +661,25 @@ public Long answer() throws Throwable {
     byte actual[] = Files.readAllBytes(file.toPath());
     assertArrayEquals(expected, actual);
   }
+
+  @Test
+  public void testDownloadToWithException() throws Exception {
+    StorageRpc mockStorageRpc = prepareForDownload();
+    Exception exception = new IllegalStateException("test");
+    expect(
+            mockStorageRpc.read(
+                anyObject(StorageObject.class),
+                anyObject(Map.class),
+                eq(0l),
+                anyObject(OutputStream.class)))
+        .andThrow(exception);
+    replay(mockStorageRpc);
+    File file = File.createTempFile("blob", ".tmp");
+    try {
+      blob.downloadTo(file.toPath());
+      fail();
+    } catch (StorageException e) {
+      assertSame(exception, e.getCause());
+    }
+  }
 }