Skip to content

Commit 5599f29

Browse files
authored
fix: Blob.downloadTo() methods do not wrap RetryHelper$RetryHelperException (#218)
fix: wrap exception
1 parent b5dce9e commit 5599f29

File tree

2 files changed

+53
-28
lines changed
  • google-cloud-storage/src

2 files changed

+53
-28
lines changed

google-cloud-storage/src/main/java/com/google/cloud/storage/Blob.java

+20-15
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import com.google.auth.ServiceAccountSigner;
2727
import com.google.auth.ServiceAccountSigner.SigningException;
2828
import com.google.cloud.ReadChannel;
29+
import com.google.cloud.RetryHelper;
2930
import com.google.cloud.Tuple;
3031
import com.google.cloud.WriteChannel;
3132
import com.google.cloud.storage.Acl.Entity;
@@ -230,21 +231,25 @@ public void downloadTo(OutputStream outputStream, BlobSourceOption... options) {
230231
final CountingOutputStream countingOutputStream = new CountingOutputStream(outputStream);
231232
final StorageRpc storageRpc = this.options.getStorageRpcV1();
232233
final Map<StorageRpc.Option, ?> requestOptions = StorageImpl.optionMap(getBlobId(), options);
233-
runWithRetries(
234-
callable(
235-
new Runnable() {
236-
@Override
237-
public void run() {
238-
storageRpc.read(
239-
getBlobId().toPb(),
240-
requestOptions,
241-
countingOutputStream.getCount(),
242-
countingOutputStream);
243-
}
244-
}),
245-
this.options.getRetrySettings(),
246-
StorageImpl.EXCEPTION_HANDLER,
247-
this.options.getClock());
234+
try {
235+
runWithRetries(
236+
callable(
237+
new Runnable() {
238+
@Override
239+
public void run() {
240+
storageRpc.read(
241+
getBlobId().toPb(),
242+
requestOptions,
243+
countingOutputStream.getCount(),
244+
countingOutputStream);
245+
}
246+
}),
247+
this.options.getRetrySettings(),
248+
StorageImpl.EXCEPTION_HANDLER,
249+
this.options.getClock());
250+
} catch (RetryHelper.RetryHelperException e) {
251+
StorageException.translateAndThrow(e);
252+
}
248253
}
249254

250255
/**

google-cloud-storage/src/test/java/com/google/cloud/storage/BlobTest.java

+33-13
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import static org.junit.Assert.assertNull;
3333
import static org.junit.Assert.assertSame;
3434
import static org.junit.Assert.assertTrue;
35+
import static org.junit.Assert.fail;
3536

3637
import com.google.api.core.ApiClock;
3738
import com.google.api.gax.retrying.RetrySettings;
@@ -585,17 +586,22 @@ public void testBuilder() {
585586
assertTrue(blob.isDirectory());
586587
}
587588

588-
@Test
589-
public void testDownload() throws Exception {
590-
final byte[] expected = {1, 2};
589+
private StorageRpc prepareForDownload() {
591590
StorageRpc mockStorageRpc = createNiceMock(StorageRpc.class);
592-
expect(storage.getOptions()).andReturn(mockOptions).times(1);
591+
expect(storage.getOptions()).andReturn(mockOptions);
593592
replay(storage);
594593
expect(mockOptions.getStorageRpcV1()).andReturn(mockStorageRpc);
595594
expect(mockOptions.getRetrySettings()).andReturn(RETRY_SETTINGS);
596595
expect(mockOptions.getClock()).andReturn(API_CLOCK);
597596
replay(mockOptions);
598597
blob = new Blob(storage, new BlobInfo.BuilderImpl(BLOB_INFO));
598+
return mockStorageRpc;
599+
}
600+
601+
@Test
602+
public void testDownloadTo() throws Exception {
603+
final byte[] expected = {1, 2};
604+
StorageRpc mockStorageRpc = prepareForDownload();
599605
expect(
600606
mockStorageRpc.read(
601607
anyObject(StorageObject.class),
@@ -618,16 +624,9 @@ public Long answer() throws Throwable {
618624
}
619625

620626
@Test
621-
public void testDownloadWithRetries() throws Exception {
627+
public void testDownloadToWithRetries() throws Exception {
622628
final byte[] expected = {1, 2};
623-
StorageRpc mockStorageRpc = createNiceMock(StorageRpc.class);
624-
expect(storage.getOptions()).andReturn(mockOptions);
625-
replay(storage);
626-
expect(mockOptions.getStorageRpcV1()).andReturn(mockStorageRpc);
627-
expect(mockOptions.getRetrySettings()).andReturn(RETRY_SETTINGS);
628-
expect(mockOptions.getClock()).andReturn(API_CLOCK);
629-
replay(mockOptions);
630-
blob = new Blob(storage, new BlobInfo.BuilderImpl(BLOB_INFO));
629+
StorageRpc mockStorageRpc = prepareForDownload();
631630
expect(
632631
mockStorageRpc.read(
633632
anyObject(StorageObject.class),
@@ -662,4 +661,25 @@ public Long answer() throws Throwable {
662661
byte actual[] = Files.readAllBytes(file.toPath());
663662
assertArrayEquals(expected, actual);
664663
}
664+
665+
@Test
666+
public void testDownloadToWithException() throws Exception {
667+
StorageRpc mockStorageRpc = prepareForDownload();
668+
Exception exception = new IllegalStateException("test");
669+
expect(
670+
mockStorageRpc.read(
671+
anyObject(StorageObject.class),
672+
anyObject(Map.class),
673+
eq(0l),
674+
anyObject(OutputStream.class)))
675+
.andThrow(exception);
676+
replay(mockStorageRpc);
677+
File file = File.createTempFile("blob", ".tmp");
678+
try {
679+
blob.downloadTo(file.toPath());
680+
fail();
681+
} catch (StorageException e) {
682+
assertSame(exception, e.getCause());
683+
}
684+
}
665685
}

0 commit comments

Comments
 (0)