diff --git a/src/main/java/dev/openfeature/sdk/ErrorDetails.java b/src/main/java/dev/openfeature/sdk/ErrorDetails.java new file mode 100644 index 000000000..60319cf05 --- /dev/null +++ b/src/main/java/dev/openfeature/sdk/ErrorDetails.java @@ -0,0 +1,61 @@ +package dev.openfeature.sdk; + +import dev.openfeature.sdk.exceptions.OpenFeatureError; +import lombok.Builder; +import lombok.Value; + +/** + * Represents details about an error that occurred during a flag evaluation or other operations. + * This class captures the exception, evaluation details, error code, and an error message. + * + * @param The type of the value being evaluated in the {@link FlagEvaluationDetails}. + */ +@Value +@Builder +public class ErrorDetails { + Exception error; + FlagEvaluationDetails details; + ErrorCode errorCode; + String errorMessage; + + /** + * Creates an {@code ErrorDetails} instance from the given {@link FlagEvaluationDetails}. + * This method extracts the error message and error code from the provided details. + * + * @param details The {@link FlagEvaluationDetails} object containing flag evaluation information. + * @param The type of the value being evaluated in the {@link FlagEvaluationDetails}. + * @return An {@code ErrorDetails} object populated with the provided evaluation details. + */ + public static ErrorDetails from(FlagEvaluationDetails details) { + return ErrorDetails.builder() + .details(details) + .errorMessage(details.getErrorMessage()) + .errorCode(details.getErrorCode()) + .build(); + } + + /** + * Creates an {@code ErrorDetails} instance from the given exception and {@link FlagEvaluationDetails}. + * If the exception is an instance of {@link OpenFeatureError}, its error code is extracted + * and set in the {@link FlagEvaluationDetails}. Otherwise, a general error code is used. + * The exception's message is also set as the error message. + * + * @param exception The exception that occurred during the operation. + * @param details The {@link FlagEvaluationDetails} object containing flag evaluation information. + * @param The type of the value being evaluated in the {@link FlagEvaluationDetails}. + * @return An {@code ErrorDetails} object populated with the exception and evaluation details. + */ + public static ErrorDetails from(Exception exception, FlagEvaluationDetails details) { + if (exception instanceof OpenFeatureError) { + details.setErrorCode(((OpenFeatureError) exception).getErrorCode()); + } else { + details.setErrorCode(ErrorCode.GENERAL); + } + details.setErrorMessage(exception.getMessage()); + return ErrorDetails.builder() + .error(exception) + .errorMessage(exception.getMessage()) + .errorCode(details.getErrorCode()) + .build(); + } +} diff --git a/src/main/java/dev/openfeature/sdk/Hook.java b/src/main/java/dev/openfeature/sdk/Hook.java index 08aa18314..019058366 100644 --- a/src/main/java/dev/openfeature/sdk/Hook.java +++ b/src/main/java/dev/openfeature/sdk/Hook.java @@ -1,5 +1,6 @@ package dev.openfeature.sdk; +import dev.openfeature.sdk.exceptions.ExceptionUtils; import java.util.Map; import java.util.Optional; @@ -35,9 +36,22 @@ default void after(HookContext ctx, FlagEvaluationDetails details, Map ctx, ErrorDetails error, Map hints) { + error(ctx, ExceptionUtils.instantiateErrorByErrorCode(error.getErrorCode(), error.getErrorMessage()), hints); + } + + /** + * Run when evaluation encounters an error. This will always run. Errors thrown will be swallowed. + * + * @param ctx Information about the particular flag evaluation + * @param error The exception that was thrown or instantiated. + * @param hints An immutable mapping of data for users to communicate to the hooks. + * @deprecated Please use ErrorDetails instead of Exceptions. + */ + @Deprecated default void error(HookContext ctx, Exception error, Map hints) {} /** diff --git a/src/main/java/dev/openfeature/sdk/HookSupport.java b/src/main/java/dev/openfeature/sdk/HookSupport.java index 73518ee8e..4e778e6e6 100644 --- a/src/main/java/dev/openfeature/sdk/HookSupport.java +++ b/src/main/java/dev/openfeature/sdk/HookSupport.java @@ -40,7 +40,7 @@ public void afterAllHooks( public void errorHooks( FlagValueType flagValueType, HookContext hookCtx, - Exception e, + ErrorDetails e, List hooks, Map hints) { executeHooks(flagValueType, hooks, "error", hook -> hook.error(hookCtx, e, hints)); diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java index b5522b66a..e251a02da 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java @@ -1,9 +1,7 @@ package dev.openfeature.sdk; -import dev.openfeature.sdk.exceptions.ExceptionUtils; import dev.openfeature.sdk.exceptions.FatalError; import dev.openfeature.sdk.exceptions.GeneralError; -import dev.openfeature.sdk.exceptions.OpenFeatureError; import dev.openfeature.sdk.exceptions.ProviderNotReadyError; import dev.openfeature.sdk.internal.ObjectUtils; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; @@ -204,9 +202,8 @@ private FlagEvaluationDetails evaluateFlag( details = FlagEvaluationDetails.from(providerEval, key); if (details.getErrorCode() != null) { - var error = - ExceptionUtils.instantiateErrorByErrorCode(details.getErrorCode(), details.getErrorMessage()); enrichDetailsWithErrorDefaults(defaultValue, details); + ErrorDetails error = ErrorDetails.from(details); hookSupport.errorHooks(type, afterHookContext, error, mergedHooks, hints); } else { hookSupport.afterHooks(type, afterHookContext, details, mergedHooks, hints); @@ -215,14 +212,9 @@ private FlagEvaluationDetails evaluateFlag( if (details == null) { details = FlagEvaluationDetails.builder().flagKey(key).build(); } - if (e instanceof OpenFeatureError) { - details.setErrorCode(((OpenFeatureError) e).getErrorCode()); - } else { - details.setErrorCode(ErrorCode.GENERAL); - } - details.setErrorMessage(e.getMessage()); enrichDetailsWithErrorDefaults(defaultValue, details); - hookSupport.errorHooks(type, afterHookContext, e, mergedHooks, hints); + ErrorDetails error = ErrorDetails.from(e, details); + hookSupport.errorHooks(type, afterHookContext, error, mergedHooks, hints); } finally { hookSupport.afterAllHooks(type, afterHookContext, details, mergedHooks, hints); } diff --git a/src/test/java/dev/openfeature/sdk/HookSpecTest.java b/src/test/java/dev/openfeature/sdk/HookSpecTest.java index 3a953d18a..6a046fa4d 100644 --- a/src/test/java/dev/openfeature/sdk/HookSpecTest.java +++ b/src/test/java/dev/openfeature/sdk/HookSpecTest.java @@ -4,7 +4,6 @@ import static org.assertj.core.api.Assertions.assertThatCode; import static org.assertj.core.api.Assertions.fail; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; @@ -16,7 +15,6 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -import dev.openfeature.sdk.exceptions.FlagNotFoundError; import dev.openfeature.sdk.fixtures.HookFixtures; import dev.openfeature.sdk.testutils.TestEventsProvider; import java.util.ArrayList; @@ -200,7 +198,7 @@ void error_hook_run_during_non_finally_stage() { Hook h = mockBooleanHook(); doThrow(RuntimeException.class).when(h).finallyAfter(any(), any(), any()); - verify(h, times(0)).error(any(), any(), any()); + verify(h, times(0)).error(any(), any(Exception.class), any()); } @Test @@ -225,16 +223,16 @@ void error_hook_must_run_if_resolution_details_returns_an_error_code() { invocationCtx, FlagEvaluationOptions.builder().hook(hook).build()); - ArgumentCaptor captor = ArgumentCaptor.forClass(Exception.class); + ArgumentCaptor captor = ArgumentCaptor.forClass(ErrorDetails.class); verify(hook, times(1)).before(any(), any()); verify(hook, times(1)).error(any(), captor.capture(), any()); verify(hook, times(1)).finallyAfter(any(), any(), any()); verify(hook, never()).after(any(), any(), any()); - Exception exception = captor.getValue(); - assertEquals(errorMessage, exception.getMessage()); - assertInstanceOf(FlagNotFoundError.class, exception); + ErrorDetails errorDetails = captor.getValue(); + assertEquals(errorMessage, errorDetails.getErrorMessage()); + assertEquals(ErrorCode.FLAG_NOT_FOUND, errorDetails.getErrorCode()); } @Specification( @@ -279,7 +277,8 @@ public void after( } @Override - public void error(HookContext ctx, Exception error, Map hints) { + public void error( + HookContext ctx, ErrorDetails error, Map hints) { evalOrder.add("provider error"); } @@ -308,7 +307,7 @@ public void after( } @Override - public void error(HookContext ctx, Exception error, Map hints) { + public void error(HookContext ctx, ErrorDetails error, Map hints) { evalOrder.add("api error"); } @@ -334,7 +333,7 @@ public void after( } @Override - public void error(HookContext ctx, Exception error, Map hints) { + public void error(HookContext ctx, ErrorDetails error, Map hints) { evalOrder.add("client error"); } @@ -367,7 +366,8 @@ public void after( } @Override - public void error(HookContext ctx, Exception error, Map hints) { + public void error( + HookContext ctx, ErrorDetails error, Map hints) { evalOrder.add("invocation error"); } @@ -546,7 +546,7 @@ void error_hooks__before() { new ImmutableContext(), FlagEvaluationOptions.builder().hook(hook).build()); verify(hook, times(1)).before(any(), any()); - verify(hook, times(1)).error(any(), any(), any()); + verify(hook, times(1)).error(any(), any(ErrorDetails.class), any()); assertEquals(false, value, "Falls through to the default."); } @@ -564,7 +564,7 @@ void error_hooks__after() { new ImmutableContext(), FlagEvaluationOptions.builder().hook(hook).build()); verify(hook, times(1)).after(any(), any(), any()); - verify(hook, times(1)).error(any(), any(), any()); + verify(hook, times(1)).error(any(), any(ErrorDetails.class), any()); } @Test @@ -609,7 +609,7 @@ void shortCircuit_flagResolution_runsHooksWithAllFields() { FlagEvaluationOptions.builder().hook(hook).build()); verify(hook).before(any(), any()); - verify(hook).error(any(HookContext.class), any(Exception.class), any(Map.class)); + verify(hook).error(any(HookContext.class), any(ErrorDetails.class), any(Map.class)); verify(hook).finallyAfter(any(HookContext.class), any(FlagEvaluationDetails.class), any(Map.class)); } @@ -655,8 +655,8 @@ void multi_hooks_early_out__before() { verify(hook, times(1)).before(any(), any()); verify(hook2, times(0)).before(any(), any()); - verify(hook, times(1)).error(any(), any(), any()); - verify(hook2, times(1)).error(any(), any(), any()); + verify(hook, times(1)).error(any(), any(ErrorDetails.class), any()); + verify(hook2, times(1)).error(any(), any(ErrorDetails.class), any()); } @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() { void first_error_broken() { Hook hook = mockBooleanHook(); doThrow(RuntimeException.class).when(hook).before(any(), any()); - doThrow(RuntimeException.class).when(hook).error(any(), any(), any()); + doThrow(RuntimeException.class).when(hook).error(any(), any(Exception.class), any()); Hook hook2 = mockBooleanHook(); InOrder order = inOrder(hook, hook2); @@ -772,8 +772,8 @@ void first_error_broken() { FlagEvaluationOptions.builder().hook(hook2).hook(hook).build()); order.verify(hook).before(any(), any()); - order.verify(hook2).error(any(), any(), any()); - order.verify(hook).error(any(), any(), any()); + order.verify(hook2).error(any(), any(ErrorDetails.class), any()); + order.verify(hook).error(any(), any(ErrorDetails.class), any()); } private Client getClient(FeatureProvider provider) { diff --git a/src/test/java/dev/openfeature/sdk/HookSupportTest.java b/src/test/java/dev/openfeature/sdk/HookSupportTest.java index 02a8ff90c..cef7f9a16 100644 --- a/src/test/java/dev/openfeature/sdk/HookSupportTest.java +++ b/src/test/java/dev/openfeature/sdk/HookSupportTest.java @@ -55,31 +55,22 @@ void shouldAlwaysCallGenericHook(FlagValueType flagValueType) { () -> "client", () -> "provider"); + FlagEvaluationDetails details = FlagEvaluationDetails.builder().build(); + ErrorDetails error = ErrorDetails.from(expectedException, details); + hookSupport.beforeHooks( flagValueType, hookContext, Collections.singletonList(genericHook), Collections.emptyMap()); hookSupport.afterHooks( - flagValueType, - hookContext, - FlagEvaluationDetails.builder().build(), - Collections.singletonList(genericHook), - Collections.emptyMap()); + flagValueType, hookContext, details, Collections.singletonList(genericHook), Collections.emptyMap()); hookSupport.afterAllHooks( - flagValueType, - hookContext, - FlagEvaluationDetails.builder().build(), - Collections.singletonList(genericHook), - Collections.emptyMap()); + flagValueType, hookContext, details, Collections.singletonList(genericHook), Collections.emptyMap()); hookSupport.errorHooks( - flagValueType, - hookContext, - expectedException, - Collections.singletonList(genericHook), - Collections.emptyMap()); + flagValueType, hookContext, error, Collections.singletonList(genericHook), Collections.emptyMap()); verify(genericHook).before(any(), any()); verify(genericHook).after(any(), any(), any()); verify(genericHook).finallyAfter(any(), any(), any()); - verify(genericHook).error(any(), any(), any()); + verify(genericHook).error(any(), any(ErrorDetails.class), any()); } private Object createDefaultValue(FlagValueType flagValueType) {