fix: setProviderAndWait must throw (#794)

Signed-off-by: Todd Baert <[email protected]>
open-feature · Feb 8, 2024 · da47b7f · da47b7f
1 parent d5a0620
commit da47b7f
Show file tree

Hide file tree

Showing 6 changed files with 60 additions and 34 deletions.
diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java
@@ -9,6 +9,7 @@
 
 import javax.annotation.Nullable;
 
+import dev.openfeature.sdk.exceptions.OpenFeatureError;
 import dev.openfeature.sdk.internal.AutoCloseableLock;
 import dev.openfeature.sdk.internal.AutoCloseableReentrantReadWriteLock;
 import lombok.extern.slf4j.Slf4j;
@@ -131,14 +132,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 OpenFeatureError {
         try (AutoCloseableLock __ = lock.writeLockAutoCloseable()) {
             providerRepository.setProvider(
                     provider,
                     this::attachEventProvider,
                     this::emitReady,
                     this::detachEventProvider,
-                    this::emitError,
+                    this::emitErrorAndThrow,
                     true);
         }
     }
@@ -149,14 +150,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 OpenFeatureError {
         try (AutoCloseableLock __ = lock.writeLockAutoCloseable()) {
             providerRepository.setProvider(clientName,
                     provider,
                     this::attachEventProvider,
                     this::emitReady,
                     this::detachEventProvider,
-                    this::emitError,
+                    this::emitErrorAndThrow,
                     true);
         }
     }
@@ -179,9 +180,14 @@ private void detachEventProvider(FeatureProvider provider) {
         }
     }
 
-    private void emitError(FeatureProvider provider, String message) {
+    private void emitError(FeatureProvider provider, OpenFeatureError exception) {
         runHandlersForProvider(provider, ProviderEvent.PROVIDER_ERROR,
-                ProviderEventDetails.builder().message(message).build());
+                ProviderEventDetails.builder().message(exception.getMessage()).build());
+    }
+
+    private void emitErrorAndThrow(FeatureProvider provider, OpenFeatureError exception) throws OpenFeatureError {
+        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
@@ -15,6 +15,8 @@
 
 import javax.annotation.Nullable;
 
+import dev.openfeature.sdk.exceptions.GeneralError;
+import dev.openfeature.sdk.exceptions.OpenFeatureError;
 import lombok.extern.slf4j.Slf4j;
 
 @Slf4j
@@ -66,7 +68,7 @@ public void setProvider(FeatureProvider provider,
             Consumer<FeatureProvider> afterSet,
             Consumer<FeatureProvider> afterInit,
             Consumer<FeatureProvider> afterShutdown,
-            BiConsumer<FeatureProvider, String> afterError,
+            BiConsumer<FeatureProvider, OpenFeatureError> afterError,
             boolean waitForInit) {
         if (provider == null) {
             throw new IllegalArgumentException("Provider cannot be null");
@@ -83,12 +85,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, OpenFeatureError> afterError,
+            boolean waitForInit) {
         if (provider == null) {
             throw new IllegalArgumentException("Provider cannot be null");
         }
@@ -103,7 +105,7 @@ private void prepareAndInitializeProvider(@Nullable String clientName,
               Consumer<FeatureProvider> afterSet,
               Consumer<FeatureProvider> afterInit,
               Consumer<FeatureProvider> afterShutdown,
-              BiConsumer<FeatureProvider, String> afterError,
+              BiConsumer<FeatureProvider, OpenFeatureError> afterError,
               boolean waitForInit) {
 
         if (!isProviderRegistered(newProvider)) {
@@ -129,17 +131,20 @@ private void prepareAndInitializeProvider(@Nullable String clientName,
     private void initializeProvider(FeatureProvider newProvider,
             Consumer<FeatureProvider> afterInit,
             Consumer<FeatureProvider> afterShutdown,
-            BiConsumer<FeatureProvider, String> afterError,
+            BiConsumer<FeatureProvider, OpenFeatureError> afterError,
             FeatureProvider oldProvider) {
         try {
             if (ProviderState.NOT_READY.equals(newProvider.getState())) {
                 newProvider.initialize(OpenFeatureAPI.getInstance().getEvaluationContext());
                 afterInit.accept(newProvider);
             }
             shutDownOld(oldProvider, afterShutdown);
+        } catch (OpenFeatureError 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 GeneralError(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;
@@ -18,9 +19,6 @@
 import java.util.List;
 import java.util.Map;
 
-import dev.openfeature.sdk.providers.memory.InMemoryProvider;
-import dev.openfeature.sdk.testutils.TestEventsProvider;
-import lombok.SneakyThrows;
 import org.awaitility.Awaitility;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
@@ -31,8 +29,12 @@
 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.providers.memory.InMemoryProvider;
 import dev.openfeature.sdk.testutils.FeatureProviderTestUtils;
+import dev.openfeature.sdk.testutils.TestEventsProvider;
+import lombok.SneakyThrows;
 
 class FlagEvaluationSpecTest implements HookFixtures {
 
@@ -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
@@ -1,16 +1,17 @@
 package dev.openfeature.sdk;
 
-import dev.openfeature.sdk.providers.memory.InMemoryProvider;
-import dev.openfeature.sdk.testutils.FeatureProviderTestUtils;
-import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.Test;
-
-import java.util.Collections;
-
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatCode;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 
+import java.util.Collections;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import dev.openfeature.sdk.providers.memory.InMemoryProvider;
+import dev.openfeature.sdk.testutils.FeatureProviderTestUtils;
+
 class OpenFeatureAPITest {
 
     private static final String CLIENT_NAME = "client name";
@@ -45,7 +46,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
@@ -9,7 +9,6 @@
 import static org.awaitility.Awaitility.await;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.eq;
-import static org.mockito.Mockito.atMostOnce;
 import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
@@ -29,6 +28,7 @@
 import org.junit.jupiter.api.Nested;
 import org.junit.jupiter.api.Test;
 
+import dev.openfeature.sdk.exceptions.OpenFeatureError;
 import dev.openfeature.sdk.testutils.exception.TestException;
 
 class ProviderRepositoryTest {
@@ -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, OpenFeatureError> 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, OpenFeatureError> 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, OpenFeatureError> 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, OpenFeatureError> 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;
         }