Skip to content

chore: changing error hook api to contain object #1525

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 61 additions & 0 deletions src/main/java/dev/openfeature/sdk/ErrorDetails.java
Original file line number Diff line number Diff line change
@@ -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 <T> The type of the value being evaluated in the {@link FlagEvaluationDetails}.
*/
@Value
@Builder
public class ErrorDetails<T> {
Exception error;
FlagEvaluationDetails<T> 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 <T> The type of the value being evaluated in the {@link FlagEvaluationDetails}.
* @return An {@code ErrorDetails} object populated with the provided evaluation details.
*/
public static <T> ErrorDetails<T> from(FlagEvaluationDetails<T> details) {
return ErrorDetails.<T>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 <T> 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 <T> ErrorDetails<T> from(Exception exception, FlagEvaluationDetails<T> details) {
if (exception instanceof OpenFeatureError) {
details.setErrorCode(((OpenFeatureError) exception).getErrorCode());
} else {
details.setErrorCode(ErrorCode.GENERAL);
}
details.setErrorMessage(exception.getMessage());
return ErrorDetails.<T>builder()
.error(exception)
.errorMessage(exception.getMessage())
.errorCode(details.getErrorCode())
.build();
}
}
16 changes: 15 additions & 1 deletion src/main/java/dev/openfeature/sdk/Hook.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package dev.openfeature.sdk;

import dev.openfeature.sdk.exceptions.ExceptionUtils;
import java.util.Map;
import java.util.Optional;

Expand Down Expand Up @@ -35,9 +36,22 @@ default void after(HookContext<T> ctx, FlagEvaluationDetails<T> details, Map<Str
* 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.
* @param error The error details might contain an exception that was thrown.
* @param hints An immutable mapping of data for users to communicate to the hooks.
*/
default void error(HookContext<T> ctx, ErrorDetails<T> error, Map<String, Object> 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<T> ctx, Exception error, Map<String, Object> hints) {}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/dev/openfeature/sdk/HookSupport.java
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public void afterAllHooks(
public void errorHooks(
FlagValueType flagValueType,
HookContext hookCtx,
Exception e,
ErrorDetails e,
List<Hook> hooks,
Map<String, Object> hints) {
executeHooks(flagValueType, hooks, "error", hook -> hook.error(hookCtx, e, hints));
Expand Down
14 changes: 3 additions & 11 deletions src/main/java/dev/openfeature/sdk/OpenFeatureClient.java
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -204,9 +202,8 @@ private <T> FlagEvaluationDetails<T> 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);
Expand All @@ -215,14 +212,9 @@ private <T> FlagEvaluationDetails<T> evaluateFlag(
if (details == null) {
details = FlagEvaluationDetails.<T>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);
}
Expand Down
38 changes: 19 additions & 19 deletions src/test/java/dev/openfeature/sdk/HookSpecTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -225,16 +223,16 @@ void error_hook_must_run_if_resolution_details_returns_an_error_code() {
invocationCtx,
FlagEvaluationOptions.builder().hook(hook).build());

ArgumentCaptor<Exception> captor = ArgumentCaptor.forClass(Exception.class);
ArgumentCaptor<ErrorDetails> 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(
Expand Down Expand Up @@ -279,7 +277,8 @@ public void after(
}

@Override
public void error(HookContext<Boolean> ctx, Exception error, Map<String, Object> hints) {
public void error(
HookContext<Boolean> ctx, ErrorDetails<Boolean> error, Map<String, Object> hints) {
evalOrder.add("provider error");
}

Expand Down Expand Up @@ -308,7 +307,7 @@ public void after(
}

@Override
public void error(HookContext<Boolean> ctx, Exception error, Map<String, Object> hints) {
public void error(HookContext<Boolean> ctx, ErrorDetails<Boolean> error, Map<String, Object> hints) {
evalOrder.add("api error");
}

Expand All @@ -334,7 +333,7 @@ public void after(
}

@Override
public void error(HookContext<Boolean> ctx, Exception error, Map<String, Object> hints) {
public void error(HookContext<Boolean> ctx, ErrorDetails<Boolean> error, Map<String, Object> hints) {
evalOrder.add("client error");
}

Expand Down Expand Up @@ -367,7 +366,8 @@ public void after(
}

@Override
public void error(HookContext<Boolean> ctx, Exception error, Map<String, Object> hints) {
public void error(
HookContext<Boolean> ctx, ErrorDetails<Boolean> error, Map<String, Object> hints) {
evalOrder.add("invocation error");
}

Expand Down Expand Up @@ -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.");
}

Expand All @@ -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
Expand Down Expand Up @@ -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));
}

Expand Down Expand Up @@ -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.")
Expand Down Expand Up @@ -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);

Expand All @@ -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) {
Expand Down
23 changes: 7 additions & 16 deletions src/test/java/dev/openfeature/sdk/HookSupportTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -55,31 +55,22 @@ void shouldAlwaysCallGenericHook(FlagValueType flagValueType) {
() -> "client",
() -> "provider");

FlagEvaluationDetails<Object> details = FlagEvaluationDetails.builder().build();
ErrorDetails<Object> 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) {
Expand Down
Loading