From 06b7ad2c1ca21e1e1d4c0d96c578e4f565fb008b Mon Sep 17 00:00:00 2001 From: jarebudev <23311805+jarebudev@users.noreply.github.com> Date: Thu, 23 May 2024 23:42:11 +0100 Subject: [PATCH] fix so error hook also runs when provider returns error code Signed-off-by: jarebudev <23311805+jarebudev@users.noreply.github.com> --- .../openfeature/sdk/OpenFeatureClient.java | 8 ++- .../openfeature/sdk/internal/ErrorUtils.java | 42 +++++++++++++++ .../dev/openfeature/sdk/HookSpecTest.java | 37 +++++++++++++ .../sdk/internal/ErrorUtilsTest.java | 53 +++++++++++++++++++ 4 files changed, 139 insertions(+), 1 deletion(-) create mode 100644 src/main/java/dev/openfeature/sdk/internal/ErrorUtils.java create mode 100644 src/test/java/dev/openfeature/sdk/internal/ErrorUtilsTest.java diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java index b03ef6aa6..8c86a7212 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java @@ -11,6 +11,7 @@ import dev.openfeature.sdk.exceptions.OpenFeatureError; import dev.openfeature.sdk.internal.AutoCloseableLock; import dev.openfeature.sdk.internal.AutoCloseableReentrantReadWriteLock; +import dev.openfeature.sdk.internal.ErrorUtils; import dev.openfeature.sdk.internal.ObjectUtils; import lombok.Getter; import lombok.extern.slf4j.Slf4j; @@ -125,7 +126,12 @@ private FlagEvaluationDetails evaluateFlag(FlagValueType type, String key defaultValue, provider, mergedCtx); details = FlagEvaluationDetails.from(providerEval, key); - hookSupport.afterHooks(type, hookCtx, details, mergedHooks, hints); + if (details.getErrorCode() != null) { + Exception e = ErrorUtils.instantiateErrorByErrorCode(details.getErrorCode(), details.getErrorMessage()); + hookSupport.errorHooks(type, hookCtx, e, mergedHooks, hints); + } else { + hookSupport.afterHooks(type, hookCtx, details, mergedHooks, hints); + } } catch (Exception e) { log.error("Unable to correctly evaluate flag with key '{}'", key, e); if (details == null) { diff --git a/src/main/java/dev/openfeature/sdk/internal/ErrorUtils.java b/src/main/java/dev/openfeature/sdk/internal/ErrorUtils.java new file mode 100644 index 000000000..e4989a167 --- /dev/null +++ b/src/main/java/dev/openfeature/sdk/internal/ErrorUtils.java @@ -0,0 +1,42 @@ +package dev.openfeature.sdk.internal; + +import dev.openfeature.sdk.ErrorCode; +import dev.openfeature.sdk.exceptions.FlagNotFoundError; +import dev.openfeature.sdk.exceptions.GeneralError; +import dev.openfeature.sdk.exceptions.InvalidContextError; +import dev.openfeature.sdk.exceptions.OpenFeatureError; +import dev.openfeature.sdk.exceptions.ParseError; +import dev.openfeature.sdk.exceptions.ProviderNotReadyError; +import dev.openfeature.sdk.exceptions.TargetingKeyMissingError; +import dev.openfeature.sdk.exceptions.TypeMismatchError; +import lombok.experimental.UtilityClass; + +@SuppressWarnings("checkstyle:MissingJavadocType") +@UtilityClass +public class ErrorUtils { + + /** + * Creates an Error for the specific error code. + * @param errorCode the ErrorCode to use + * @param errorMessage the error message to include in the returned error + * @return the specific OpenFeatureError for the errorCode + */ + public static OpenFeatureError instantiateErrorByErrorCode(ErrorCode errorCode, String errorMessage) { + switch (errorCode) { + case FLAG_NOT_FOUND: + return new FlagNotFoundError(errorMessage); + case PARSE_ERROR: + return new ParseError(errorMessage); + case TYPE_MISMATCH: + return new TypeMismatchError(errorMessage); + case TARGETING_KEY_MISSING: + return new TargetingKeyMissingError(errorMessage); + case INVALID_CONTEXT: + return new InvalidContextError(errorMessage); + case PROVIDER_NOT_READY: + return new ProviderNotReadyError(errorMessage); + default: + return new GeneralError(errorMessage); + } + } +} diff --git a/src/test/java/dev/openfeature/sdk/HookSpecTest.java b/src/test/java/dev/openfeature/sdk/HookSpecTest.java index d0e173b2a..098d80d7f 100644 --- a/src/test/java/dev/openfeature/sdk/HookSpecTest.java +++ b/src/test/java/dev/openfeature/sdk/HookSpecTest.java @@ -1,5 +1,6 @@ package dev.openfeature.sdk; +import dev.openfeature.sdk.exceptions.FlagNotFoundError; import dev.openfeature.sdk.fixtures.HookFixtures; import dev.openfeature.sdk.testutils.FeatureProviderTestUtils; import lombok.SneakyThrows; @@ -19,12 +20,14 @@ 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; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -180,6 +183,40 @@ void emptyApiHooks() { verify(h, times(0)).error(any(), any(), any()); } + + @Test void error_hook_must_run_if_resolution_details_returns_an_error_code() { + + String errorMessage = "not found..."; + + EvaluationContext invocationCtx = new ImmutableContext(); + Hook hook = mockBooleanHook(); + FeatureProvider provider = mock(FeatureProvider.class); + when(provider.getBooleanEvaluation(any(), any(), any())).thenReturn(ProviderEvaluation.builder() + .errorCode(ErrorCode.FLAG_NOT_FOUND) + .errorMessage(errorMessage) + .build()); + + OpenFeatureAPI api = OpenFeatureAPI.getInstance(); + FeatureProviderTestUtils.setFeatureProvider(provider); + Client client = api.getClient(); + client.getBooleanValue("key", false, invocationCtx, + FlagEvaluationOptions.builder() + .hook(hook) + .build()); + + ArgumentCaptor captor = ArgumentCaptor.forClass(Exception.class); + + verify(hook, times(1)).before(any(), any()); + verify(hook, times(1)).error(any(), captor.capture(), any()); + verify(hook, times(1)).finallyAfter(any(), any()); + verify(hook, never()).after(any(),any(), any()); + + Exception exception = captor.getValue(); + assertEquals(errorMessage, exception.getMessage()); + assertInstanceOf(FlagNotFoundError.class, exception); + } + + @Specification(number="4.3.6", text="The after stage MUST run after flag resolution occurs. It accepts a hook context (required), flag evaluation details (required) and hook hints (optional). It has no return value.") @Specification(number="4.3.7", text="The error hook MUST run when errors are encountered in the before stage, the after stage or during flag resolution. It accepts hook context (required), exception representing what went wrong (required), and hook hints (optional). It has no return value.") @Specification(number="4.3.8", text="The finally hook MUST run after the before, after, and error stages. It accepts a hook context (required) and hook hints (optional). There is no return value.") diff --git a/src/test/java/dev/openfeature/sdk/internal/ErrorUtilsTest.java b/src/test/java/dev/openfeature/sdk/internal/ErrorUtilsTest.java new file mode 100644 index 000000000..ded56c45c --- /dev/null +++ b/src/test/java/dev/openfeature/sdk/internal/ErrorUtilsTest.java @@ -0,0 +1,53 @@ +package dev.openfeature.sdk.internal; + +import dev.openfeature.sdk.ErrorCode; +import dev.openfeature.sdk.exceptions.FlagNotFoundError; +import dev.openfeature.sdk.exceptions.GeneralError; +import dev.openfeature.sdk.exceptions.InvalidContextError; +import dev.openfeature.sdk.exceptions.OpenFeatureError; +import dev.openfeature.sdk.exceptions.ParseError; +import dev.openfeature.sdk.exceptions.ProviderNotReadyError; +import dev.openfeature.sdk.exceptions.TargetingKeyMissingError; +import dev.openfeature.sdk.exceptions.TypeMismatchError; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.extension.ExtensionContext; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.ArgumentsProvider; +import org.junit.jupiter.params.provider.ArgumentsSource; + +import java.util.stream.Stream; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; + +class ErrorUtilsTest { + + @ParameterizedTest + @DisplayName("should produce correct exception for a provided ErrorCode") + @ArgumentsSource(ErrorCodeTestParameters.class) + void shouldProduceCorrectExceptionForErrorCode(ErrorCode errorCode, Class exception) { + + String errorMessage = "error message"; + OpenFeatureError openFeatureError = ErrorUtils.instantiateErrorByErrorCode(errorCode, errorMessage); + assertInstanceOf(exception, openFeatureError); + assertThat(openFeatureError.getMessage()).isEqualTo(errorMessage); + assertThat(openFeatureError.getErrorCode()).isEqualByComparingTo(errorCode); + } + + static class ErrorCodeTestParameters implements ArgumentsProvider { + + @Override + public Stream provideArguments(ExtensionContext context) { + return Stream.of( + Arguments.of(ErrorCode.GENERAL, GeneralError.class), + Arguments.of(ErrorCode.FLAG_NOT_FOUND, FlagNotFoundError.class), + Arguments.of(ErrorCode.PROVIDER_NOT_READY, ProviderNotReadyError.class), + Arguments.of(ErrorCode.INVALID_CONTEXT, InvalidContextError.class), + Arguments.of(ErrorCode.PARSE_ERROR, ParseError.class), + Arguments.of(ErrorCode.TARGETING_KEY_MISSING, TargetingKeyMissingError.class), + Arguments.of(ErrorCode.TYPE_MISMATCH, TypeMismatchError.class) + ); + } + } +}