From 4904bf8dfe0b16af2292aa13c55db95de63c93fb Mon Sep 17 00:00:00 2001 From: Thomas Poignant Date: Sun, 11 Jan 2026 12:09:27 +0100 Subject: [PATCH] fix(gofeatureflag): fix unreachable code when flag not found - Move FLAG_NOT_FOUND check before general error handling in EvaluationService - This makes the FlagNotFoundError exception reachable when a flag is not found - Add unit tests to validate the fix Fixes #1618 Signed-off-by: Thomas Poignant --- .../service/EvaluationService.java | 10 +- .../service/EvaluationServiceTest.java | 101 ++++++++++++++++++ 2 files changed, 107 insertions(+), 4 deletions(-) create mode 100644 providers/go-feature-flag/src/test/java/dev/openfeature/contrib/providers/gofeatureflag/service/EvaluationServiceTest.java diff --git a/providers/go-feature-flag/src/main/java/dev/openfeature/contrib/providers/gofeatureflag/service/EvaluationService.java b/providers/go-feature-flag/src/main/java/dev/openfeature/contrib/providers/gofeatureflag/service/EvaluationService.java index 56bc24d35..da452d39f 100644 --- a/providers/go-feature-flag/src/main/java/dev/openfeature/contrib/providers/gofeatureflag/service/EvaluationService.java +++ b/providers/go-feature-flag/src/main/java/dev/openfeature/contrib/providers/gofeatureflag/service/EvaluationService.java @@ -68,6 +68,12 @@ public ProviderEvaluation getEvaluation( val goffResp = evaluator.evaluate(flagKey, defaultValue, evaluationContext); + // Check for FLAG_NOT_FOUND error first, before general error handling + if (goffResp.getErrorCode() != null + && ErrorCode.FLAG_NOT_FOUND.name().equalsIgnoreCase(goffResp.getErrorCode())) { + throw new FlagNotFoundError("Flag " + flagKey + " was not found in your configuration"); + } + // If we have an error code, we return the error directly. if (goffResp.getErrorCode() != null && !goffResp.getErrorCode().isEmpty()) { return ProviderEvaluation.builder() @@ -88,10 +94,6 @@ public ProviderEvaluation getEvaluation( .build(); } - if (ErrorCode.FLAG_NOT_FOUND.name().equalsIgnoreCase(goffResp.getErrorCode())) { - throw new FlagNotFoundError("Flag " + flagKey + " was not found in your configuration"); - } - // Convert the value received from the API. T flagValue = convertValue(goffResp.getValue(), expectedType); diff --git a/providers/go-feature-flag/src/test/java/dev/openfeature/contrib/providers/gofeatureflag/service/EvaluationServiceTest.java b/providers/go-feature-flag/src/test/java/dev/openfeature/contrib/providers/gofeatureflag/service/EvaluationServiceTest.java new file mode 100644 index 000000000..fb6df43ea --- /dev/null +++ b/providers/go-feature-flag/src/test/java/dev/openfeature/contrib/providers/gofeatureflag/service/EvaluationServiceTest.java @@ -0,0 +1,101 @@ +package dev.openfeature.contrib.providers.gofeatureflag.service; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import dev.openfeature.contrib.providers.gofeatureflag.bean.GoFeatureFlagResponse; +import dev.openfeature.contrib.providers.gofeatureflag.evaluator.IEvaluator; +import dev.openfeature.sdk.ErrorCode; +import dev.openfeature.sdk.EvaluationContext; +import dev.openfeature.sdk.ImmutableContext; +import dev.openfeature.sdk.ProviderEvaluation; +import dev.openfeature.sdk.Reason; +import dev.openfeature.sdk.exceptions.FlagNotFoundError; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +@DisplayName("EvaluationService tests") +class EvaluationServiceTest { + private IEvaluator mockEvaluator; + private EvaluationService evaluationService; + private EvaluationContext evaluationContext; + + @BeforeEach + void setUp() { + mockEvaluator = mock(IEvaluator.class); + evaluationService = new EvaluationService(mockEvaluator); + evaluationContext = new ImmutableContext("test-targeting-key"); + } + + @DisplayName("Should throw FlagNotFoundError when flag is not found") + @Test + void shouldThrowFlagNotFoundErrorWhenFlagIsNotFound() { + // Given: evaluator returns a response with FLAG_NOT_FOUND error code + GoFeatureFlagResponse response = new GoFeatureFlagResponse(); + response.setErrorCode(ErrorCode.FLAG_NOT_FOUND.name()); + response.setErrorDetails("Flag test-flag was not found in your configuration"); + response.setValue(false); + + when(mockEvaluator.evaluate(anyString(), any(), any(EvaluationContext.class))) + .thenReturn(response); + + // When/Then: getEvaluation should throw FlagNotFoundError + FlagNotFoundError exception = assertThrows( + FlagNotFoundError.class, + () -> evaluationService.getEvaluation("test-flag", false, evaluationContext, Boolean.class)); + + assertEquals("Flag test-flag was not found in your configuration", exception.getMessage()); + } + + @DisplayName("Should return error response for other error codes") + @Test + void shouldReturnErrorResponseForOtherErrorCodes() { + // Given: evaluator returns a response with a different error code + GoFeatureFlagResponse response = new GoFeatureFlagResponse(); + response.setErrorCode(ErrorCode.GENERAL.name()); + response.setErrorDetails("Some other error occurred"); + response.setValue(false); + + when(mockEvaluator.evaluate(anyString(), any(), any(EvaluationContext.class))) + .thenReturn(response); + + // When: getEvaluation is called + ProviderEvaluation result = + evaluationService.getEvaluation("test-flag", false, evaluationContext, Boolean.class); + + // Then: should return error response, not throw exception + assertEquals(ErrorCode.GENERAL, result.getErrorCode()); + assertEquals("Some other error occurred", result.getErrorMessage()); + assertEquals(Reason.ERROR.name(), result.getReason()); + assertEquals(false, result.getValue()); + } + + @DisplayName("Should handle successful evaluation") + @Test + void shouldHandleSuccessfulEvaluation() { + // Given: evaluator returns a successful response + GoFeatureFlagResponse response = new GoFeatureFlagResponse(); + response.setValue(true); + response.setReason(Reason.TARGETING_MATCH.name()); + response.setVariationType("enabled"); + response.setErrorCode(null); + + when(mockEvaluator.evaluate(anyString(), any(), any(EvaluationContext.class))) + .thenReturn(response); + + // When: getEvaluation is called + ProviderEvaluation result = + evaluationService.getEvaluation("test-flag", false, evaluationContext, Boolean.class); + + // Then: should return successful evaluation + assertEquals(true, result.getValue()); + assertEquals(Reason.TARGETING_MATCH.name(), result.getReason()); + assertEquals("enabled", result.getVariant()); + assertEquals(null, result.getErrorCode()); + } +}