Skip to content

Commit

Permalink
fix so error hook also runs when provider returns error code
Browse files Browse the repository at this point in the history
Signed-off-by: jarebudev <23311805+jarebudev@users.noreply.github.com>
  • Loading branch information
jarebudev authored and Kavindu-Dodan committed May 24, 2024
1 parent 253f29d commit 06b7ad2
Show file tree
Hide file tree
Showing 4 changed files with 139 additions and 1 deletion.
8 changes: 7 additions & 1 deletion src/main/java/dev/openfeature/sdk/OpenFeatureClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -125,7 +126,12 @@ private <T> FlagEvaluationDetails<T> 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) {
Expand Down
42 changes: 42 additions & 0 deletions src/main/java/dev/openfeature/sdk/internal/ErrorUtils.java
Original file line number Diff line number Diff line change
@@ -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);
}
}
}
37 changes: 37 additions & 0 deletions src/test/java/dev/openfeature/sdk/HookSpecTest.java
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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<Boolean> hook = mockBooleanHook();
FeatureProvider provider = mock(FeatureProvider.class);
when(provider.getBooleanEvaluation(any(), any(), any())).thenReturn(ProviderEvaluation.<Boolean>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<Exception> 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.")
Expand Down
53 changes: 53 additions & 0 deletions src/test/java/dev/openfeature/sdk/internal/ErrorUtilsTest.java
Original file line number Diff line number Diff line change
@@ -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<? extends OpenFeatureError> 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<? extends Arguments> 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)
);
}
}
}

0 comments on commit 06b7ad2

Please sign in to comment.