Repository: Unleash/unleash-client-java
Pull request #222 (2023-11-08 12:16:31)

fix: prevent failure count going below zero
While testing this, we found that successful invocation reduced failure count below 0, this caused the backoff to not work as intended


Patch:
diff --git a/src/main/java/io/getunleash/util/Throttler.java b/src/main/java/io/getunleash/util/Throttler.java
index 6427f1c1..66601875 100644
--- a/src/main/java/io/getunleash/util/Throttler.java
+++ b/src/main/java/io/getunleash/util/Throttler.java
@@ -30,7 +30,9 @@ public class Throttler {
      * immediately after a sequence of errors.
      */
     public void decrementFailureCountAndResetSkips() {
-        skips.set(Math.max(failures.decrementAndGet(), 0));
+        if (failures.get() > 0) {
+            skips.set(Math.max(failures.decrementAndGet(), 0));
+        }
     }
 
     /**
@@ -61,13 +63,14 @@ public class Throttler {
     }
 
     public void handleHttpErrorCodes(int responseCode) {
-        if (responseCode == HttpURLConnection.HTTP_UNAUTHORIZED || responseCode == HttpURLConnection.HTTP_FORBIDDEN) {
+        if (responseCode == HttpURLConnection.HTTP_UNAUTHORIZED
+                || responseCode == HttpURLConnection.HTTP_FORBIDDEN) {
             maximizeSkips();
             LOGGER.error(
-                "Client was not authorized to talk to the Unleash API at {}. Backing off to {} times our poll interval (of {} seconds) to avoid overloading server",
-                this.target,
-                maxSkips,
-                this.intervalLength);
+                    "Client was not authorized to talk to the Unleash API at {}. Backing off to {} times our poll interval (of {} seconds) to avoid overloading server",
+                    this.target,
+                    maxSkips,
+                    this.intervalLength);
         }
         if (responseCode == HttpURLConnection.HTTP_NOT_FOUND) {
             maximizeSkips();


Test patch:
diff --git a/src/test/java/io/getunleash/util/ThrottlerTest.java b/src/test/java/io/getunleash/util/ThrottlerTest.java
new file mode 100644
index 00000000..433f4953
--- /dev/null
+++ b/src/test/java/io/getunleash/util/ThrottlerTest.java
@@ -0,0 +1,71 @@
+package io.getunleash.util;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.net.MalformedURLException;
+import java.net.URI;
+import org.junit.jupiter.api.Test;
+
+class ThrottlerTest {
+
+    @Test
+    public void shouldNeverDecrementFailuresOrSkipsBelowZero() throws MalformedURLException {
+        Throttler throttler =
+                new Throttler(10, 300, URI.create("https://localhost:1500/api").toURL());
+        throttler.decrementFailureCountAndResetSkips();
+        throttler.decrementFailureCountAndResetSkips();
+        throttler.decrementFailureCountAndResetSkips();
+        throttler.decrementFailureCountAndResetSkips();
+        throttler.decrementFailureCountAndResetSkips();
+        assertThat(throttler.getSkips()).isEqualTo(0);
+        assertThat(throttler.getFailures()).isEqualTo(0);
+    }
+
+    @Test
+    public void setToMaxShouldReduceDownEventually() throws MalformedURLException {
+        Throttler throttler =
+                new Throttler(150, 300, URI.create("https://localhost:1500/api").toURL());
+        throttler.handleHttpErrorCodes(404);
+        assertThat(throttler.getSkips()).isEqualTo(2);
+        assertThat(throttler.getFailures()).isEqualTo(1);
+        throttler.skipped();
+        assertThat(throttler.getSkips()).isEqualTo(1);
+        assertThat(throttler.getFailures()).isEqualTo(1);
+        throttler.skipped();
+        assertThat(throttler.getSkips()).isEqualTo(0);
+        assertThat(throttler.getFailures()).isEqualTo(1);
+        throttler.decrementFailureCountAndResetSkips();
+        assertThat(throttler.getSkips()).isEqualTo(0);
+        assertThat(throttler.getFailures()).isEqualTo(0);
+        throttler.decrementFailureCountAndResetSkips();
+        assertThat(throttler.getSkips()).isEqualTo(0);
+        assertThat(throttler.getFailures()).isEqualTo(0);
+    }
+
+    @Test
+    public void handleIntermittentFailures() throws MalformedURLException {
+        Throttler throttler =
+                new Throttler(50, 300, URI.create("https://localhost:1500/api").toURL());
+        throttler.handleHttpErrorCodes(429);
+        throttler.handleHttpErrorCodes(429);
+        throttler.handleHttpErrorCodes(503);
+        throttler.handleHttpErrorCodes(429);
+        assertThat(throttler.getSkips()).isEqualTo(4);
+        assertThat(throttler.getFailures()).isEqualTo(4);
+        throttler.decrementFailureCountAndResetSkips();
+        assertThat(throttler.getSkips()).isEqualTo(3);
+        assertThat(throttler.getFailures()).isEqualTo(3);
+        throttler.handleHttpErrorCodes(429);
+        assertThat(throttler.getSkips()).isEqualTo(4);
+        assertThat(throttler.getFailures()).isEqualTo(4);
+        throttler.decrementFailureCountAndResetSkips();
+        throttler.decrementFailureCountAndResetSkips();
+        throttler.decrementFailureCountAndResetSkips();
+        throttler.decrementFailureCountAndResetSkips();
+        throttler.decrementFailureCountAndResetSkips();
+        throttler.decrementFailureCountAndResetSkips();
+        throttler.decrementFailureCountAndResetSkips();
+        assertThat(throttler.getSkips()).isEqualTo(0);
+        assertThat(throttler.getFailures()).isEqualTo(0);
+    }
+}