Repository: Unleash/unleash-client-java
Pull request #196 (2023-03-30 08:54:25)

fix: synchronous fetch on initialization
## About the changes
When `synchronousFetchOnInitialization` is enabled, we should fail synchronously if the initial fetch fails.

Currently, we're swallowing the exception

There are a few sonatype-lift recommendations which seem reasonable, but I'm not sure I'd like to double check with @chriswk 

sonatype-lift[bot]: ## 🛠 Lift Auto-fix
Some of the Lift findings in this PR can be automatically fixed. You can download and apply these changes in your local project directory of your branch to review the suggestions before committing.[^1]
```bash
# Download the patch
curl https://lift.sonatype.com/api/patch/github.com/Unleash/unleash-client-java/196.diff -o lift-autofixes.diff

# Apply the patch with git
git apply lift-autofixes.diff

# Review the changes
git diff
```
Want it all in a single command? Open a terminal in your project's directory and copy and paste the following command:
```bash
curl https://lift.sonatype.com/api/patch/github.com/Unleash/unleash-client-java/196.diff | git apply
```
Once you're satisfied, commit and push your changes in your project.
[^1]: You can preview the patch by opening the [patch URL](https://lift.sonatype.com/api/patch/github.com/Unleash/unleash-client-java/196.diff) in the browser.

Patch:
diff --git a/examples/cli-example/build.gradle.kts b/examples/cli-example/build.gradle.kts
index e98b4a82..f5bc3f97 100644
--- a/examples/cli-example/build.gradle.kts
+++ b/examples/cli-example/build.gradle.kts
@@ -13,5 +13,5 @@ repositories {
 }
 
 dependencies {
-    implementation("io.getunleash:unleash-client-java:6.0.1")
+    implementation("io.getunleash:unleash-client-java:7.1.0")
 }
diff --git a/src/main/java/io/getunleash/repository/FeatureRepository.java b/src/main/java/io/getunleash/repository/FeatureRepository.java
index 248b10f6..e826b948 100644
--- a/src/main/java/io/getunleash/repository/FeatureRepository.java
+++ b/src/main/java/io/getunleash/repository/FeatureRepository.java
@@ -10,6 +10,7 @@ import io.getunleash.util.UnleashConfig;
 import io.getunleash.util.UnleashScheduledExecutor;
 import java.util.Collections;
 import java.util.List;
+import java.util.function.Consumer;
 import java.util.stream.Collectors;
 
 public class FeatureRepository implements IFeatureRepository {
@@ -68,6 +69,7 @@ public class FeatureRepository implements IFeatureRepository {
         this.initCollections(executor);
     }
 
+    @SuppressWarnings("FutureReturnValueIgnored")
     private void initCollections(UnleashScheduledExecutor executor) {
         this.featureCollection = this.featureBackupHandler.read();
         if (this.featureCollection.getToggleCollection().getFeatures().isEmpty()) {
@@ -75,18 +77,20 @@ public class FeatureRepository implements IFeatureRepository {
         }
 
         if (unleashConfig.isSynchronousFetchOnInitialisation()) {
-            updateFeatures().run();
+            updateFeatures(null).run();
         }
+
         if (!unleashConfig.isDisablePolling()) {
+            Runnable updateFeatures = updateFeatures(this.eventDispatcher::dispatch);
             if (unleashConfig.getFetchTogglesInterval() > 0) {
-                executor.setInterval(updateFeatures(), 0, unleashConfig.getFetchTogglesInterval());
+                executor.setInterval(updateFeatures, 0, unleashConfig.getFetchTogglesInterval());
             } else {
-                executor.scheduleOnce(updateFeatures());
+                executor.scheduleOnce(updateFeatures);
             }
         }
     }
 
-    private Runnable updateFeatures() {
+    private Runnable updateFeatures(@Nullable final Consumer<UnleashException> handler) {
         return () -> {
             try {
                 ClientFeaturesResponse response = featureFetcher.fetchFeatures();
@@ -108,7 +112,11 @@ public class FeatureRepository implements IFeatureRepository {
                     ready = true;
                 }
             } catch (UnleashException e) {
-                eventDispatcher.dispatch(e);
+                if (handler != null) {
+                    handler.accept(e);
+                } else {
+                    throw e;
+                }
             }
         };
     }


Test patch:
diff --git a/src/test/java/io/getunleash/DefaultUnleashTest.java b/src/test/java/io/getunleash/DefaultUnleashTest.java
index 33140a31..3faddf89 100644
--- a/src/test/java/io/getunleash/DefaultUnleashTest.java
+++ b/src/test/java/io/getunleash/DefaultUnleashTest.java
@@ -9,17 +9,23 @@ import ch.qos.logback.classic.Logger;
 import ch.qos.logback.classic.spi.ILoggingEvent;
 import ch.qos.logback.core.read.ListAppender;
 import io.getunleash.event.EventDispatcher;
+import io.getunleash.metric.MetricSender;
 import io.getunleash.metric.UnleashMetricService;
-import io.getunleash.repository.FeatureRepository;
+import io.getunleash.repository.*;
 import io.getunleash.strategy.DefaultStrategy;
 import io.getunleash.strategy.Strategy;
+import io.getunleash.util.MetricSenderFactory;
 import io.getunleash.util.UnleashConfig;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+
+import io.getunleash.util.UnleashFeatureFetcherFactory;
+import io.getunleash.util.UnleashScheduledExecutor;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.mockito.ArgumentCaptor;
 import org.slf4j.LoggerFactory;
 
 class DefaultUnleashTest {
@@ -211,6 +217,45 @@ class DefaultUnleashTest {
                                 + "] running. Please double check your code where you are instantiating the Unleash SDK");
     }
 
+    @Test
+    public void synchronous_fetch_on_initialisation_fails_on_initialization() {
+        UnleashConfig config =
+            UnleashConfig.builder()
+                .unleashAPI("http://wrong:4242")
+                .appName("wrong_upstream")
+                .apiKey("default:development:1234567890123456")
+                .instanceId("multiple_connection_exception")
+                .synchronousFetchOnInitialisation(true)
+                .build();
+
+        assertThatThrownBy(() -> new DefaultUnleash(config)).isInstanceOf(UnleashException.class);
+    }
+
+    @Test
+    public void asynchronous_fetch_on_initialisation_fails_silently_and_retries() throws InterruptedException {
+        FeatureFetcher fetcher = mock(FeatureFetcher.class);
+        FeatureCollection expectedResponse = new FeatureCollection();
+        FeatureToggleResponse.Status expectedStatus = FeatureToggleResponse.Status.CHANGED;
+        when(fetcher.fetchFeatures())
+            .thenThrow(UnleashException.class)
+            .thenReturn(new ClientFeaturesResponse(expectedStatus, expectedResponse));
+
+        UnleashConfig config =
+            UnleashConfig.builder()
+                .unleashAPI("http://wrong:4242")
+                .appName("wrong_upstream")
+                .apiKey("default:development:1234567890123456")
+                .instanceId("multiple_connection_exception")
+                .fetchTogglesInterval(1)
+                .unleashFeatureFetcherFactory((UnleashConfig c) -> fetcher)
+                .build();
+
+        Unleash unleash = new DefaultUnleash(config);
+        verify(fetcher, times(1)).fetchFeatures();
+        Thread.sleep(1200);
+        verify(fetcher, times(2)).fetchFeatures();
+    }
+
     @Test
     public void client_identifier_handles_api_key_being_null() {
         UnleashConfig config =
diff --git a/src/test/java/io/getunleash/example/UnleashUsageTest.java b/src/test/java/io/getunleash/example/UnleashUsageTest.java
index a8598285..1d325288 100644
--- a/src/test/java/io/getunleash/example/UnleashUsageTest.java
+++ b/src/test/java/io/getunleash/example/UnleashUsageTest.java
@@ -15,7 +15,7 @@ public class UnleashUsageTest {
                 new UnleashConfig.Builder()
                         .appName("test")
                         .instanceId("my-hostname:6517")
-                        .synchronousFetchOnInitialisation(true)
+                        .synchronousFetchOnInitialisation(false)
                         .unleashAPI("http://localhost:4242/api")
                         .build();
 
diff --git a/src/test/java/io/getunleash/repository/FeatureToggleRepositoryTest.java b/src/test/java/io/getunleash/repository/FeatureToggleRepositoryTest.java
index 3f24dcfe..318e3fe4 100644
--- a/src/test/java/io/getunleash/repository/FeatureToggleRepositoryTest.java
+++ b/src/test/java/io/getunleash/repository/FeatureToggleRepositoryTest.java
@@ -1,6 +1,7 @@
 package io.getunleash.repository;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.junit.jupiter.api.Assertions.*;
 import static org.mockito.Mockito.*;
 
@@ -156,6 +157,27 @@ public class FeatureToggleRepositoryTest {
         verify(toggleFetcher, times(1)).fetchToggles();
     }
 
+    @Test
+    public void should_perform_synchronous_fetch_on_initialisation_and_fail_if_invalid_upstream() {
+        UnleashConfig config =
+                UnleashConfig.builder()
+                        .synchronousFetchOnInitialisation(true)
+                        .appName("test-sync-update")
+                        .unleashAPI("http://wrong-host:8383")
+                        .build();
+        UnleashScheduledExecutor executor = mock(UnleashScheduledExecutor.class);
+        ToggleFetcher toggleFetcher = mock(ToggleFetcher.class);
+        BackupHandler<ToggleCollection> toggleBackupHandler = mock(BackupHandler.class);
+        when(toggleBackupHandler.read()).thenReturn(new ToggleCollection(Collections.emptyList()));
+
+        // setup fetcher
+        when(toggleFetcher.fetchToggles()).thenThrow(RuntimeException.class);
+
+        assertThatThrownBy(() -> {
+            new FeatureToggleRepository(config, executor, toggleFetcher, toggleBackupHandler);
+        }).isInstanceOf(RuntimeException.class);
+    }
+
     @Test
     public void should_not_perform_synchronous_fetch_on_initialisation() {
         UnleashConfig config =