Repository: googleapis/java-storage (java)

Issue:
Tracking bug: com.google.cloud.storage.StorageException: Connection closed prematurely:..
Using a ReadChannel, if a premature connection closure occurs, the client isn't able to recover when it should be able to. The client at that moment has a specific range context which makes it retryable. 

Additionally, the implementation is too reliant on etag[1] not changing when generation number can do the same thing. This also needs to be addressed. 

Example failure:
```
CONFIG: -------------- REQUEST  --------------
GET http://localhost:8080/download/storage/v1/b/test-bucket/o/test-create-blob?alt=media
Accept-Encoding: gzip
Authorization: <Not Logged>
Range: bytes=0-10485759
User-Agent: gcloud-java/ Google-API-Java-Client/1.30.9 Google-HTTP-Java-Client/1.34.2 (gzip)
x-goog-api-client: gl-java/1.8.0 gdcl/1.30.9 linux/5.2.17
x-goog-testbench-instructions: return-broken-stream

Mar 06, 2020 8:47:01 AM com.google.api.client.http.HttpRequest execute
CONFIG: curl -v --compressed -H 'Accept-Encoding: gzip' -H 'Authorization: <Not Logged>' -H 'Range: bytes=0-10485759' -H 'User-Agent: gcloud-java/ Google-API-Java-Client/1.30.9 Google-HTTP-Java-Client/1.34.2 (gzip)' -H 'x-goog-api-client: gl-java/1.8.0 gdcl/1.30.9 linux/5.2.17' -H 'x-goog-testbench-instructions: return-broken-stream' -- 'http://localhost:8080/download/storage/v1/b/test-bucket/o/test-create-blob?alt=media'
Mar 06, 2020 8:47:01 AM com.google.api.client.http.HttpResponse <init>
CONFIG: -------------- RESPONSE --------------
HTTP/1.0 200 OK
Server: Werkzeug/1.0.0 Python/3.7.0
Content-Range: bytes 0-10485758/10485760
x-goog-generation: 1
Content-Length: 10485760
Date: Fri, 06 Mar 2020 16:47:01 GMT
Content-Type: text/html; charset=utf-8
x-goog-hash: md5=8clkXbwU793H2KMiaF8m6w==,crc32c=CPJwmw==

Mar 06, 2020 8:47:03 AM com.google.api.client.util.LoggingByteArrayOutputStream close
CONFIG: Total: 1,114,112 bytes (logging first 16,384 bytes)
Mar 06, 2020 8:47:03 AM com.google.api.client.util.LoggingByteArrayOutputStream close
CONFIG:                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                 



com.google.cloud.storage.StorageException: Connection closed prematurely: bytesRead = 1114112, Content-Length = 10485760

	at com.google.cloud.storage.spi.v1.HttpStorageRpc.translate(HttpStorageRpc.java:230)
	at com.google.cloud.storage.spi.v1.HttpStorageRpc.read(HttpStorageRpc.java:711)
	at com.google.cloud.storage.BlobReadChannel$1.call(BlobReadChannel.java:129)
	at com.google.cloud.storage.BlobReadChannel$1.call(BlobReadChannel.java:125)
	at com.google.api.gax.retrying.DirectRetryingExecutor.submit(DirectRetryingExecutor.java:105)
	at com.google.cloud.RetryHelper.run(RetryHelper.java:76)
	at com.google.cloud.RetryHelper.runWithRetries(RetryHelper.java:50)
	at com.google.cloud.storage.BlobReadChannel.read(BlobReadChannel.java:124)
	at sun.nio.ch.ChannelInputStream.read(ChannelInputStream.java:65)
	at sun.nio.ch.ChannelInputStream.read(ChannelInputStream.java:109)
	at sun.nio.ch.ChannelInputStream.read(ChannelInputStream.java:103)
	at java.io.BufferedInputStream.read1(BufferedInputStream.java:284)
	at java.io.BufferedInputStream.read(BufferedInputStream.java:345)
	at java.io.FilterInputStream.read(FilterInputStream.java:107)
	at com.google.cloud.storage.it.ITStorageTest.testFailure(ITStorageTest.java:359)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:63)
	at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329)
	at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293)
	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
	at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:413)
	at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
	at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:68)
	at com.intellij.rt.execution.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:47)
	at com.intellij.rt.execution.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:242)
	at com.intellij.rt.execution.junit.JUnitStarter.main(JUnitStarter.java:70)
Caused by: java.io.IOException: Connection closed prematurely: bytesRead = 1114112, Content-Length = 10485760
	at com.google.api.client.http.javanet.NetHttpResponse$SizeValidatingInputStream.throwIfFalseEOF(NetHttpResponse.java:204)
	at com.google.api.client.http.javanet.NetHttpResponse$SizeValidatingInputStream.read(NetHttpResponse.java:166)
	at java.io.FilterInputStream.read(FilterInputStream.java:133)
	at com.google.api.client.util.LoggingInputStream.read(LoggingInputStream.java:57)
	at java.io.FilterInputStream.read(FilterInputStream.java:107)
	at com.google.api.client.util.ByteStreams.copy(ByteStreams.java:49)
	at com.google.api.client.util.IOUtils.copy(IOUtils.java:87)
	at com.google.api.client.util.IOUtils.copy(IOUtils.java:59)
	at com.google.api.client.http.HttpResponse.download(HttpResponse.java:410)
	at com.google.cloud.storage.spi.v1.HttpStorageRpc.read(HttpStorageRpc.java:706)
	... 40 more
```

Using C++ testbench[2] to reproduce issue but requires tweaks[3] to work with the Java library.

[1]: https://github.com/googleapis/java-storage/blob/master/google-cloud-storage/src/main/java/com/google/cloud/storage/BlobReadChannel.java#L133-L137
[2]: https://github.com/googleapis/google-cloud-cpp/tree/master/google/cloud/storage/testbench
[3]: https://github.com/googleapis/google-cloud-cpp/compare/master...frankyn:update-test-bench?expand=1

Pull request:
fix: connection closed prematurely in BlobReadChannel & ConnectionReset
Fixes #165 

Introduce:

- `public StorageException(int code, String message, String reason, Throwable cause)`:
to capture the reason which is used to check RETRYABLE_ERRORS.
- `public static StorageException translate(IOException exception)`: to help translate idempotent request exceptions in case they're RETRYABLE_ERRORS. This translate method should be introduced for other idempotent requests, but that's out of scope for this request.


Patch:
diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageException.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageException.java
index 01153a35..1f740194 100644
--- a/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageException.java
+++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageException.java
@@ -33,6 +33,9 @@ import java.util.Set;
  */
 @InternalApi
 public final class StorageException extends BaseHttpServiceException {
+  private static final String INTERNAL_ERROR = "internalError";
+  private static final String CONNECTION_CLOSED_PREMATURELY = "connectionClosedPrematurely";
+  private static final String CONNECTION_RESET = "connectionReset";
 
   // see: https://cloud.google.com/storage/docs/resumable-uploads-xml#practices
   private static final Set<Error> RETRYABLE_ERRORS =
@@ -43,7 +46,9 @@ public final class StorageException extends BaseHttpServiceException {
           new Error(500, null),
           new Error(429, null),
           new Error(408, null),
-          new Error(null, "internalError"));
+          new Error(null, INTERNAL_ERROR),
+          new Error(null, CONNECTION_CLOSED_PREMATURELY),
+          new Error(null, CONNECTION_RESET));
 
   private static final long serialVersionUID = -4168430271327813063L;
 
@@ -55,6 +60,10 @@ public final class StorageException extends BaseHttpServiceException {
     super(code, message, null, true, RETRYABLE_ERRORS, cause);
   }
 
+  public StorageException(int code, String message, String reason, Throwable cause) {
+    super(code, message, reason, true, RETRYABLE_ERRORS, cause);
+  }
+
   public StorageException(IOException exception) {
     super(exception, true, RETRYABLE_ERRORS);
   }
@@ -73,4 +82,23 @@ public final class StorageException extends BaseHttpServiceException {
     BaseServiceException.translate(ex);
     throw new StorageException(UNKNOWN_CODE, ex.getMessage(), ex.getCause());
   }
+
+  /**
+   * Translate IOException to a StorageException representing the cause of the error. This method
+   * defaults to idempotent always being {@code true}. Additionally, this method translates
+   * transient issues Connection Closed Prematurely and Connection Reset as retryable errors.
+   *
+   * @returns {@code StorageException}
+   */
+  public static StorageException translate(IOException exception) {
+    if (exception.getMessage().contains("Connection closed prematurely")) {
+      return new StorageException(
+          0, exception.getMessage(), CONNECTION_CLOSED_PREMATURELY, exception);
+    } else if (exception.getMessage().contains("Connection reset")) {
+      return new StorageException(0, exception.getMessage(), CONNECTION_RESET, exception);
+    } else {
+      // default
+      return new StorageException(exception);
+    }
+  }
 }
diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/spi/v1/HttpStorageRpc.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/spi/v1/HttpStorageRpc.java
index 5a6e947a..c677cbdb 100644
--- a/google-cloud-storage/src/main/java/com/google/cloud/storage/spi/v1/HttpStorageRpc.java
+++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/spi/v1/HttpStorageRpc.java
@@ -701,7 +701,7 @@ public class HttpStorageRpc implements StorageRpc {
       return Tuple.of(etag, output.toByteArray());
     } catch (IOException ex) {
       span.setStatus(Status.UNKNOWN.withDescription(ex.getMessage()));
-      StorageException serviceException = translate(ex);
+      StorageException serviceException = StorageException.translate(ex);
       if (serviceException.getCode() == SC_REQUESTED_RANGE_NOT_SATISFIABLE) {
         return Tuple.of(null, new byte[0]);
       }
diff --git a/pom.xml b/pom.xml
index 2a438244..802908a9 100644
--- a/pom.xml
+++ b/pom.xml
@@ -63,7 +63,7 @@
     <project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>
     <github.global.server>github</github.global.server>
     <site.installationModule>google-cloud-storage-parent</site.installationModule>
-    <google.core.version>1.93.1</google.core.version>
+    <google.core.version>1.93.2</google.core.version>
     <google.api-common.version>1.8.1</google.api-common.version>
     <junit.version>4.13</junit.version>
     <threeten.version>1.4.1</threeten.version>
@@ -103,7 +103,7 @@
       <dependency>
         <groupId>io.grpc</groupId>
         <artifactId>grpc-bom</artifactId>
-        <version>1.27.2</version>
+        <version>1.28.0</version>
         <type>pom</type>
         <scope>import</scope>
       </dependency>


Test patch:
diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/BlobReadChannelTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/BlobReadChannelTest.java
index 181e5414..f1388ebe 100644
--- a/google-cloud-storage/src/test/java/com/google/cloud/storage/BlobReadChannelTest.java
+++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/BlobReadChannelTest.java
@@ -28,7 +28,6 @@ import static org.junit.Assert.fail;
 
 import com.google.cloud.ReadChannel;
 import com.google.cloud.RestorableState;
-import com.google.cloud.ServiceOptions;
 import com.google.cloud.Tuple;
 import com.google.cloud.storage.spi.StorageRpcFactory;
 import com.google.cloud.storage.spi.v1.StorageRpc;
@@ -68,7 +67,6 @@ public class BlobReadChannelTest {
         StorageOptions.newBuilder()
             .setProjectId("projectId")
             .setServiceRpcFactory(rpcFactoryMock)
-            .setRetrySettings(ServiceOptions.getNoRetrySettings())
             .build();
   }
 
diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageExceptionTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageExceptionTest.java
index 5060fdc6..e065f401 100644
--- a/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageExceptionTest.java
+++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageExceptionTest.java
@@ -32,7 +32,9 @@ import com.google.api.client.http.HttpResponseException;
 import com.google.cloud.BaseServiceException;
 import com.google.cloud.RetryHelper.RetryHelperException;
 import java.io.IOException;
+import java.net.SocketException;
 import java.net.SocketTimeoutException;
+import javax.net.ssl.SSLException;
 import org.junit.Test;
 
 public class StorageExceptionTest {
@@ -135,6 +137,29 @@ public class StorageExceptionTest {
     assertTrue(exception.isRetryable());
   }
 
+  @Test
+  public void testTranslateConnectionReset() {
+    StorageException exception =
+        StorageException.translate(
+            new IOException(
+                "Connection has been shutdown: "
+                    + new SSLException(new SocketException("Connection reset"))));
+    assertEquals(0, exception.getCode());
+    assertEquals("connectionReset", exception.getReason());
+    assertTrue(exception.isRetryable());
+  }
+
+  @Test
+  public void testTranslateConnectionClosedPrematurely() {
+    StorageException exception =
+        StorageException.translate(
+            new IOException(
+                "Connection closed prematurely: bytesRead = 1114112, Content-Length = 10485760"));
+    assertEquals(0, exception.getCode());
+    assertEquals("connectionClosedPrematurely", exception.getReason());
+    assertTrue(exception.isRetryable());
+  }
+
   @Test
   public void testTranslateAndThrow() throws Exception {
     Exception cause = new StorageException(503, "message");