Skip to content

Commit da47b7f

Browse files
toddbaerttoddbaert
authored
fix: setProviderAndWait must throw (open-feature#794)
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
1 parent d5a0620 commit da47b7f

File tree

6 files changed

+60
-34
lines changed

6 files changed

+60
-34
lines changed

‎src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
import javax.annotation.Nullable;
1111

12+
import dev.openfeature.sdk.exceptions.OpenFeatureError;
1213
import dev.openfeature.sdk.internal.AutoCloseableLock;
1314
import dev.openfeature.sdk.internal.AutoCloseableReentrantReadWriteLock;
1415
import lombok.extern.slf4j.Slf4j;
@@ -131,14 +132,14 @@ public void setProvider(String clientName, FeatureProvider provider) {
131132
/**
132133
* Set the default provider and wait for initialization to finish.
133134
*/
134-
public void setProviderAndWait(FeatureProvider provider) {
135+
public void setProviderAndWait(FeatureProvider provider) throws OpenFeatureError {
135136
try (AutoCloseableLock __ = lock.writeLockAutoCloseable()) {
136137
providerRepository.setProvider(
137138
provider,
138139
this::attachEventProvider,
139140
this::emitReady,
140141
this::detachEventProvider,
141-
this::emitError,
142+
this::emitErrorAndThrow,
142143
true);
143144
}
144145
}
@@ -149,14 +150,14 @@ public void setProviderAndWait(FeatureProvider provider) {
149150
* @param clientName The name of the client.
150151
* @param provider The provider to set.
151152
*/
152-
public void setProviderAndWait(String clientName, FeatureProvider provider) {
153+
public void setProviderAndWait(String clientName, FeatureProvider provider) throws OpenFeatureError {
153154
try (AutoCloseableLock __ = lock.writeLockAutoCloseable()) {
154155
providerRepository.setProvider(clientName,
155156
provider,
156157
this::attachEventProvider,
157158
this::emitReady,
158159
this::detachEventProvider,
159-
this::emitError,
160+
this::emitErrorAndThrow,
160161
true);
161162
}
162163
}
@@ -179,9 +180,14 @@ private void detachEventProvider(FeatureProvider provider) {
179180
}
180181
}
181182

182-
private void emitError(FeatureProvider provider, String message) {
183+
private void emitError(FeatureProvider provider, OpenFeatureError exception) {
183184
runHandlersForProvider(provider, ProviderEvent.PROVIDER_ERROR,
184-
ProviderEventDetails.builder().message(message).build());
185+
ProviderEventDetails.builder().message(exception.getMessage()).build());
186+
}
187+
188+
private void emitErrorAndThrow(FeatureProvider provider, OpenFeatureError exception) throws OpenFeatureError {
189+
this.emitError(provider, exception);
190+
throw exception;
185191
}
186192

187193
/**

‎src/main/java/dev/openfeature/sdk/ProviderRepository.java

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515

1616
import javax.annotation.Nullable;
1717

18+
import dev.openfeature.sdk.exceptions.GeneralError;
19+
import dev.openfeature.sdk.exceptions.OpenFeatureError;
1820
import lombok.extern.slf4j.Slf4j;
1921

2022
@Slf4j
@@ -66,7 +68,7 @@ public void setProvider(FeatureProvider provider,
6668
Consumer<FeatureProvider> afterSet,
6769
Consumer<FeatureProvider> afterInit,
6870
Consumer<FeatureProvider> afterShutdown,
69-
BiConsumer<FeatureProvider, String> afterError,
71+
BiConsumer<FeatureProvider, OpenFeatureError> afterError,
7072
boolean waitForInit) {
7173
if (provider == null) {
7274
throw new IllegalArgumentException("Provider cannot be null");
@@ -83,12 +85,12 @@ public void setProvider(FeatureProvider provider,
8385
* Otherwise, initialization happens in the background.
8486
*/
8587
public void setProvider(String clientName,
86-
FeatureProvider provider,
87-
Consumer<FeatureProvider> afterSet,
88-
Consumer<FeatureProvider> afterInit,
89-
Consumer<FeatureProvider> afterShutdown,
90-
BiConsumer<FeatureProvider, String> afterError,
91-
boolean waitForInit) {
88+
FeatureProvider provider,
89+
Consumer<FeatureProvider> afterSet,
90+
Consumer<FeatureProvider> afterInit,
91+
Consumer<FeatureProvider> afterShutdown,
92+
BiConsumer<FeatureProvider, OpenFeatureError> afterError,
93+
boolean waitForInit) {
9294
if (provider == null) {
9395
throw new IllegalArgumentException("Provider cannot be null");
9496
}
@@ -103,7 +105,7 @@ private void prepareAndInitializeProvider(@Nullable String clientName,
103105
Consumer<FeatureProvider> afterSet,
104106
Consumer<FeatureProvider> afterInit,
105107
Consumer<FeatureProvider> afterShutdown,
106-
BiConsumer<FeatureProvider, String> afterError,
108+
BiConsumer<FeatureProvider, OpenFeatureError> afterError,
107109
boolean waitForInit) {
108110

109111
if (!isProviderRegistered(newProvider)) {
@@ -129,17 +131,20 @@ private void prepareAndInitializeProvider(@Nullable String clientName,
129131
private void initializeProvider(FeatureProvider newProvider,
130132
Consumer<FeatureProvider> afterInit,
131133
Consumer<FeatureProvider> afterShutdown,
132-
BiConsumer<FeatureProvider, String> afterError,
134+
BiConsumer<FeatureProvider, OpenFeatureError> afterError,
133135
FeatureProvider oldProvider) {
134136
try {
135137
if (ProviderState.NOT_READY.equals(newProvider.getState())) {
136138
newProvider.initialize(OpenFeatureAPI.getInstance().getEvaluationContext());
137139
afterInit.accept(newProvider);
138140
}
139141
shutDownOld(oldProvider, afterShutdown);
142+
} catch (OpenFeatureError e) {
143+
log.error("Exception when initializing feature provider {}", newProvider.getClass().getName(), e);
144+
afterError.accept(newProvider, e);
140145
} catch (Exception e) {
141146
log.error("Exception when initializing feature provider {}", newProvider.getClass().getName(), e);
142-
afterError.accept(newProvider, e.getMessage());
147+
afterError.accept(newProvider, new GeneralError(e));
143148
}
144149
}
145150

‎src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import static org.junit.jupiter.api.Assertions.assertNotNull;
99
import static org.junit.jupiter.api.Assertions.assertNull;
1010
import static org.junit.jupiter.api.Assertions.assertSame;
11+
import static org.junit.jupiter.api.Assertions.assertThrows;
1112
import static org.junit.jupiter.api.Assertions.assertTrue;
1213
import static org.mockito.ArgumentMatchers.any;
1314
import static org.mockito.Mockito.mock;
@@ -18,9 +19,6 @@
1819
import java.util.List;
1920
import java.util.Map;
2021

21-
import dev.openfeature.sdk.providers.memory.InMemoryProvider;
22-
import dev.openfeature.sdk.testutils.TestEventsProvider;
23-
import lombok.SneakyThrows;
2422
import org.awaitility.Awaitility;
2523
import org.junit.jupiter.api.AfterEach;
2624
import org.junit.jupiter.api.BeforeEach;
@@ -31,8 +29,12 @@
3129
import org.slf4j.Logger;
3230

3331
import dev.openfeature.sdk.exceptions.FlagNotFoundError;
32+
import dev.openfeature.sdk.exceptions.GeneralError;
3433
import dev.openfeature.sdk.fixtures.HookFixtures;
34+
import dev.openfeature.sdk.providers.memory.InMemoryProvider;
3535
import dev.openfeature.sdk.testutils.FeatureProviderTestUtils;
36+
import dev.openfeature.sdk.testutils.TestEventsProvider;
37+
import lombok.SneakyThrows;
3638

3739
class FlagEvaluationSpecTest implements HookFixtures {
3840

@@ -87,6 +89,17 @@ void getApiInstance() {
8789
assertThat(api.getProvider(providerName).getState()).isEqualTo(ProviderState.READY);
8890
}
8991

92+
@SneakyThrows
93+
@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.")
94+
@Test void providerAndWaitError() {
95+
FeatureProvider provider1 = new TestEventsProvider(500, true, "fake error");
96+
assertThrows(GeneralError.class, () -> api.setProviderAndWait(provider1));
97+
98+
FeatureProvider provider2 = new TestEventsProvider(500, true, "fake error");
99+
String providerName = "providerAndWaitError";
100+
assertThrows(GeneralError.class, () -> api.setProviderAndWait(providerName, provider2));
101+
}
102+
90103
@Specification(number="2.4.5", text="The provider SHOULD indicate an error if flag resolution is attempted before the provider is ready.")
91104
@Test void shouldReturnNotReadyIfNotInitialized() {
92105
FeatureProvider provider = new InMemoryProvider(new HashMap<>()) {

‎src/test/java/dev/openfeature/sdk/OpenFeatureAPITest.java

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,17 @@
11
package dev.openfeature.sdk;
22

3-
import dev.openfeature.sdk.providers.memory.InMemoryProvider;
4-
import dev.openfeature.sdk.testutils.FeatureProviderTestUtils;
5-
import org.junit.jupiter.api.BeforeEach;
6-
import org.junit.jupiter.api.Test;
7-
8-
import java.util.Collections;
9-
103
import static org.assertj.core.api.Assertions.assertThat;
114
import static org.assertj.core.api.Assertions.assertThatCode;
125
import static org.junit.jupiter.api.Assertions.assertEquals;
136

7+
import java.util.Collections;
8+
9+
import org.junit.jupiter.api.BeforeEach;
10+
import org.junit.jupiter.api.Test;
11+
12+
import dev.openfeature.sdk.providers.memory.InMemoryProvider;
13+
import dev.openfeature.sdk.testutils.FeatureProviderTestUtils;
14+
1415
class OpenFeatureAPITest {
1516

1617
private static final String CLIENT_NAME = "client name";
@@ -45,7 +46,7 @@ void namedProviderOverwrittenTest() {
4546
}
4647

4748
@Test
48-
void providerToMultipleNames() {
49+
void providerToMultipleNames() throws Exception {
4950
FeatureProvider inMemAsEventingProvider = new InMemoryProvider(Collections.EMPTY_MAP);
5051
FeatureProvider noOpAsNonEventingProvider = new NoOpProvider();
5152

‎src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
import static org.awaitility.Awaitility.await;
1010
import static org.mockito.ArgumentMatchers.any;
1111
import static org.mockito.ArgumentMatchers.eq;
12-
import static org.mockito.Mockito.atMostOnce;
1312
import static org.mockito.Mockito.doThrow;
1413
import static org.mockito.Mockito.mock;
1514
import static org.mockito.Mockito.never;
@@ -29,6 +28,7 @@
2928
import org.junit.jupiter.api.Nested;
3029
import org.junit.jupiter.api.Test;
3130

31+
import dev.openfeature.sdk.exceptions.OpenFeatureError;
3232
import dev.openfeature.sdk.testutils.exception.TestException;
3333

3434
class ProviderRepositoryTest {
@@ -253,7 +253,7 @@ void shouldRunLambdasOnSuccessful() {
253253
Consumer<FeatureProvider> afterSet = mock(Consumer.class);
254254
Consumer<FeatureProvider> afterInit = mock(Consumer.class);
255255
Consumer<FeatureProvider> afterShutdown = mock(Consumer.class);
256-
BiConsumer<FeatureProvider, String> afterError = mock(BiConsumer.class);
256+
BiConsumer<FeatureProvider, OpenFeatureError> afterError = mock(BiConsumer.class);
257257

258258
FeatureProvider oldProvider = providerRepository.getProvider();
259259
FeatureProvider featureProvider1 = createMockedProvider();
@@ -274,7 +274,7 @@ void shouldRunLambdasOnError() throws Exception {
274274
Consumer<FeatureProvider> afterSet = mock(Consumer.class);
275275
Consumer<FeatureProvider> afterInit = mock(Consumer.class);
276276
Consumer<FeatureProvider> afterShutdown = mock(Consumer.class);
277-
BiConsumer<FeatureProvider, String> afterError = mock(BiConsumer.class);
277+
BiConsumer<FeatureProvider, OpenFeatureError> afterError = mock(BiConsumer.class);
278278

279279
FeatureProvider errorFeatureProvider = createMockedErrorProvider();
280280

@@ -310,7 +310,7 @@ private void setFeatureProvider(FeatureProvider provider) {
310310

311311
private void setFeatureProvider(FeatureProvider provider, Consumer<FeatureProvider> afterSet,
312312
Consumer<FeatureProvider> afterInit, Consumer<FeatureProvider> afterShutdown,
313-
BiConsumer<FeatureProvider, String> afterError) {
313+
BiConsumer<FeatureProvider, OpenFeatureError> afterError) {
314314
providerRepository.setProvider(provider, afterSet, afterInit, afterShutdown,
315315
afterError, false);
316316
waitForSettingProviderHasBeenCompleted(ProviderRepository::getProvider, provider);
@@ -348,8 +348,8 @@ private Consumer<FeatureProvider> mockAfterShutdown() {
348348
};
349349
}
350350

351-
private BiConsumer<FeatureProvider, String> mockAfterError() {
352-
return (fp, message) -> {
351+
private BiConsumer<FeatureProvider, OpenFeatureError> mockAfterError() {
352+
return (fp, ex) -> {
353353
};
354354
}
355355

‎src/test/java/dev/openfeature/sdk/testutils/TestEventsProvider.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import dev.openfeature.sdk.ProviderEventDetails;
99
import dev.openfeature.sdk.ProviderState;
1010
import dev.openfeature.sdk.Value;
11+
import dev.openfeature.sdk.exceptions.GeneralError;
1112

1213
public class TestEventsProvider extends EventProvider {
1314

@@ -63,7 +64,7 @@ public void initialize(EvaluationContext evaluationContext) throws Exception {
6364
Thread.sleep(initTimeoutMs);
6465
if (this.initError) {
6566
this.state = ProviderState.ERROR;
66-
throw new Exception(initErrorMessage);
67+
throw new GeneralError(initErrorMessage);
6768
}
6869
this.state = ProviderState.READY;
6970
}

0 commit comments

Comments
 (0)