From 68a0bee889a823620a96d9ff4d743d4079e295ef Mon Sep 17 00:00:00 2001 From: BenWhitehead Date: Mon, 13 Feb 2023 13:06:05 -0500 Subject: [PATCH] fix: update Default RetryStrategy to retry SSLException caused by SocketException Update Default RetryStrategy to retry when an SSLException is caused by a SocketException --- .../storage/DefaultStorageRetryStrategy.java | 11 +++++++- .../DefaultRetryHandlingBehaviorTest.java | 25 +++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/DefaultStorageRetryStrategy.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/DefaultStorageRetryStrategy.java index d580f54830..a6b197902d 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/DefaultStorageRetryStrategy.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/DefaultStorageRetryStrategy.java @@ -24,7 +24,9 @@ import com.google.common.collect.ImmutableSet; import com.google.gson.stream.MalformedJsonException; import java.io.IOException; +import java.net.SocketException; import java.util.Set; +import javax.net.ssl.SSLException; final class DefaultStorageRetryStrategy implements StorageRetryStrategy { @@ -102,7 +104,14 @@ private RetryResult shouldRetryIOException(IOException ioException) { return RetryResult.RETRY; } else if (ioException instanceof MalformedJsonException && idempotent) { // Gson return RetryResult.RETRY; - } else if (BaseServiceException.isRetryable(idempotent, ioException)) { + } else if (ioException instanceof SSLException && idempotent) { + Throwable cause = ioException.getCause(); + if (cause instanceof SocketException) { + SocketException se = (SocketException) cause; + return shouldRetryIOException(se); + } + } + if (BaseServiceException.isRetryable(idempotent, ioException)) { return RetryResult.RETRY; } else { return RetryResult.NO_RETRY; diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/DefaultRetryHandlingBehaviorTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/DefaultRetryHandlingBehaviorTest.java index 477594ddfe..e710143398 100644 --- a/google-cloud-storage/src/test/java/com/google/cloud/storage/DefaultRetryHandlingBehaviorTest.java +++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/DefaultRetryHandlingBehaviorTest.java @@ -268,6 +268,7 @@ enum ThrowableCategory { SOCKET_EXCEPTION(C.SOCKET_EXCEPTION), SSL_EXCEPTION(C.SSL_EXCEPTION), SSL_EXCEPTION_CONNECTION_SHUTDOWN(C.SSL_EXCEPTION_CONNECTION_SHUTDOWN), + SSL_EXCEPTION_CONNECTION_RESET(C.SSL_EXCEPTION_CONNECTION_RESET), SSL_HANDSHAKE_EXCEPTION(C.SSL_HANDSHAKE_EXCEPTION), SSL_HANDSHAKE_EXCEPTION_CAUSED_BY_CERTIFICATE_EXCEPTION( C.SSL_HANDSHAKE_EXCEPTION_CERTIFICATE_EXCEPTION), @@ -305,6 +306,8 @@ enum ThrowableCategory { STORAGE_EXCEPTION_SSL_EXCEPTION(new StorageException(C.SSL_EXCEPTION)), STORAGE_EXCEPTION_SSL_EXCEPTION_CONNECTION_SHUTDOWN( new StorageException(C.SSL_EXCEPTION_CONNECTION_SHUTDOWN)), + STORAGE_EXCEPTION_SSL_EXCEPTION_CONNECTION_RESET( + new StorageException(C.SSL_EXCEPTION_CONNECTION_RESET)), STORAGE_EXCEPTION_SSL_HANDSHAKE_EXCEPTION(new StorageException(C.SSL_HANDSHAKE_EXCEPTION)), STORAGE_EXCEPTION_SSL_HANDSHAKE_EXCEPTION_CAUSED_BY_CERTIFICATE_EXCEPTION( new StorageException(C.SSL_HANDSHAKE_EXCEPTION_CERTIFICATE_EXCEPTION)), @@ -361,6 +364,8 @@ private static final class C { private static final SSLException SSL_EXCEPTION = new SSLException("unknown"); private static final SSLException SSL_EXCEPTION_CONNECTION_SHUTDOWN = new SSLException("Connection has been shutdown: asdf"); + private static final SSLException SSL_EXCEPTION_CONNECTION_RESET = + new SSLException("Connection reset", new SocketException("Connection reset")); private static final SSLHandshakeException SSL_HANDSHAKE_EXCEPTION = newSslHandshakeExceptionWithCause(new SSLProtocolException(DEFAULT_MESSAGE)); private static final SSLHandshakeException SSL_HANDSHAKE_EXCEPTION_CERTIFICATE_EXCEPTION = @@ -614,6 +619,16 @@ private static ImmutableList getAllCases() { HandlerCategory.NONIDEMPOTENT, ExpectRetry.NO, Behavior.SAME), + new Case( + ThrowableCategory.SSL_EXCEPTION_CONNECTION_RESET, + HandlerCategory.IDEMPOTENT, + ExpectRetry.YES, + Behavior.DEFAULT_MORE_PERMISSIBLE), + new Case( + ThrowableCategory.SSL_EXCEPTION_CONNECTION_RESET, + HandlerCategory.NONIDEMPOTENT, + ExpectRetry.NO, + Behavior.SAME), new Case( ThrowableCategory.SSL_HANDSHAKE_EXCEPTION, HandlerCategory.IDEMPOTENT, @@ -884,6 +899,16 @@ private static ImmutableList getAllCases() { HandlerCategory.NONIDEMPOTENT, ExpectRetry.NO, Behavior.DEFAULT_MORE_STRICT), + new Case( + ThrowableCategory.STORAGE_EXCEPTION_SSL_EXCEPTION_CONNECTION_RESET, + HandlerCategory.IDEMPOTENT, + ExpectRetry.YES, + Behavior.DEFAULT_MORE_PERMISSIBLE), + new Case( + ThrowableCategory.STORAGE_EXCEPTION_SSL_EXCEPTION_CONNECTION_RESET, + HandlerCategory.NONIDEMPOTENT, + ExpectRetry.NO, + Behavior.SAME), new Case( ThrowableCategory.STORAGE_EXCEPTION_SSL_HANDSHAKE_EXCEPTION, HandlerCategory.IDEMPOTENT,