Issue 47: make retryable exception predicate less strict (#48)

Issue 47: make retryable exception predicate less strict to allow more exceptions to be retried. Signed-off-by: Christophe Balczunas <[email protected]>
pravega · Sep 17, 2021 · fcb0654 · fcb0654
1 parent 46eec40
commit fcb0654
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 21 deletions.
diff --git a/build.gradle b/build.gradle
@@ -12,7 +12,6 @@ import com.github.jengelman.gradle.plugins.shadow.tasks.ConfigureShadowRelocatio
 
 buildscript {
     repositories {
-        jcenter()
         mavenCentral()
     }
 }
@@ -33,9 +32,13 @@ subprojects {
     apply plugin: 'com.github.johnrengelman.shadow'
 
     repositories {
-        jcenter()
+        mavenCentral()
         maven {
-            url "https://oss.jfrog.org/jfrog-dependencies"
+            url "https://maven.pkg.github.com/pravega/pravega"
+            credentials {
+                username = "pravega-public"
+                password = "\u0067\u0068\u0070\u005F\u0048\u0034\u0046\u0079\u0047\u005A\u0031\u006B\u0056\u0030\u0051\u0070\u006B\u0079\u0058\u006D\u0035\u0063\u0034\u0055\u0033\u006E\u0032\u0065\u0078\u0039\u0032\u0046\u006E\u0071\u0033\u0053\u0046\u0076\u005A\u0049"
+            }
         }
     }
 

diff --git a/client/src/main/java/io/pravega/keycloak/client/KeycloakAuthzClient.java b/client/src/main/java/io/pravega/keycloak/client/KeycloakAuthzClient.java
@@ -118,17 +118,18 @@ public String getRPT() {
     }
 
     /**
-     * Predicate to determine what is retryable and what is not.
+     * Predicate to determine what exception is retryable and what is not.
+     * Typically, the incoming exception will be wrapped in a RuntimeException.
+     * It is unwrapped here to look at the root cause.
      * HttpResponseException with error code of 400, 401, 403 are not retryable.
-     * All other HttpResponseException are retryable as well as java.net.ConnectException.
-     * All others are not retryable.
+     * All other HttpResponseException are retryable as well as any other non HttpResponseException exceptions.
      * @return
      */
     private static Predicate<Throwable> isRetryable() {
         return e -> {
             Throwable rootCause = unwrap(e);
             if (rootCause instanceof HttpResponseException) {
-                int statusCode = ((HttpResponseException) e).getStatusCode();
+                int statusCode = ((HttpResponseException) rootCause).getStatusCode();
                 if (statusCode == HttpStatus.SC_BAD_REQUEST ||
                         statusCode == HttpStatus.SC_UNAUTHORIZED ||
                         statusCode == HttpStatus.SC_FORBIDDEN ) {
@@ -139,14 +140,10 @@ private static Predicate<Throwable> isRetryable() {
                     LOG.warn("Retryable HttpResponseException with HTTP code:  {}", statusCode);
                     return true;
                 }
-            } else if (rootCause instanceof ConnectException || rootCause instanceof UnknownHostException) {
-                LOG.warn("Retryable connection exception", rootCause);
-                return true;
-            } else {
-                    // random unexpected Exceptions, don't retry, these should be debugged.
-                    LOG.error("Other non retryable exception", rootCause);
-                    return false;
-                }
+            }
+            LOG.warn("Retryable exception ", e);
+            LOG.warn("Retryable exception root cause", rootCause);
+            return true;
         };
     }
 

diff --git a/client/src/test/java/io/pravega/keycloak/client/KeycloakAuthzClientTest.java b/client/src/test/java/io/pravega/keycloak/client/KeycloakAuthzClientTest.java
@@ -109,6 +109,16 @@ public void getRPTWithHttp500Exception() {
         assertRetried(new HttpResponseException("", 500, "", null), 3);
     }
 
+    @Test
+    public void getRPTWithHttp401Exception() {
+        assertRetried(new HttpResponseException("", 401, "", null), 1);
+    }
+
+    @Test
+    public void getRPTWithHttp403Exception() {
+        assertRetried(new HttpResponseException("", 403, "", null), 1);
+    }
+
     @Test
     public void getRPTWithRuntimeConnectException() {
         assertRetried(new RuntimeException(new ConnectException()), 3);
@@ -121,7 +131,7 @@ public void getRPTWithRuntimeUnknownHostException() {
 
     @Test
     public void getRPTWithRandomRuntimeException() {
-        assertRetried(new RuntimeException("bogus"), 1);
+        assertRetried(new RuntimeException("bogus"), 3);
     }
 
     @Test

diff --git a/gradle.properties b/gradle.properties
@@ -14,7 +14,7 @@ guavaVersion=28.2-jre
 junitVersion=4.13.2
 keycloakVersion=12.0.4
 mockitoVersion=3.3.3
-pravegaVersion=0.9.0
+pravegaVersion=0.10.0-2976.ea3db6f-SNAPSHOT
 slf4jVersion=1.7.30
 
 buildVersion=0.10.0-SNAPSHOT
@@ -27,16 +27,13 @@ buildVersion=0.10.0-SNAPSHOT
 # Properties for publishing to repository
 # ------------------------------------------------
 
-# Alternatives: mavenCentral, jcenter, localFileRepo, any string (in conjunction with `publishUrl...`
+# Alternatives: mavenCentral, localFileRepo, any string (in conjunction with `publishUrl...`
 publishRepo=localFileRepo
 
 # Repo credentials
 publishUsername=
 publishPassword=
 
-# Publish URL, when publishing to Artifactory (dev-time)
-publishUrl=https://oss.jfrog.org
-
 # Specified if a custom `publishRepo` is specified
 publishUrlForSnapshots=
 publishUrlForReleases=