Repository: open-feature/java-sdk (java)

Issue:
[BUG] Error hooks only run if the provider throws
See the issue reported for JS SDK [^1]

Similarly, we need to check details for error and execute error hook in Java SDK [^2]

[^1]: - https://github.com/open-feature/js-sdk/issues/925
[^2]: - https://github.com/open-feature/java-sdk/blob/v1.8.0/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java#L126-L141

Pull request:
fix: run error hook when provider returns error code

## This PR

- ensures that the error hook is ran when the provider returns an error code

### Related Issues

Fixes #928 

### Notes

saw this bug and seemed like a straightforward fix, wasn't sure if anyone has already picked it up so feel free to reject if it's already been taken on


Patch:
diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java
index b03ef6a..452c98d 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.GeneralError;
 import dev.openfeature.sdk.exceptions.OpenFeatureError;
 import dev.openfeature.sdk.internal.AutoCloseableLock;
 import dev.openfeature.sdk.internal.AutoCloseableReentrantReadWriteLock;
+import dev.openfeature.sdk.exceptions.ExceptionUtils;
 import dev.openfeature.sdk.internal.ObjectUtils;
 import lombok.Getter;
 import lombok.extern.slf4j.Slf4j;
@@ -125,7 +126,11 @@ public class OpenFeatureClient implements Client {
                     defaultValue, provider, mergedCtx);
 
             details = FlagEvaluationDetails.from(providerEval, key);
-            hookSupport.afterHooks(type, hookCtx, details, mergedHooks, hints);
+            if (details.getErrorCode() != null) {
+                throw ExceptionUtils.instantiateErrorByErrorCode(details.getErrorCode(), details.getErrorMessage());
+            } 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/exceptions/ExceptionUtils.java b/src/main/java/dev/openfeature/sdk/exceptions/ExceptionUtils.java
new file mode 100644
index 0000000..28c7cd7
--- /dev/null
+++ b/src/main/java/dev/openfeature/sdk/exceptions/ExceptionUtils.java
@@ -0,0 +1,34 @@
+package dev.openfeature.sdk.exceptions;
+
+import dev.openfeature.sdk.ErrorCode;
+import lombok.experimental.UtilityClass;
+
+@SuppressWarnings("checkstyle:MissingJavadocType")
+@UtilityClass
+public class ExceptionUtils {
+
+    /**
+     * 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);
+        }
+    }
+}


Test patch:
diff --git a/src/test/java/dev/openfeature/sdk/HookSpecTest.java b/src/test/java/dev/openfeature/sdk/HookSpecTest.java
index d0e173b..098d80d 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 java.util.Optional;
 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 @@ class HookSpecTest implements HookFixtures {
         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.")
diff --git a/src/test/java/dev/openfeature/sdk/exceptions/ExceptionUtilsTest.java b/src/test/java/dev/openfeature/sdk/exceptions/ExceptionUtilsTest.java
new file mode 100644
index 0000000..58e5945
--- /dev/null
+++ b/src/test/java/dev/openfeature/sdk/exceptions/ExceptionUtilsTest.java
@@ -0,0 +1,45 @@
+package dev.openfeature.sdk.exceptions;
+
+import dev.openfeature.sdk.ErrorCode;
+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 ExceptionUtilsTest {
+
+    @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 = ExceptionUtils.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)
+            );
+        }
+    }
+}