[FFM-10821] - Add Retry-After HTTP header support (#184)

* [FFM-10821] - Add Retry-After HTTP header support

What
If Retry-After header is detected, use that first when retrying a failed API connection

Why
Allow backend to control how SDKs retry

Testing
Manual + unit testing

Author: andybharness

examples/src/main/java/io/harness/ff/examples/GettingStarted.java

@@ -1,6 +1,8 @@
 package io.harness.ff.examples;
 
 import io.harness.cf.client.api.*;
+import io.harness.cf.client.connector.HarnessConfig;
+import io.harness.cf.client.connector.HarnessConnector;
 import io.harness.cf.client.dto.Target;
 
 import java.util.concurrent.Executors;
@@ -19,8 +21,13 @@ public class GettingStarted {
     public static void main(String[] args) {
         System.out.println("Harness SDK Getting Started");
 
+        final HarnessConnector connector = new HarnessConnector(apiKey, HarnessConfig.builder()
+                //.configUrl("http://localhost:8000/api/1.0")
+                //.eventUrl("http://localhost:8001/api/1.0")
+                .build());
+
         //Create a Feature Flag Client
-        try (CfClient cfClient = new CfClient(apiKey, BaseConfig.builder().build())) {
+        try (CfClient cfClient = new CfClient(connector)) {
             cfClient.waitForInitialization();
 
             // Create a target (different targets can get different results based on rules.  This includes a custom attribute 'location')

src/main/java/io/harness/cf/client/api/PollingProcessor.java

@@ -8,10 +8,7 @@
 import io.harness.cf.model.FeatureConfig;
 import io.harness.cf.model.Segment;
 import java.util.List;
-import java.util.concurrent.CompletableFuture;
-import java.util.concurrent.Executors;
-import java.util.concurrent.ScheduledExecutorService;
-import java.util.concurrent.ScheduledFuture;
+import java.util.concurrent.*;
 import lombok.NonNull;
 import lombok.extern.slf4j.Slf4j;
 
@@ -81,7 +78,12 @@ public CompletableFuture<List<Segment>> retrieveSegments() {
   }
 
   public void retrieveAll() {
-    CompletableFuture.allOf(retrieveFlags(), retrieveSegments()).join();
+    try {
+      CompletableFuture.allOf(retrieveFlags(), retrieveSegments()).join();
+    } catch (CompletionException | CancellationException ex) {
+      log.warn("retrieveAll failed: {} - {}", ex.getClass().getSimpleName(), ex.getMessage());
+      log.trace("retrieveAll failed", ex);
+    }
   }
 
   private void runOneIteration() {

src/main/java/io/harness/cf/client/connector/NewRetryInterceptor.java

@@ -1,14 +1,23 @@
 package io.harness.cf.client.connector;
 
 import java.io.IOException;
+import java.text.ParseException;
+import java.text.SimpleDateFormat;
+import java.time.Duration;
+import java.time.Instant;
+import java.util.Date;
+import java.util.Locale;
 import java.util.concurrent.TimeUnit;
-import lombok.extern.slf4j.Slf4j;
 import okhttp3.*;
 import org.jetbrains.annotations.NotNull;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
-@Slf4j
 public class NewRetryInterceptor implements Interceptor {
 
+  private static final Logger log = LoggerFactory.getLogger(NewRetryInterceptor.class);
+  private static final SimpleDateFormat imfDateFormat =
+      new SimpleDateFormat("EEE, dd MMM yyyy HH:mm:ss zzz", Locale.US);
   private final long retryBackoffDelay;
   private final long maxTryCount;
 
@@ -27,6 +36,7 @@ public NewRetryInterceptor(long maxTryCount, long retryBackoffDelay) {
   public Response intercept(@NotNull Chain chain) throws IOException {
     int tryCount = 1;
     boolean successful;
+    boolean limitReached = false;
     Response response = null;
     String msg = "";
     do {
@@ -36,7 +46,9 @@ public Response intercept(@NotNull Chain chain) throws IOException {
         response = chain.proceed(chain.request());
         successful = response.isSuccessful();
         if (!successful) {
-          msg = String.format("httpCode=%d %s", response.code(), response.message());
+          msg =
+              String.format(
+                  Locale.getDefault(), "httpCode=%d %s", response.code(), response.message());
           if (!shouldRetryHttpErrorCode(response.code())) {
             return response;
           }
@@ -47,34 +59,81 @@ public Response intercept(@NotNull Chain chain) throws IOException {
 
       } catch (Exception ex) {
         log.trace("Error while attempting to make request", ex);
-        msg = ex.getMessage();
+        msg = ex.getClass().getSimpleName() + ": " + ex.getMessage();
         response = makeErrorResp(chain, msg);
         successful = false;
         if (!shouldRetryException(ex)) {
           return response;
         }
       }
       if (!successful) {
-        final boolean limitReached = tryCount > maxTryCount;
+        int retryAfterHeaderValue = getRetryAfterHeaderInSeconds(response);
+        long backOffDelayMs;
+        if (retryAfterHeaderValue > 0) {
+          // Use Retry-After header if detected first
+          log.trace("Retry-After header detected: {} seconds", retryAfterHeaderValue);
+          backOffDelayMs = retryAfterHeaderValue * 1000L;
+        } else {
+          // Else fallback to a randomized exponential backoff
+          backOffDelayMs = retryBackoffDelay * tryCount;
+        }
+
+        limitReached = tryCount >= maxTryCount;
         log.warn(
             "Request attempt {} to {} was not successful, [{}]{}",
             tryCount,
             chain.request().url(),
             msg,
             limitReached
                 ? ", retry limited reached"
-                : String.format(", retrying in %dms", retryBackoffDelay * tryCount));
+                : String.format(
+                    Locale.getDefault(),
+                    ", retrying in %dms  (retry-after hdr: %b)",
+                    backOffDelayMs,
+                    retryAfterHeaderValue > 0));
 
         if (!limitReached) {
-          sleep(retryBackoffDelay * tryCount);
+          sleep(backOffDelayMs);
         }
       }
-    } while (!successful && tryCount++ <= maxTryCount);
+      tryCount++;
+    } while (!successful && !limitReached);
 
     return response;
   }
 
+  int getRetryAfterHeaderInSeconds(Response response) {
+    final String retryAfterValue = response.header("Retry-After");
+    if (retryAfterValue == null) {
+      return 0;
+    }
+
+    int seconds = 0;
+    try {
+      seconds = Integer.parseInt(retryAfterValue);
+    } catch (NumberFormatException ignored) {
+    }
+
+    if (seconds <= 0) {
+      try {
+        final Date then = imfDateFormat.parse(retryAfterValue);
+        if (then != null) {
+          seconds = (int) Duration.between(Instant.now(), then.toInstant()).getSeconds();
+        }
+      } catch (ParseException ignored) {
+      }
+    }
+
+    if (seconds < 0) {
+      seconds = 0;
+    }
+
+    return Math.min(seconds, 3600);
+  }
+
   private boolean shouldRetryException(Exception ex) {
+    log.debug(
+        "should retry exception check: {} - {}", ex.getClass().getSimpleName(), ex.getMessage());
     return true;
   }
 
@@ -87,7 +146,7 @@ private boolean shouldRetryHttpErrorCode(int httpCode) {
 
   private Response makeErrorResp(Chain chain, String msg) {
     return new Response.Builder()
-        .code(404) /* dummy response: real reason is in the message */
+        .code(400) /* dummy response: real reason is in the message */
         .request(chain.request())
         .protocol(Protocol.HTTP_2)
         .message(msg)

src/test/java/io/harness/cf/client/api/CfClientTest.java

@@ -148,7 +148,7 @@ void shouldAllowEvaluationsToContinueWhenAuthFails(boolean shouldWaitForInit)
             .streamEnabled(true)
             .build();
 
-    Http4xxOnAuthDispatcher webserverDispatcher = new Http4xxOnAuthDispatcher(429, 4); // auth error
+    Http4xxOnAuthDispatcher webserverDispatcher = new Http4xxOnAuthDispatcher(429, 3); // auth error
 
     try (MockWebServer mockSvr = new MockWebServer()) {
       mockSvr.setDispatcher(webserverDispatcher);
@@ -205,7 +205,7 @@ void shouldAllowEvaluationsToContinueWhenAuthFails(boolean shouldWaitForInit)
         webserverDispatcher.waitForAllConnections(15);
 
         assertTrue(
-            webserverDispatcher.getUrlMap().get(AUTH_ENDPOINT) >= (3 + 1),
+            webserverDispatcher.getUrlMap().get(AUTH_ENDPOINT) >= (2 + 1),
             "not enough authentication attempts");
 
         assertEquals(
@@ -465,7 +465,7 @@ void shouldRetryThenReAuthenticateWithoutThrowingIllegalStateException() throws
         // First 3 attempts to connect to auth endpoint will return a 408, followed by a 200 success
         webserverDispatcher.waitForAllConnections(15);
 
-        final int expectedAuths = 3 + 3 + 1; // 3+3 failed retries (4xx), 1 success (200)
+        final int expectedAuths = 2 + 2 + 1; // 2+2 failed retries (4xx), 1 success (200)
 
         assertEquals(
             expectedAuths,

src/test/java/io/harness/cf/client/api/dispatchers/Http4xxOnFirstAuthDispatcher.java

@@ -25,7 +25,7 @@ public Http4xxOnFirstAuthDispatcher(int minAuthAttempts) {
   private MockResponse dispatchAuthResp() {
     System.out.println("DISPATCH authAttempts = " + authAttempts.get());
 
-    if (authAttempts.getAndIncrement() < 6) {
+    if (authAttempts.getAndIncrement() < 4) {
       System.out.println("--> 408");
       return makeAuthResponse(408);
     }