fix: setProviderAndWait must throw

toddbaert · toddbaert · commit ee1d18bebb7d · 2024-02-07T15:01:14.000-05:00
Signed-off-by: Todd Baert &lt;todd.baert@dynatrace.com&gt;
diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java
@@ -131,14 +131,14 @@ public void setProvider(String clientName, FeatureProvider provider) {
     /**
      * Set the default provider and wait for initialization to finish.
      */
-    public void setProviderAndWait(FeatureProvider provider) {
+    public void setProviderAndWait(FeatureProvider provider) throws RuntimeException {
         try (AutoCloseableLock __ = lock.writeLockAutoCloseable()) {
             providerRepository.setProvider(
                     provider,
                     this::attachEventProvider,
                     this::emitReady,
                     this::detachEventProvider,
-                    this::emitError,
+                    this::emitErrorAndThrow,
                     true);
         }
     }
@@ -149,14 +149,14 @@ public void setProviderAndWait(FeatureProvider provider) {
      * @param clientName The name of the client.
      * @param provider   The provider to set.
      */
-    public void setProviderAndWait(String clientName, FeatureProvider provider) {
+    public void setProviderAndWait(String clientName, FeatureProvider provider) throws RuntimeException {
         try (AutoCloseableLock __ = lock.writeLockAutoCloseable()) {
             providerRepository.setProvider(clientName,
                     provider,
                     this::attachEventProvider,
                     this::emitReady,
                     this::detachEventProvider,
-                    this::emitError,
+                    this::emitErrorAndThrow,
                     true);
         }
     }
@@ -179,9 +179,14 @@ private void detachEventProvider(FeatureProvider provider) {
         }
     }
 
-    private void emitError(FeatureProvider provider, String message) {
+    private void emitError(FeatureProvider provider, Exception exception) {
         runHandlersForProvider(provider, ProviderEvent.PROVIDER_ERROR,
-                ProviderEventDetails.builder().message(message).build());
+                ProviderEventDetails.builder().message(exception.getMessage()).build());
+    }
+
+    private void emitErrorAndThrow(FeatureProvider provider, RuntimeException exception) throws RuntimeException {
+        this.emitError(provider, exception);
+        throw exception;
     }
 
     /**
diff --git a/src/main/java/dev/openfeature/sdk/ProviderRepository.java b/src/main/java/dev/openfeature/sdk/ProviderRepository.java
@@ -66,7 +66,7 @@ public void setProvider(FeatureProvider provider,
             Consumer<FeatureProvider> afterSet,
             Consumer<FeatureProvider> afterInit,
             Consumer<FeatureProvider> afterShutdown,
-            BiConsumer<FeatureProvider, String> afterError,
+            BiConsumer<FeatureProvider, RuntimeException> afterError,
             boolean waitForInit) {
         if (provider == null) {
             throw new IllegalArgumentException("Provider cannot be null");
@@ -83,12 +83,12 @@ public void setProvider(FeatureProvider provider,
      *                    Otherwise, initialization happens in the background.
      */
     public void setProvider(String clientName,
-             FeatureProvider provider,
-             Consumer<FeatureProvider> afterSet,
-             Consumer<FeatureProvider> afterInit,
-             Consumer<FeatureProvider> afterShutdown,
-             BiConsumer<FeatureProvider, String> afterError,
-             boolean waitForInit) {
+            FeatureProvider provider,
+            Consumer<FeatureProvider> afterSet,
+            Consumer<FeatureProvider> afterInit,
+            Consumer<FeatureProvider> afterShutdown,
+            BiConsumer<FeatureProvider, RuntimeException> afterError,
+            boolean waitForInit) {
         if (provider == null) {
             throw new IllegalArgumentException("Provider cannot be null");
         }
@@ -103,7 +103,7 @@ private void prepareAndInitializeProvider(@Nullable String clientName,
               Consumer<FeatureProvider> afterSet,
               Consumer<FeatureProvider> afterInit,
               Consumer<FeatureProvider> afterShutdown,
-              BiConsumer<FeatureProvider, String> afterError,
+              BiConsumer<FeatureProvider, RuntimeException> afterError,
               boolean waitForInit) {
 
         if (!isProviderRegistered(newProvider)) {
@@ -129,17 +129,20 @@ private void prepareAndInitializeProvider(@Nullable String clientName,
     private void initializeProvider(FeatureProvider newProvider,
             Consumer<FeatureProvider> afterInit,
             Consumer<FeatureProvider> afterShutdown,
-            BiConsumer<FeatureProvider, String> afterError,
+            BiConsumer<FeatureProvider, RuntimeException> afterError,
             FeatureProvider oldProvider) {
         try {
             if (ProviderState.NOT_READY.equals(newProvider.getState())) {
                 newProvider.initialize(OpenFeatureAPI.getInstance().getEvaluationContext());
                 afterInit.accept(newProvider);
             }
             shutDownOld(oldProvider, afterShutdown);
+        } catch (RuntimeException e) {
+            log.error("Exception when initializing feature provider {}", newProvider.getClass().getName(), e);
+            afterError.accept(newProvider, e);
         } catch (Exception e) {
             log.error("Exception when initializing feature provider {}", newProvider.getClass().getName(), e);
-            afterError.accept(newProvider, e.getMessage());
+            afterError.accept(newProvider, new RuntimeException(e));
         }
     }
 
diff --git a/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java b/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java
@@ -8,6 +8,7 @@
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertSame;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
@@ -31,6 +32,7 @@
 import org.slf4j.Logger;
 
 import dev.openfeature.sdk.exceptions.FlagNotFoundError;
+import dev.openfeature.sdk.exceptions.GeneralError;
 import dev.openfeature.sdk.fixtures.HookFixtures;
 import dev.openfeature.sdk.testutils.FeatureProviderTestUtils;
 
@@ -87,6 +89,17 @@ void getApiInstance() {
         assertThat(api.getProvider(providerName).getState()).isEqualTo(ProviderState.READY);
     }
 
+    @SneakyThrows
+    @Specification(number="1.1.8", text="The API SHOULD provide functions to set a provider and wait for the initialize function to return or throw.")
+    @Test void providerAndWaitError() {
+        FeatureProvider provider1 = new TestEventsProvider(500, true, "fake error");
+        assertThrows(GeneralError.class, () -> api.setProviderAndWait(provider1));
+
+        FeatureProvider provider2 = new TestEventsProvider(500, true, "fake error");
+        String providerName = "providerAndWaitError";
+        assertThrows(GeneralError.class, () -> api.setProviderAndWait(providerName, provider2));
+    }
+
     @Specification(number="2.4.5", text="The provider SHOULD indicate an error if flag resolution is attempted before the provider is ready.")
     @Test void shouldReturnNotReadyIfNotInitialized() {
         FeatureProvider provider = new InMemoryProvider(new HashMap<>()) {
diff --git a/src/test/java/dev/openfeature/sdk/OpenFeatureAPITest.java b/src/test/java/dev/openfeature/sdk/OpenFeatureAPITest.java
@@ -45,7 +45,7 @@ void namedProviderOverwrittenTest() {
     }
 
     @Test
-    void providerToMultipleNames() {
+    void providerToMultipleNames() throws Exception {
         FeatureProvider inMemAsEventingProvider = new InMemoryProvider(Collections.EMPTY_MAP);
         FeatureProvider noOpAsNonEventingProvider = new NoOpProvider();
 
diff --git a/src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java b/src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java
@@ -253,7 +253,7 @@ void shouldRunLambdasOnSuccessful() {
                 Consumer<FeatureProvider> afterSet = mock(Consumer.class);
                 Consumer<FeatureProvider> afterInit = mock(Consumer.class);
                 Consumer<FeatureProvider> afterShutdown = mock(Consumer.class);
-                BiConsumer<FeatureProvider, String> afterError = mock(BiConsumer.class);
+                BiConsumer<FeatureProvider, RuntimeException> afterError = mock(BiConsumer.class);
         
                 FeatureProvider oldProvider = providerRepository.getProvider();
                 FeatureProvider featureProvider1 = createMockedProvider();
@@ -274,7 +274,7 @@ void shouldRunLambdasOnError() throws Exception {
                 Consumer<FeatureProvider> afterSet = mock(Consumer.class);
                 Consumer<FeatureProvider> afterInit = mock(Consumer.class);
                 Consumer<FeatureProvider> afterShutdown = mock(Consumer.class);
-                BiConsumer<FeatureProvider, String> afterError = mock(BiConsumer.class);
+                BiConsumer<FeatureProvider, RuntimeException> afterError = mock(BiConsumer.class);
         
                 FeatureProvider errorFeatureProvider = createMockedErrorProvider();
         
@@ -310,7 +310,7 @@ private void setFeatureProvider(FeatureProvider provider) {
 
     private void setFeatureProvider(FeatureProvider provider, Consumer<FeatureProvider> afterSet,
             Consumer<FeatureProvider> afterInit, Consumer<FeatureProvider> afterShutdown,
-            BiConsumer<FeatureProvider, String> afterError) {
+            BiConsumer<FeatureProvider, RuntimeException> afterError) {
         providerRepository.setProvider(provider, afterSet, afterInit, afterShutdown,
                 afterError, false);
         waitForSettingProviderHasBeenCompleted(ProviderRepository::getProvider, provider);
@@ -348,8 +348,8 @@ private Consumer<FeatureProvider> mockAfterShutdown() {
         };
     }
 
-    private BiConsumer<FeatureProvider, String> mockAfterError() {
-        return (fp, message) -> {
+    private BiConsumer<FeatureProvider, RuntimeException> mockAfterError() {
+        return (fp, ex) -> {
         };
     }
 
diff --git a/src/test/java/dev/openfeature/sdk/testutils/TestEventsProvider.java b/src/test/java/dev/openfeature/sdk/testutils/TestEventsProvider.java
@@ -8,6 +8,7 @@
 import dev.openfeature.sdk.ProviderEventDetails;
 import dev.openfeature.sdk.ProviderState;
 import dev.openfeature.sdk.Value;
+import dev.openfeature.sdk.exceptions.GeneralError;
 
 public class TestEventsProvider extends EventProvider {
 
@@ -63,7 +64,7 @@ public void initialize(EvaluationContext evaluationContext) throws Exception {
             Thread.sleep(initTimeoutMs);
             if (this.initError) {
                 this.state = ProviderState.ERROR;
-                throw new Exception(initErrorMessage);
+                throw new GeneralError(initErrorMessage);
             }
             this.state = ProviderState.READY;
         }

Original file line number	Diff line number	Diff line change
`@@ -45,7 +45,7 @@ void namedProviderOverwrittenTest() {`
`45`	`45`	`}`
`46`	`46`
`47`	`47`	`@Test`
`48`		`- void providerToMultipleNames() {`
	`48`	`+ void providerToMultipleNames() throws Exception {`
`49`	`49`	`FeatureProvider inMemAsEventingProvider = new InMemoryProvider(Collections.EMPTY_MAP);`
`50`	`50`	`FeatureProvider noOpAsNonEventingProvider = new NoOpProvider();`
`51`	`51`
Original file line number	Diff line number	Diff line change
`@@ -8,6 +8,7 @@`
`8`	`8`	`import dev.openfeature.sdk.ProviderEventDetails;`
`9`	`9`	`import dev.openfeature.sdk.ProviderState;`
`10`	`10`	`import dev.openfeature.sdk.Value;`
	`11`	`+import dev.openfeature.sdk.exceptions.GeneralError;`
`11`	`12`
`12`	`13`	`public class TestEventsProvider extends EventProvider {`
`13`	`14`
`@@ -63,7 +64,7 @@ public void initialize(EvaluationContext evaluationContext) throws Exception {`
`63`	`64`	`Thread.sleep(initTimeoutMs);`
`64`	`65`	`if (this.initError) {`
`65`	`66`	`this.state = ProviderState.ERROR;`
`66`		`- throw new Exception(initErrorMessage);`
	`67`	`+ throw new GeneralError(initErrorMessage);`
`67`	`68`	`}`
`68`	`69`	`this.state = ProviderState.READY;`
`69`	`70`	`}`