Don't retry permanent failures when uploading ICANN monthly reports (#328)

* Don't retry permanent failures when uploading ICANN monthly reports

There are two kinds of permanent failures that this checks for that we know will
never succeed, so it makes no sense to continue retrying 11 more times before
moving onto the next file to upload. These errors are:

1.
com.google.api.client.http.HttpResponseException: 403
Your IP address xx.xx.xx.xx is not allowed to connect

2.
com.google.api.client.http.HttpResponseException: 400
<?xml version="1.0" encoding="UTF-8" standalone="yes"?><response xmlns="urn:ietf:params:xml:ns:iirdea-1.0"><result code="2002"><msg>A report for that month already exists, the cut-off date already passed.</msg><description>Date: 2019-09</description></result></response>

In order to implement this new functionality, this commit also adds a new way to
call Retriable that allows specifying the isRetryable Predicate (which is quite
useful).

util/src/main/java/google/registry/util/Retrier.java

@@ -70,32 +70,8 @@ public class Retrier implements Serializable {
    *
    * @return <V> the value returned by the {@link Callable}.
    */
-  private <V> V callWithRetry(
-      Callable<V> callable,
-      FailureReporter failureReporter,
-      Predicate<Throwable> isRetryable) {
-    int failures = 0;
-    while (true) {
-      try {
-        return callable.call();
-      } catch (Throwable e) {
-        if (++failures == attempts || !isRetryable.test(e)) {
-          throwIfUnchecked(e);
-          throw new RuntimeException(e);
-        }
-        failureReporter.beforeRetry(e, failures, attempts);
-        try {
-          // Wait 100ms on the first attempt, doubling on each subsequent attempt.
-          sleeper.sleep(Duration.millis(pow(2, failures) * 100));
-        } catch (InterruptedException e2) {
-          // Since we're not rethrowing InterruptedException, set the interrupt state on the thread
-          // so the next blocking operation will know to abort the thread.
-          Thread.currentThread().interrupt();
-          throwIfUnchecked(e);
-          throw new RuntimeException(e);
-        }
-      }
-    }
+  public <V> V callWithRetry(Callable<V> callable, Predicate<Throwable> isRetryable) {
+    return callWithRetry(callable, LOGGING_FAILURE_REPORTER, isRetryable);
   }
 
   /**
@@ -169,6 +145,34 @@ public class Retrier implements Serializable {
     callWithRetry(callable.asCallable(), failureReporter, retryableError, moreRetryableErrors);
   }
 
+  private <V> V callWithRetry(
+      Callable<V> callable,
+      FailureReporter failureReporter,
+      Predicate<Throwable> isRetryable) {
+    int failures = 0;
+    while (true) {
+      try {
+        return callable.call();
+      } catch (Throwable e) {
+        if (++failures == attempts || !isRetryable.test(e)) {
+          throwIfUnchecked(e);
+          throw new RuntimeException(e);
+        }
+        failureReporter.beforeRetry(e, failures, attempts);
+        try {
+          // Wait 100ms on the first attempt, doubling on each subsequent attempt.
+          sleeper.sleep(Duration.millis(pow(2, failures) * 100));
+        } catch (InterruptedException e2) {
+          // Since we're not rethrowing InterruptedException, set the interrupt state on the thread
+          // so the next blocking operation will know to abort the thread.
+          Thread.currentThread().interrupt();
+          throwIfUnchecked(e);
+          throw new RuntimeException(e);
+        }
+      }
+    }
+  }
+
   private static final FailureReporter LOGGING_FAILURE_REPORTER =
       (thrown, failures, maxAttempts) ->
           logger.atInfo().withCause(thrown).log(

util/src/test/java/google/registry/util/RetrierTest.java

@@ -38,7 +38,7 @@ public class RetrierTest {
     }
   }
 
-  /** Test object that always throws an exception with the current count. */
+  /** Test object that throws CountingExceptions up to a given limit, then succeeds. */
   static class CountingThrower implements Callable<Integer> {
 
     int count = 0;
@@ -125,4 +125,21 @@ public class RetrierTest {
         .isEqualTo(0);
     assertThat(reporter.numBeforeRetry).isEqualTo(0);
   }
+
+  @Test
+  public void testRetryPredicate_succeedsWhenRetries() {
+    // Throws a retryable "1" exception is retryable, and then it succeeds on "1".
+    assertThat(retrier.callWithRetry(new CountingThrower(1), e -> e.getMessage().equals("1")))
+        .isEqualTo(1);
+  }
+
+  @Test
+  public void testRetryPredicate_failsWhenDoesntRetry() {
+    // Throws a retryable "1" exception, then a non-retryable "2" exception, resulting in failure.
+    CountingException ex =
+        assertThrows(
+            CountingException.class,
+            () -> retrier.callWithRetry(new CountingThrower(2), e -> e.getMessage().equals("1")));
+    assertThat(ex).hasMessageThat().isEqualTo("2");
+  }
 }