Skip to content

Commit ec55588

Browse files
committed
demo: changing error hook api to contain object
This is a fast demo of open-feature/spec#326 As errors in the flag evaluation do not have to be exceptions, this opens up improvements for flow control via error codes from the EvaluationDetails. Signed-off-by: Simon Schrottner <[email protected]>
1 parent db47b7e commit ec55588

File tree

6 files changed

+106
-48
lines changed

6 files changed

+106
-48
lines changed
Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
package dev.openfeature.sdk;
2+
3+
import dev.openfeature.sdk.exceptions.OpenFeatureError;
4+
import lombok.Builder;
5+
import lombok.Value;
6+
7+
/**
8+
* Represents details about an error that occurred during a flag evaluation or other operations.
9+
* This class captures the exception, evaluation details, error code, and an error message.
10+
*
11+
* @param <T> The type of the value being evaluated in the {@link FlagEvaluationDetails}.
12+
*/
13+
@Value
14+
@Builder
15+
public class ErrorDetails<T> {
16+
Exception error;
17+
FlagEvaluationDetails<T> details;
18+
ErrorCode errorCode;
19+
String errorMessage;
20+
21+
/**
22+
* Creates an {@code ErrorDetails} instance from the given {@link FlagEvaluationDetails}.
23+
* This method extracts the error message and error code from the provided details.
24+
*
25+
* @param details The {@link FlagEvaluationDetails} object containing flag evaluation information.
26+
* @param <T> The type of the value being evaluated in the {@link FlagEvaluationDetails}.
27+
* @return An {@code ErrorDetails} object populated with the provided evaluation details.
28+
*/
29+
public static <T> ErrorDetails<T> from(FlagEvaluationDetails<T> details) {
30+
return ErrorDetails.<T>builder()
31+
.details(details)
32+
.errorMessage(details.getErrorMessage())
33+
.errorCode(details.getErrorCode())
34+
.build();
35+
}
36+
37+
/**
38+
* Creates an {@code ErrorDetails} instance from the given exception and {@link FlagEvaluationDetails}.
39+
* If the exception is an instance of {@link OpenFeatureError}, its error code is extracted
40+
* and set in the {@link FlagEvaluationDetails}. Otherwise, a general error code is used.
41+
* The exception's message is also set as the error message.
42+
*
43+
* @param exception The exception that occurred during the operation.
44+
* @param details The {@link FlagEvaluationDetails} object containing flag evaluation information.
45+
* @param <T> The type of the value being evaluated in the {@link FlagEvaluationDetails}.
46+
* @return An {@code ErrorDetails} object populated with the exception and evaluation details.
47+
*/
48+
public static <T> ErrorDetails<T> from(Exception exception, FlagEvaluationDetails<T> details) {
49+
if (exception instanceof OpenFeatureError) {
50+
details.setErrorCode(((OpenFeatureError) exception).getErrorCode());
51+
} else {
52+
details.setErrorCode(ErrorCode.GENERAL);
53+
}
54+
details.setErrorMessage(exception.getMessage());
55+
return ErrorDetails.<T>builder()
56+
.error(exception)
57+
.errorMessage(exception.getMessage())
58+
.errorCode(details.getErrorCode())
59+
.build();
60+
}
61+
}

src/main/java/dev/openfeature/sdk/Hook.java

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package dev.openfeature.sdk;
22

3+
import dev.openfeature.sdk.exceptions.ExceptionUtils;
34
import java.util.Map;
45
import java.util.Optional;
56

@@ -35,9 +36,22 @@ default void after(HookContext<T> ctx, FlagEvaluationDetails<T> details, Map<Str
3536
* Run when evaluation encounters an error. This will always run. Errors thrown will be swallowed.
3637
*
3738
* @param ctx Information about the particular flag evaluation
38-
* @param error The exception that was thrown.
39+
* @param error The error details might contain an exception that was thrown.
3940
* @param hints An immutable mapping of data for users to communicate to the hooks.
4041
*/
42+
default void error(HookContext<T> ctx, ErrorDetails<T> error, Map<String, Object> hints) {
43+
error(ctx, ExceptionUtils.instantiateErrorByErrorCode(error.getErrorCode(), error.getErrorMessage()), hints);
44+
}
45+
46+
/**
47+
* Run when evaluation encounters an error. This will always run. Errors thrown will be swallowed.
48+
*
49+
* @param ctx Information about the particular flag evaluation
50+
* @param error The exception that was thrown or instantiated.
51+
* @param hints An immutable mapping of data for users to communicate to the hooks.
52+
* @deprecated Please use ErrorDetails instead of Exceptions.
53+
*/
54+
@Deprecated
4155
default void error(HookContext<T> ctx, Exception error, Map<String, Object> hints) {}
4256

4357
/**

src/main/java/dev/openfeature/sdk/HookSupport.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ public void afterAllHooks(
4040
public void errorHooks(
4141
FlagValueType flagValueType,
4242
HookContext hookCtx,
43-
Exception e,
43+
ErrorDetails e,
4444
List<Hook> hooks,
4545
Map<String, Object> hints) {
4646
executeHooks(flagValueType, hooks, "error", hook -> hook.error(hookCtx, e, hints));

src/main/java/dev/openfeature/sdk/OpenFeatureClient.java

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

3-
import dev.openfeature.sdk.exceptions.ExceptionUtils;
43
import dev.openfeature.sdk.exceptions.FatalError;
54
import dev.openfeature.sdk.exceptions.GeneralError;
6-
import dev.openfeature.sdk.exceptions.OpenFeatureError;
75
import dev.openfeature.sdk.exceptions.ProviderNotReadyError;
86
import dev.openfeature.sdk.internal.ObjectUtils;
97
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
@@ -204,9 +202,8 @@ private <T> FlagEvaluationDetails<T> evaluateFlag(
204202

205203
details = FlagEvaluationDetails.from(providerEval, key);
206204
if (details.getErrorCode() != null) {
207-
var error =
208-
ExceptionUtils.instantiateErrorByErrorCode(details.getErrorCode(), details.getErrorMessage());
209205
enrichDetailsWithErrorDefaults(defaultValue, details);
206+
ErrorDetails error = ErrorDetails.from(details);
210207
hookSupport.errorHooks(type, afterHookContext, error, mergedHooks, hints);
211208
} else {
212209
hookSupport.afterHooks(type, afterHookContext, details, mergedHooks, hints);
@@ -215,14 +212,9 @@ private <T> FlagEvaluationDetails<T> evaluateFlag(
215212
if (details == null) {
216213
details = FlagEvaluationDetails.<T>builder().flagKey(key).build();
217214
}
218-
if (e instanceof OpenFeatureError) {
219-
details.setErrorCode(((OpenFeatureError) e).getErrorCode());
220-
} else {
221-
details.setErrorCode(ErrorCode.GENERAL);
222-
}
223-
details.setErrorMessage(e.getMessage());
224215
enrichDetailsWithErrorDefaults(defaultValue, details);
225-
hookSupport.errorHooks(type, afterHookContext, e, mergedHooks, hints);
216+
ErrorDetails error = ErrorDetails.from(e, details);
217+
hookSupport.errorHooks(type, afterHookContext, error, mergedHooks, hints);
226218
} finally {
227219
hookSupport.afterAllHooks(type, afterHookContext, details, mergedHooks, hints);
228220
}

src/test/java/dev/openfeature/sdk/HookSpecTest.java

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
import static org.assertj.core.api.Assertions.assertThatCode;
55
import static org.assertj.core.api.Assertions.fail;
66
import static org.junit.jupiter.api.Assertions.assertEquals;
7-
import static org.junit.jupiter.api.Assertions.assertInstanceOf;
87
import static org.junit.jupiter.api.Assertions.assertNotNull;
98
import static org.junit.jupiter.api.Assertions.assertTrue;
109
import static org.mockito.ArgumentMatchers.any;
@@ -16,7 +15,6 @@
1615
import static org.mockito.Mockito.verify;
1716
import static org.mockito.Mockito.when;
1817

19-
import dev.openfeature.sdk.exceptions.FlagNotFoundError;
2018
import dev.openfeature.sdk.fixtures.HookFixtures;
2119
import dev.openfeature.sdk.testutils.TestEventsProvider;
2220
import java.util.ArrayList;
@@ -200,7 +198,7 @@ void error_hook_run_during_non_finally_stage() {
200198
Hook h = mockBooleanHook();
201199
doThrow(RuntimeException.class).when(h).finallyAfter(any(), any(), any());
202200

203-
verify(h, times(0)).error(any(), any(), any());
201+
verify(h, times(0)).error(any(), any(Exception.class), any());
204202
}
205203

206204
@Test
@@ -225,16 +223,16 @@ void error_hook_must_run_if_resolution_details_returns_an_error_code() {
225223
invocationCtx,
226224
FlagEvaluationOptions.builder().hook(hook).build());
227225

228-
ArgumentCaptor<Exception> captor = ArgumentCaptor.forClass(Exception.class);
226+
ArgumentCaptor<ErrorDetails> captor = ArgumentCaptor.forClass(ErrorDetails.class);
229227

230228
verify(hook, times(1)).before(any(), any());
231229
verify(hook, times(1)).error(any(), captor.capture(), any());
232230
verify(hook, times(1)).finallyAfter(any(), any(), any());
233231
verify(hook, never()).after(any(), any(), any());
234232

235-
Exception exception = captor.getValue();
236-
assertEquals(errorMessage, exception.getMessage());
237-
assertInstanceOf(FlagNotFoundError.class, exception);
233+
ErrorDetails errorDetails = captor.getValue();
234+
assertEquals(errorMessage, errorDetails.getErrorMessage());
235+
assertEquals(ErrorCode.FLAG_NOT_FOUND, errorDetails.getErrorCode());
238236
}
239237

240238
@Specification(
@@ -279,7 +277,8 @@ public void after(
279277
}
280278

281279
@Override
282-
public void error(HookContext<Boolean> ctx, Exception error, Map<String, Object> hints) {
280+
public void error(
281+
HookContext<Boolean> ctx, ErrorDetails<Boolean> error, Map<String, Object> hints) {
283282
evalOrder.add("provider error");
284283
}
285284

@@ -308,7 +307,7 @@ public void after(
308307
}
309308

310309
@Override
311-
public void error(HookContext<Boolean> ctx, Exception error, Map<String, Object> hints) {
310+
public void error(HookContext<Boolean> ctx, ErrorDetails<Boolean> error, Map<String, Object> hints) {
312311
evalOrder.add("api error");
313312
}
314313

@@ -334,7 +333,7 @@ public void after(
334333
}
335334

336335
@Override
337-
public void error(HookContext<Boolean> ctx, Exception error, Map<String, Object> hints) {
336+
public void error(HookContext<Boolean> ctx, ErrorDetails<Boolean> error, Map<String, Object> hints) {
338337
evalOrder.add("client error");
339338
}
340339

@@ -367,7 +366,8 @@ public void after(
367366
}
368367

369368
@Override
370-
public void error(HookContext<Boolean> ctx, Exception error, Map<String, Object> hints) {
369+
public void error(
370+
HookContext<Boolean> ctx, ErrorDetails<Boolean> error, Map<String, Object> hints) {
371371
evalOrder.add("invocation error");
372372
}
373373

@@ -546,7 +546,7 @@ void error_hooks__before() {
546546
new ImmutableContext(),
547547
FlagEvaluationOptions.builder().hook(hook).build());
548548
verify(hook, times(1)).before(any(), any());
549-
verify(hook, times(1)).error(any(), any(), any());
549+
verify(hook, times(1)).error(any(), any(ErrorDetails.class), any());
550550
assertEquals(false, value, "Falls through to the default.");
551551
}
552552

@@ -564,7 +564,7 @@ void error_hooks__after() {
564564
new ImmutableContext(),
565565
FlagEvaluationOptions.builder().hook(hook).build());
566566
verify(hook, times(1)).after(any(), any(), any());
567-
verify(hook, times(1)).error(any(), any(), any());
567+
verify(hook, times(1)).error(any(), any(ErrorDetails.class), any());
568568
}
569569

570570
@Test
@@ -609,7 +609,7 @@ void shortCircuit_flagResolution_runsHooksWithAllFields() {
609609
FlagEvaluationOptions.builder().hook(hook).build());
610610

611611
verify(hook).before(any(), any());
612-
verify(hook).error(any(HookContext.class), any(Exception.class), any(Map.class));
612+
verify(hook).error(any(HookContext.class), any(ErrorDetails.class), any(Map.class));
613613
verify(hook).finallyAfter(any(HookContext.class), any(FlagEvaluationDetails.class), any(Map.class));
614614
}
615615

@@ -655,8 +655,8 @@ void multi_hooks_early_out__before() {
655655
verify(hook, times(1)).before(any(), any());
656656
verify(hook2, times(0)).before(any(), any());
657657

658-
verify(hook, times(1)).error(any(), any(), any());
659-
verify(hook2, times(1)).error(any(), any(), any());
658+
verify(hook, times(1)).error(any(), any(ErrorDetails.class), any());
659+
verify(hook2, times(1)).error(any(), any(ErrorDetails.class), any());
660660
}
661661

662662
@Specification(number = "4.1.4", text = "The evaluation context MUST be mutable only within the before hook.")
@@ -760,7 +760,7 @@ void first_finally_broken() {
760760
void first_error_broken() {
761761
Hook hook = mockBooleanHook();
762762
doThrow(RuntimeException.class).when(hook).before(any(), any());
763-
doThrow(RuntimeException.class).when(hook).error(any(), any(), any());
763+
doThrow(RuntimeException.class).when(hook).error(any(), any(Exception.class), any());
764764
Hook hook2 = mockBooleanHook();
765765
InOrder order = inOrder(hook, hook2);
766766

@@ -772,8 +772,8 @@ void first_error_broken() {
772772
FlagEvaluationOptions.builder().hook(hook2).hook(hook).build());
773773

774774
order.verify(hook).before(any(), any());
775-
order.verify(hook2).error(any(), any(), any());
776-
order.verify(hook).error(any(), any(), any());
775+
order.verify(hook2).error(any(), any(ErrorDetails.class), any());
776+
order.verify(hook).error(any(), any(ErrorDetails.class), any());
777777
}
778778

779779
private Client getClient(FeatureProvider provider) {

src/test/java/dev/openfeature/sdk/HookSupportTest.java

Lines changed: 7 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -55,31 +55,22 @@ void shouldAlwaysCallGenericHook(FlagValueType flagValueType) {
5555
() -> "client",
5656
() -> "provider");
5757

58+
FlagEvaluationDetails<Object> details = FlagEvaluationDetails.builder().build();
59+
ErrorDetails<Object> error = ErrorDetails.from(expectedException, details);
60+
5861
hookSupport.beforeHooks(
5962
flagValueType, hookContext, Collections.singletonList(genericHook), Collections.emptyMap());
6063
hookSupport.afterHooks(
61-
flagValueType,
62-
hookContext,
63-
FlagEvaluationDetails.builder().build(),
64-
Collections.singletonList(genericHook),
65-
Collections.emptyMap());
64+
flagValueType, hookContext, details, Collections.singletonList(genericHook), Collections.emptyMap());
6665
hookSupport.afterAllHooks(
67-
flagValueType,
68-
hookContext,
69-
FlagEvaluationDetails.builder().build(),
70-
Collections.singletonList(genericHook),
71-
Collections.emptyMap());
66+
flagValueType, hookContext, details, Collections.singletonList(genericHook), Collections.emptyMap());
7267
hookSupport.errorHooks(
73-
flagValueType,
74-
hookContext,
75-
expectedException,
76-
Collections.singletonList(genericHook),
77-
Collections.emptyMap());
68+
flagValueType, hookContext, error, Collections.singletonList(genericHook), Collections.emptyMap());
7869

7970
verify(genericHook).before(any(), any());
8071
verify(genericHook).after(any(), any(), any());
8172
verify(genericHook).finallyAfter(any(), any(), any());
82-
verify(genericHook).error(any(), any(), any());
73+
verify(genericHook).error(any(), any(ErrorDetails.class), any());
8374
}
8475

8576
private Object createDefaultValue(FlagValueType flagValueType) {

0 commit comments

Comments
 (0)