fix: connection closed prematurely in BlobReadChannel & ConnectionReset (#173)

frankyn · web-flow · commit 27bccda384da · 2020-03-13T10:18:20.000-07:00
* fix: connection closed prematurely

* add connection reset retry

* remove integration test

* format

* clean up tests

* change design of transient retry strategy

* add documentation

* format docs

* address feedback

* address feedback
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
@@ -33,6 +33,9 @@
  */
 @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 StorageException(int code, String message, Throwable cause) {
     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 static StorageException translateAndThrow(RetryHelperException ex) {
     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
@@ -701,7 +701,7 @@ public Tuple<String, byte[]> read(
       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/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 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 void setUp() {
         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
@@ -32,7 +32,9 @@
 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 void testStorageException() {
     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");
