Spanner: Throw exception when SSLHandshakeException occurs instead of infinite retry loop (#4663)

* #3889 throw exception when an SSLHandshakeException occurs

SSLHandshakeExceptions are not retryable, as it is most probably an
indication that the client does not accept the server certificate.

* #3889 added test for retryability of SSLHandshakeException

* fixed formatting

Author: olavloite

google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerExceptionFactory.java

@@ -16,10 +16,9 @@
 
 package com.google.cloud.spanner;
 
-import static com.google.cloud.spanner.SpannerException.DoNotConstructDirectly;
-
 import com.google.api.gax.grpc.GrpcStatusCode;
 import com.google.api.gax.rpc.ApiException;
+import com.google.cloud.spanner.SpannerException.DoNotConstructDirectly;
 import com.google.common.base.MoreObjects;
 import com.google.common.base.Predicate;
 import io.grpc.Context;
@@ -28,6 +27,7 @@
 import java.util.concurrent.CancellationException;
 import java.util.concurrent.TimeoutException;
 import javax.annotation.Nullable;
+import javax.net.ssl.SSLHandshakeException;
 
 /**
  * A factory for creating instances of {@link SpannerException} and its subtypes. All creation of
@@ -168,7 +168,9 @@ private static boolean isRetryable(ErrorCode code, @Nullable Throwable cause) {
       case INTERNAL:
         return hasCauseMatching(cause, Matchers.isRetryableInternalError);
       case UNAVAILABLE:
-        return true;
+        // SSLHandshakeException is (probably) not retryable, as it is an indication that the server
+        // certificate was not accepted by the client.
+        return !hasCauseMatching(cause, Matchers.isSSLHandshakeException);
       case RESOURCE_EXHAUSTED:
         return SpannerException.extractRetryDelay(cause) > 0;
       default:
@@ -211,5 +213,12 @@ public boolean apply(Throwable cause) {
             return false;
           }
         };
+    static final Predicate<Throwable> isSSLHandshakeException =
+        new Predicate<Throwable>() {
+          @Override
+          public boolean apply(Throwable input) {
+            return input instanceof SSLHandshakeException;
+          }
+        };
   }
 }

google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplTest.java

@@ -17,13 +17,16 @@
 package com.google.cloud.spanner;
 
 import static com.google.common.truth.Truth.assertThat;
+import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.assertThat;
 import static org.junit.Assert.fail;
 
 import com.google.cloud.grpc.GrpcTransportOptions;
 import com.google.cloud.spanner.spi.v1.SpannerRpc;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.concurrent.Callable;
+import javax.net.ssl.SSLHandshakeException;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -133,4 +136,49 @@ public Void call() throws Exception {
       assertThat(e.getMessage().contains("Unexpected exception thrown"));
     }
   }
+
+  @Test
+  public void sslHandshakeExceptionIsNotRetryable() {
+    // Verify that a SpannerException with code UNAVAILABLE and cause SSLHandshakeException is not
+    // retryable.
+    boolean gotExpectedException = false;
+    try {
+      SpannerImpl.runWithRetries(
+          new Callable<Object>() {
+            @Override
+            public Void call() throws Exception {
+              throw SpannerExceptionFactory.newSpannerException(
+                  ErrorCode.UNAVAILABLE,
+                  "This exception should not be retryable",
+                  new SSLHandshakeException("some SSL handshake exception"));
+            }
+          });
+    } catch (SpannerException e) {
+      gotExpectedException = true;
+      assertThat(e.isRetryable(), is(false));
+      assertThat(e.getErrorCode()).isEqualTo(ErrorCode.UNAVAILABLE);
+      assertThat(e.getMessage().contains("This exception should not be retryable"));
+    }
+    assertThat(gotExpectedException, is(true));
+
+    // Verify that any other SpannerException with code UNAVAILABLE is retryable.
+    SpannerImpl.runWithRetries(
+        new Callable<Object>() {
+          private boolean firstTime = true;
+
+          @Override
+          public Void call() throws Exception {
+            // Keep track of whethr this is the first call or a subsequent call to avoid an infinite
+            // loop.
+            if (firstTime) {
+              firstTime = false;
+              throw SpannerExceptionFactory.newSpannerException(
+                  ErrorCode.UNAVAILABLE,
+                  "This exception should be retryable",
+                  new Exception("some other exception"));
+            }
+            return null;
+          }
+        });
+  }
 }