Skip to content

Commit d717373

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 d717373

File tree

6 files changed

+82
-32
lines changed

6 files changed

+82
-32
lines changed
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
package dev.openfeature.sdk;
2+
3+
import dev.openfeature.sdk.exceptions.OpenFeatureError;
4+
import lombok.Builder;
5+
import lombok.Value;
6+
import java.util.Optional;
7+
8+
@Value
9+
@Builder
10+
public class ErrorDetails<T> {
11+
Exception error;
12+
FlagEvaluationDetails<T> details;
13+
ErrorCode errorCode;
14+
String errorMessage;
15+
16+
public static <T> ErrorDetails<T> from(FlagEvaluationDetails<T> details) {
17+
return ErrorDetails.<T>builder()
18+
.details(details)
19+
.errorMessage(details.getErrorMessage())
20+
.errorCode(details.getErrorCode())
21+
.build();
22+
}
23+
public static <T> ErrorDetails<T> from(Exception exception, FlagEvaluationDetails<T> details) {
24+
25+
if (exception instanceof OpenFeatureError) {
26+
details.setErrorCode(((OpenFeatureError) exception).getErrorCode());
27+
} else {
28+
details.setErrorCode(ErrorCode.GENERAL);
29+
}
30+
details.setErrorMessage(exception.getMessage());
31+
return ErrorDetails.<T>builder()
32+
.error(exception)
33+
.errorMessage(exception.getMessage())
34+
.errorCode(details.getErrorCode())
35+
.build();
36+
}
37+
38+
39+
}

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 & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -204,9 +204,8 @@ private <T> FlagEvaluationDetails<T> evaluateFlag(
204204

205205
details = FlagEvaluationDetails.from(providerEval, key);
206206
if (details.getErrorCode() != null) {
207-
var error =
208-
ExceptionUtils.instantiateErrorByErrorCode(details.getErrorCode(), details.getErrorMessage());
209207
enrichDetailsWithErrorDefaults(defaultValue, details);
208+
ErrorDetails error = ErrorDetails.from(details);
210209
hookSupport.errorHooks(type, afterHookContext, error, mergedHooks, hints);
211210
} else {
212211
hookSupport.afterHooks(type, afterHookContext, details, mergedHooks, hints);
@@ -215,14 +214,9 @@ private <T> FlagEvaluationDetails<T> evaluateFlag(
215214
if (details == null) {
216215
details = FlagEvaluationDetails.<T>builder().flagKey(key).build();
217216
}
218-
if (e instanceof OpenFeatureError) {
219-
details.setErrorCode(((OpenFeatureError) e).getErrorCode());
220-
} else {
221-
details.setErrorCode(ErrorCode.GENERAL);
222-
}
223-
details.setErrorMessage(e.getMessage());
224217
enrichDetailsWithErrorDefaults(defaultValue, details);
225-
hookSupport.errorHooks(type, afterHookContext, e, mergedHooks, hints);
218+
ErrorDetails error = ErrorDetails.from(e, details);
219+
hookSupport.errorHooks(type, afterHookContext, error, mergedHooks, hints);
226220
} finally {
227221
hookSupport.afterAllHooks(type, afterHookContext, details, mergedHooks, hints);
228222
}

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

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,7 @@ void error_hook_run_during_non_finally_stage() {
200200
Hook h = mockBooleanHook();
201201
doThrow(RuntimeException.class).when(h).finallyAfter(any(), any(), any());
202202

203-
verify(h, times(0)).error(any(), any(), any());
203+
verify(h, times(0)).error(any(), any(Exception.class), any());
204204
}
205205

206206
@Test
@@ -225,16 +225,16 @@ void error_hook_must_run_if_resolution_details_returns_an_error_code() {
225225
invocationCtx,
226226
FlagEvaluationOptions.builder().hook(hook).build());
227227

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

230230
verify(hook, times(1)).before(any(), any());
231231
verify(hook, times(1)).error(any(), captor.capture(), any());
232232
verify(hook, times(1)).finallyAfter(any(), any(), any());
233233
verify(hook, never()).after(any(), any(), any());
234234

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

240240
@Specification(
@@ -279,7 +279,7 @@ public void after(
279279
}
280280

281281
@Override
282-
public void error(HookContext<Boolean> ctx, Exception error, Map<String, Object> hints) {
282+
public void error(HookContext<Boolean> ctx, ErrorDetails<Boolean> error, Map<String, Object> hints) {
283283
evalOrder.add("provider error");
284284
}
285285

@@ -308,7 +308,7 @@ public void after(
308308
}
309309

310310
@Override
311-
public void error(HookContext<Boolean> ctx, Exception error, Map<String, Object> hints) {
311+
public void error(HookContext<Boolean> ctx, ErrorDetails<Boolean> error, Map<String, Object> hints) {
312312
evalOrder.add("api error");
313313
}
314314

@@ -334,7 +334,7 @@ public void after(
334334
}
335335

336336
@Override
337-
public void error(HookContext<Boolean> ctx, Exception error, Map<String, Object> hints) {
337+
public void error(HookContext<Boolean> ctx, ErrorDetails<Boolean> error, Map<String, Object> hints) {
338338
evalOrder.add("client error");
339339
}
340340

@@ -367,7 +367,7 @@ public void after(
367367
}
368368

369369
@Override
370-
public void error(HookContext<Boolean> ctx, Exception error, Map<String, Object> hints) {
370+
public void error(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 & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,31 +55,34 @@ 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(
6164
flagValueType,
6265
hookContext,
63-
FlagEvaluationDetails.builder().build(),
66+
details,
6467
Collections.singletonList(genericHook),
6568
Collections.emptyMap());
6669
hookSupport.afterAllHooks(
6770
flagValueType,
6871
hookContext,
69-
FlagEvaluationDetails.builder().build(),
72+
details,
7073
Collections.singletonList(genericHook),
7174
Collections.emptyMap());
7275
hookSupport.errorHooks(
7376
flagValueType,
7477
hookContext,
75-
expectedException,
78+
error,
7679
Collections.singletonList(genericHook),
7780
Collections.emptyMap());
7881

7982
verify(genericHook).before(any(), any());
8083
verify(genericHook).after(any(), any(), any());
8184
verify(genericHook).finallyAfter(any(), any(), any());
82-
verify(genericHook).error(any(), any(), any());
85+
verify(genericHook).error(any(), any(ErrorDetails.class), any());
8386
}
8487

8588
private Object createDefaultValue(FlagValueType flagValueType) {

0 commit comments

Comments
 (0)