From 2cd1b9b0cd477b42accf4cb91648638ca4cf82ff Mon Sep 17 00:00:00 2001 From: Parth Suthar Date: Wed, 18 Mar 2026 15:29:00 -0400 Subject: [PATCH 1/2] fix: collect and propagate per-provider errors in multi-provider strategies Signed-off-by: Parth Suthar --- .../sdk/multiprovider/FirstMatchStrategy.java | 36 ++++- .../FirstSuccessfulStrategy.java | 36 ++++- .../MultiProviderEvaluation.java | 122 +++++++++++++++ .../sdk/multiprovider/ProviderError.java | 56 +++++++ .../sdk/multiprovider/Strategy.java | 4 + .../sdk/multiprovider/BaseStrategyTest.java | 13 ++ .../multiprovider/FirstMatchStrategyTest.java | 72 ++++++++- .../FirstSuccessfulStrategyTest.java | 148 ++++++++++++++++-- .../sdk/multiprovider/MultiProviderTest.java | 6 +- 9 files changed, 458 insertions(+), 35 deletions(-) create mode 100644 src/main/java/dev/openfeature/sdk/multiprovider/MultiProviderEvaluation.java create mode 100644 src/main/java/dev/openfeature/sdk/multiprovider/ProviderError.java diff --git a/src/main/java/dev/openfeature/sdk/multiprovider/FirstMatchStrategy.java b/src/main/java/dev/openfeature/sdk/multiprovider/FirstMatchStrategy.java index 8bfdec76d..d3b6f6cad 100644 --- a/src/main/java/dev/openfeature/sdk/multiprovider/FirstMatchStrategy.java +++ b/src/main/java/dev/openfeature/sdk/multiprovider/FirstMatchStrategy.java @@ -7,8 +7,11 @@ import dev.openfeature.sdk.FeatureProvider; import dev.openfeature.sdk.ProviderEvaluation; import dev.openfeature.sdk.exceptions.FlagNotFoundError; +import java.util.ArrayList; +import java.util.List; import java.util.Map; import java.util.function.Function; +import java.util.stream.Collectors; import lombok.NoArgsConstructor; import lombok.extern.slf4j.Slf4j; @@ -20,7 +23,8 @@ *
  • Skip providers that indicate they had no value due to {@code FLAG_NOT_FOUND}.
    • *
  • On any other error code, return that error result.
    • *
  • If a provider throws {@link FlagNotFoundError}, it is treated like {@code FLAG_NOT_FOUND}.
    • - *
  • If all providers report {@code FLAG_NOT_FOUND}, return a {@code FLAG_NOT_FOUND} error.
    • + *
  • If all providers report {@code FLAG_NOT_FOUND}, return a {@code FLAG_NOT_FOUND} error + * with per-provider error details.
    • * * As soon as a non-{@code FLAG_NOT_FOUND} result is returned by a provider (success or other error), * the rest of the operation short-circuits and does not call the remaining providers. @@ -36,7 +40,11 @@ public ProviderEvaluation evaluate( T defaultValue, EvaluationContext ctx, Function> providerFunction) { - for (FeatureProvider provider : providers.values()) { + List collectedErrors = new ArrayList<>(); + + for (Map.Entry entry : providers.entrySet()) { + String providerName = entry.getKey(); + FeatureProvider provider = entry.getValue(); try { ProviderEvaluation res = providerFunction.apply(provider); ErrorCode errorCode = res.getErrorCode(); @@ -45,19 +53,31 @@ public ProviderEvaluation evaluate( return res; } if (!FLAG_NOT_FOUND.equals(errorCode)) { - // Any non-FLAG_NOT_FOUND error bubbles up + // Any non-FLAG_NOT_FOUND error bubbles up immediately return res; } - // else FLAG_NOT_FOUND: skip to next provider - } catch (FlagNotFoundError ignored) { - // do not log in hot path, just skip + // FLAG_NOT_FOUND: record and skip to next provider + collectedErrors.add(ProviderError.fromResult(providerName, FLAG_NOT_FOUND, res.getErrorMessage())); + } catch (FlagNotFoundError e) { + // Treat thrown FlagNotFoundError like a FLAG_NOT_FOUND result + collectedErrors.add(ProviderError.fromException(providerName, e)); } } // All providers either threw or returned FLAG_NOT_FOUND - return ProviderEvaluation.builder() - .errorMessage("Flag not found in any provider") + String aggregateMessage = buildAggregateMessage(collectedErrors); + return MultiProviderEvaluation.multiProviderBuilder() + .errorMessage(aggregateMessage) .errorCode(FLAG_NOT_FOUND) + .providerErrors(collectedErrors) .build(); } + + private static String buildAggregateMessage(List errors) { + if (errors.isEmpty()) { + return "Flag not found in any provider"; + } + String details = errors.stream().map(ProviderError::toString).collect(Collectors.joining(", ")); + return "Flag not found in any provider. Provider errors: [" + details + "]"; + } } diff --git a/src/main/java/dev/openfeature/sdk/multiprovider/FirstSuccessfulStrategy.java b/src/main/java/dev/openfeature/sdk/multiprovider/FirstSuccessfulStrategy.java index 6a3fc4433..e455c165d 100644 --- a/src/main/java/dev/openfeature/sdk/multiprovider/FirstSuccessfulStrategy.java +++ b/src/main/java/dev/openfeature/sdk/multiprovider/FirstSuccessfulStrategy.java @@ -4,17 +4,21 @@ import dev.openfeature.sdk.EvaluationContext; import dev.openfeature.sdk.FeatureProvider; import dev.openfeature.sdk.ProviderEvaluation; +import java.util.ArrayList; +import java.util.List; import java.util.Map; import java.util.function.Function; +import java.util.stream.Collectors; import lombok.NoArgsConstructor; import lombok.extern.slf4j.Slf4j; /** * First Successful Strategy. * - *

    Similar to “First Match”, except that errors from evaluated providers do not halt execution. + *

    Similar to "First Match", except that errors from evaluated providers do not halt execution. * Instead, it returns the first successful result from a provider. If no provider successfully - * responds, it returns a {@code GENERAL} error result. + * responds, it returns a {@code GENERAL} error result that includes per-provider error details + * describing why each provider failed. */ @Slf4j @NoArgsConstructor @@ -27,22 +31,38 @@ public ProviderEvaluation evaluate( T defaultValue, EvaluationContext ctx, Function> providerFunction) { - for (FeatureProvider provider : providers.values()) { + List collectedErrors = new ArrayList<>(); + + for (Map.Entry entry : providers.entrySet()) { + String providerName = entry.getKey(); + FeatureProvider provider = entry.getValue(); try { ProviderEvaluation res = providerFunction.apply(provider); if (res.getErrorCode() == null) { // First successful result (no error code) return res; } - } catch (Exception ignored) { - // swallow and continue; errors from individual providers - // are not fatal for this strategy + // Record error-coded result + collectedErrors.add(ProviderError.fromResult(providerName, res.getErrorCode(), res.getErrorMessage())); + } catch (Exception e) { + // Record thrown exception + collectedErrors.add(ProviderError.fromException(providerName, e)); } } - return ProviderEvaluation.builder() - .errorMessage("No provider successfully responded") + String aggregateMessage = buildAggregateMessage(collectedErrors); + return MultiProviderEvaluation.multiProviderBuilder() + .errorMessage(aggregateMessage) .errorCode(ErrorCode.GENERAL) + .providerErrors(collectedErrors) .build(); } + + private static String buildAggregateMessage(List errors) { + if (errors.isEmpty()) { + return "No provider successfully responded"; + } + String details = errors.stream().map(ProviderError::toString).collect(Collectors.joining(", ")); + return "No provider successfully responded. Provider errors: [" + details + "]"; + } } diff --git a/src/main/java/dev/openfeature/sdk/multiprovider/MultiProviderEvaluation.java b/src/main/java/dev/openfeature/sdk/multiprovider/MultiProviderEvaluation.java new file mode 100644 index 000000000..737eb6353 --- /dev/null +++ b/src/main/java/dev/openfeature/sdk/multiprovider/MultiProviderEvaluation.java @@ -0,0 +1,122 @@ +package dev.openfeature.sdk.multiprovider; + +import dev.openfeature.sdk.ErrorCode; +import dev.openfeature.sdk.ImmutableMetadata; +import dev.openfeature.sdk.ProviderEvaluation; +import java.util.Collections; +import java.util.List; + +/** + * A {@link ProviderEvaluation} subtype returned by multi-provider strategies when all providers + * fail to produce a successful result. + * + *

    This type carries per-provider error details so that callers can inspect why each individual + * provider failed. It is only returned in error scenarios; successful evaluations always return + * a plain {@link ProviderEvaluation}. + * + *

    Usage: + * 

    {@code
+ * ProviderEvaluation result = strategy.evaluate(...);
+ * if (result instanceof MultiProviderEvaluation multiResult) {
+ *     for (ProviderError error : multiResult.getProviderErrors()) {
+ *         log.warn("Provider {} failed: {} - {}",
+ *             error.getProviderName(), error.getErrorCode(), error.getErrorMessage());
+ *     }
+ * }
+ * }
    + * + * @param the type of the flag being evaluated + */ +public class MultiProviderEvaluation extends ProviderEvaluation { + + private final List providerErrors; + + private MultiProviderEvaluation( + T value, + String variant, + String reason, + ErrorCode errorCode, + String errorMessage, + ImmutableMetadata flagMetadata, + List providerErrors) { + super(value, variant, reason, errorCode, errorMessage, flagMetadata); + this.providerErrors = + providerErrors != null ? Collections.unmodifiableList(providerErrors) : Collections.emptyList(); + } + + /** + * Returns the per-provider error details. + * + *

    Each entry describes why a specific provider failed during multi-provider evaluation. + * + * @return an unmodifiable list of per-provider errors, never {@code null} + */ + public List getProviderErrors() { + return providerErrors; + } + + /** + * Create a new builder for {@link MultiProviderEvaluation}. + * + * @param the flag value type + * @return a new builder + */ + public static Builder multiProviderBuilder() { + return new Builder<>(); + } + + /** + * Builder for {@link MultiProviderEvaluation}. + * + * @param the flag value type + */ + public static class Builder { + private T value; + private String variant; + private String reason; + private ErrorCode errorCode; + private String errorMessage; + private ImmutableMetadata flagMetadata; + private List providerErrors; + + public Builder value(T value) { + this.value = value; + return this; + } + + public Builder variant(String variant) { + this.variant = variant; + return this; + } + + public Builder reason(String reason) { + this.reason = reason; + return this; + } + + public Builder errorCode(ErrorCode errorCode) { + this.errorCode = errorCode; + return this; + } + + public Builder errorMessage(String errorMessage) { + this.errorMessage = errorMessage; + return this; + } + + public Builder flagMetadata(ImmutableMetadata flagMetadata) { + this.flagMetadata = flagMetadata; + return this; + } + + public Builder providerErrors(List providerErrors) { + this.providerErrors = providerErrors; + return this; + } + + public MultiProviderEvaluation build() { + return new MultiProviderEvaluation<>( + value, variant, reason, errorCode, errorMessage, flagMetadata, providerErrors); + } + } +} diff --git a/src/main/java/dev/openfeature/sdk/multiprovider/ProviderError.java b/src/main/java/dev/openfeature/sdk/multiprovider/ProviderError.java new file mode 100644 index 000000000..9d4f63965 --- /dev/null +++ b/src/main/java/dev/openfeature/sdk/multiprovider/ProviderError.java @@ -0,0 +1,56 @@ +package dev.openfeature.sdk.multiprovider; + +import dev.openfeature.sdk.ErrorCode; +import dev.openfeature.sdk.exceptions.OpenFeatureError; +import lombok.AllArgsConstructor; +import lombok.Builder; +import lombok.Data; + +/** + * Represents an error from a single provider during multi-provider evaluation. + * + *

    Captures the provider name, error code, error message, and optionally the original exception + * that occurred during flag evaluation. This allows callers to inspect per-provider error details + * when a multi-provider strategy exhausts all providers without a successful result. + */ +@Data +@Builder +@AllArgsConstructor +public class ProviderError { + private String providerName; + private ErrorCode errorCode; + private String errorMessage; + private Exception exception; + + /** + * Create a ProviderError from an error-coded {@code ProviderEvaluation} result. + * + * @param providerName the name of the provider that returned the error + * @param errorCode the error code from the evaluation result + * @param errorMessage the error message from the evaluation result (may be {@code null}) + * @return a new ProviderError + */ + public static ProviderError fromResult(String providerName, ErrorCode errorCode, String errorMessage) { + return new ProviderError(providerName, errorCode, errorMessage, null); + } + + /** + * Create a ProviderError from a thrown exception. + * + * @param providerName the name of the provider that threw the exception + * @param exception the exception that was thrown + * @return a new ProviderError + */ + public static ProviderError fromException(String providerName, Exception exception) { + ErrorCode code = ErrorCode.GENERAL; + if (exception instanceof OpenFeatureError) { + code = ((OpenFeatureError) exception).getErrorCode(); + } + return new ProviderError(providerName, code, exception.getMessage(), exception); + } + + @Override + public String toString() { + return providerName + ": " + errorCode + " (" + (errorMessage != null ? errorMessage : "unknown") + ")"; + } +} diff --git a/src/main/java/dev/openfeature/sdk/multiprovider/Strategy.java b/src/main/java/dev/openfeature/sdk/multiprovider/Strategy.java index 4c25fe8f0..31b769449 100644 --- a/src/main/java/dev/openfeature/sdk/multiprovider/Strategy.java +++ b/src/main/java/dev/openfeature/sdk/multiprovider/Strategy.java @@ -14,6 +14,10 @@ *

  • Order or select providers
    • *
  • Handle {@code FLAG_NOT_FOUND} results
    • *
  • Handle errors and exceptions from providers
    • + *
  • Collect per-provider error details when no provider returns a successful result. + * Implementations should return a {@link MultiProviderEvaluation} populated with + * a {@link ProviderError} for each failed provider, so that callers can inspect individual + * failure reasons.
    • * */ public interface Strategy { diff --git a/src/test/java/dev/openfeature/sdk/multiprovider/BaseStrategyTest.java b/src/test/java/dev/openfeature/sdk/multiprovider/BaseStrategyTest.java index 405a2f094..a7070fd11 100644 --- a/src/test/java/dev/openfeature/sdk/multiprovider/BaseStrategyTest.java +++ b/src/test/java/dev/openfeature/sdk/multiprovider/BaseStrategyTest.java @@ -211,4 +211,17 @@ protected void setupProviderSuccess(FeatureProvider provider, String value) { when(provider.getStringEvaluation(BaseStrategyTest.FLAG_KEY, DEFAULT_STRING, null)) .thenReturn(result); } + + protected void setupProviderErrorWithMessage(FeatureProvider provider, ErrorCode errorCode, String errorMessage) { + ProviderEvaluation result = mock(ProviderEvaluation.class); + when(result.getErrorCode()).thenReturn(errorCode); + when(result.getErrorMessage()).thenReturn(errorMessage); + when(provider.getStringEvaluation(BaseStrategyTest.FLAG_KEY, DEFAULT_STRING, null)) + .thenReturn(result); + } + + protected void setupProviderException(FeatureProvider provider, RuntimeException exception) { + when(provider.getStringEvaluation(BaseStrategyTest.FLAG_KEY, DEFAULT_STRING, null)) + .thenThrow(exception); + } } diff --git a/src/test/java/dev/openfeature/sdk/multiprovider/FirstMatchStrategyTest.java b/src/test/java/dev/openfeature/sdk/multiprovider/FirstMatchStrategyTest.java index 0205961cd..95a1d603e 100644 --- a/src/test/java/dev/openfeature/sdk/multiprovider/FirstMatchStrategyTest.java +++ b/src/test/java/dev/openfeature/sdk/multiprovider/FirstMatchStrategyTest.java @@ -1,11 +1,15 @@ package dev.openfeature.sdk.multiprovider; 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.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; import dev.openfeature.sdk.ErrorCode; import dev.openfeature.sdk.ProviderEvaluation; +import dev.openfeature.sdk.exceptions.FlagNotFoundError; +import java.util.List; import org.junit.jupiter.api.Test; class FirstMatchStrategyTest extends BaseStrategyTest { @@ -59,19 +63,30 @@ void shouldReturnSuccessWhenFirstProviderSucceeds() { } @Test - void shouldThrowFlagNotFoundWhenAllProvidersReturnFlagNotFound() { + void shouldReturnMultiProviderEvaluationWhenAllProvidersReturnFlagNotFound() { setupProviderFlagNotFound(mockProvider1); setupProviderFlagNotFound(mockProvider2); setupProviderFlagNotFound(mockProvider3); - ProviderEvaluation providerEvaluation = strategy.evaluate( + ProviderEvaluation result = strategy.evaluate( orderedProviders, FLAG_KEY, DEFAULT_STRING, null, p -> p.getStringEvaluation(FLAG_KEY, DEFAULT_STRING, null)); - assertEquals(ErrorCode.FLAG_NOT_FOUND, providerEvaluation.getErrorCode()); - assertEquals("Flag not found in any provider", providerEvaluation.getErrorMessage()); + assertEquals(ErrorCode.FLAG_NOT_FOUND, result.getErrorCode()); + assertTrue(result.getErrorMessage().contains("Flag not found in any provider")); + + MultiProviderEvaluation multiResult = assertInstanceOf(MultiProviderEvaluation.class, result); + List errors = multiResult.getProviderErrors(); + assertNotNull(errors); + assertEquals(3, errors.size()); + assertEquals("provider1", errors.get(0).getProviderName()); + assertEquals(ErrorCode.FLAG_NOT_FOUND, errors.get(0).getErrorCode()); + assertEquals("provider2", errors.get(1).getProviderName()); + assertEquals(ErrorCode.FLAG_NOT_FOUND, errors.get(1).getErrorCode()); + assertEquals("provider3", errors.get(2).getProviderName()); + assertEquals(ErrorCode.FLAG_NOT_FOUND, errors.get(2).getErrorCode()); } @Test @@ -88,4 +103,53 @@ void shouldSkipMultipleFlagNotFoundAndReturnFirstOtherError() { assertNotNull(result); assertEquals(ErrorCode.PARSE_ERROR, result.getErrorCode()); } + + @Test + void shouldCaptureThrownFlagNotFoundErrorsAsProviderErrors() { + setupProviderException(mockProvider1, new FlagNotFoundError("not in provider1")); + setupProviderException(mockProvider2, new FlagNotFoundError("not in provider2")); + setupProviderException(mockProvider3, new FlagNotFoundError("not in provider3")); + + ProviderEvaluation result = strategy.evaluate( + orderedProviders, + FLAG_KEY, + DEFAULT_STRING, + null, + p -> p.getStringEvaluation(FLAG_KEY, DEFAULT_STRING, null)); + + assertEquals(ErrorCode.FLAG_NOT_FOUND, result.getErrorCode()); + + MultiProviderEvaluation multiResult = assertInstanceOf(MultiProviderEvaluation.class, result); + List errors = multiResult.getProviderErrors(); + assertNotNull(errors); + assertEquals(3, errors.size()); + + assertEquals("provider1", errors.get(0).getProviderName()); + assertEquals(ErrorCode.FLAG_NOT_FOUND, errors.get(0).getErrorCode()); + assertEquals("not in provider1", errors.get(0).getErrorMessage()); + assertNotNull(errors.get(0).getException()); + + assertEquals("provider2", errors.get(1).getProviderName()); + assertEquals("provider3", errors.get(2).getProviderName()); + } + + @Test + void shouldIncludeProviderNamesInAggregateErrorMessage() { + setupProviderFlagNotFound(mockProvider1); + setupProviderFlagNotFound(mockProvider2); + setupProviderFlagNotFound(mockProvider3); + + ProviderEvaluation result = strategy.evaluate( + orderedProviders, + FLAG_KEY, + DEFAULT_STRING, + null, + p -> p.getStringEvaluation(FLAG_KEY, DEFAULT_STRING, null)); + + String message = result.getErrorMessage(); + assertTrue(message.contains("provider1")); + assertTrue(message.contains("provider2")); + assertTrue(message.contains("provider3")); + assertTrue(message.contains("FLAG_NOT_FOUND")); + } } diff --git a/src/test/java/dev/openfeature/sdk/multiprovider/FirstSuccessfulStrategyTest.java b/src/test/java/dev/openfeature/sdk/multiprovider/FirstSuccessfulStrategyTest.java index a47af8a2f..157d9e8ce 100644 --- a/src/test/java/dev/openfeature/sdk/multiprovider/FirstSuccessfulStrategyTest.java +++ b/src/test/java/dev/openfeature/sdk/multiprovider/FirstSuccessfulStrategyTest.java @@ -1,11 +1,15 @@ package dev.openfeature.sdk.multiprovider; 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.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; import dev.openfeature.sdk.ErrorCode; import dev.openfeature.sdk.ProviderEvaluation; +import dev.openfeature.sdk.exceptions.GeneralError; +import java.util.List; import org.junit.jupiter.api.Test; class FirstSuccessfulStrategyTest extends BaseStrategyTest { @@ -25,22 +29,42 @@ void shouldSkipFlagNotFoundAndReturnFirstSuccess() { assertNotNull(result); assertEquals("success", result.getValue()); assertNull(result.getErrorCode()); + // Successful results should NOT be MultiProviderEvaluation + assertTrue(!(result instanceof MultiProviderEvaluation)); } @Test - void shouldThrowGeneralErrorWhenAllProvidersFail() { - setupProviderFlagNotFound(mockProvider1); - setupProviderError(mockProvider2, ErrorCode.PARSE_ERROR); - setupProviderError(mockProvider3, ErrorCode.TYPE_MISMATCH); - ProviderEvaluation providerEvaluation = strategy.evaluate( + void shouldReturnMultiProviderEvaluationWithProviderDetailsWhenAllProvidersFail() { + setupProviderErrorWithMessage(mockProvider1, ErrorCode.FLAG_NOT_FOUND, "flag missing"); + setupProviderErrorWithMessage(mockProvider2, ErrorCode.PARSE_ERROR, "parse failed"); + setupProviderErrorWithMessage(mockProvider3, ErrorCode.TYPE_MISMATCH, "type mismatch"); + ProviderEvaluation result = strategy.evaluate( orderedProviders, FLAG_KEY, DEFAULT_STRING, null, p -> p.getStringEvaluation(FLAG_KEY, DEFAULT_STRING, null)); - assertEquals(ErrorCode.GENERAL, providerEvaluation.getErrorCode()); - assertEquals("No provider successfully responded", providerEvaluation.getErrorMessage()); + assertEquals(ErrorCode.GENERAL, result.getErrorCode()); + assertNotNull(result.getErrorMessage()); + assertTrue(result.getErrorMessage().contains("No provider successfully responded")); + + MultiProviderEvaluation multiResult = assertInstanceOf(MultiProviderEvaluation.class, result); + List errors = multiResult.getProviderErrors(); + assertNotNull(errors); + assertEquals(3, errors.size()); + + assertEquals("provider1", errors.get(0).getProviderName()); + assertEquals(ErrorCode.FLAG_NOT_FOUND, errors.get(0).getErrorCode()); + assertEquals("flag missing", errors.get(0).getErrorMessage()); + + assertEquals("provider2", errors.get(1).getProviderName()); + assertEquals(ErrorCode.PARSE_ERROR, errors.get(1).getErrorCode()); + assertEquals("parse failed", errors.get(1).getErrorMessage()); + + assertEquals("provider3", errors.get(2).getProviderName()); + assertEquals(ErrorCode.TYPE_MISMATCH, errors.get(2).getErrorCode()); + assertEquals("type mismatch", errors.get(2).getErrorMessage()); } @Test @@ -49,30 +73,126 @@ void shouldSkipProvidersThatOnlyReturnErrors() { setupProviderError(mockProvider2, ErrorCode.PROVIDER_NOT_READY); setupProviderError(mockProvider3, ErrorCode.GENERAL); - ProviderEvaluation providerEvaluation = strategy.evaluate( + ProviderEvaluation result = strategy.evaluate( orderedProviders, FLAG_KEY, DEFAULT_STRING, null, p -> p.getStringEvaluation(FLAG_KEY, DEFAULT_STRING, null)); - assertEquals(ErrorCode.GENERAL, providerEvaluation.getErrorCode()); - assertEquals("No provider successfully responded", providerEvaluation.getErrorMessage()); + assertEquals(ErrorCode.GENERAL, result.getErrorCode()); + assertTrue(result.getErrorMessage().contains("No provider successfully responded")); + + MultiProviderEvaluation multiResult = assertInstanceOf(MultiProviderEvaluation.class, result); + List errors = multiResult.getProviderErrors(); + assertNotNull(errors); + assertEquals(3, errors.size()); + assertEquals(ErrorCode.INVALID_CONTEXT, errors.get(0).getErrorCode()); + assertEquals(ErrorCode.PROVIDER_NOT_READY, errors.get(1).getErrorCode()); + assertEquals(ErrorCode.GENERAL, errors.get(2).getErrorCode()); } @Test - void shouldThrowGeneralErrorForNonExistentFlag() { + void shouldCaptureExceptionDetailsFromThrowingProviders() { + setupProviderException(mockProvider1, new GeneralError("connection timeout")); + setupProviderErrorWithMessage(mockProvider2, ErrorCode.PARSE_ERROR, "bad json"); + setupProviderException(mockProvider3, new RuntimeException("unexpected failure")); + + ProviderEvaluation result = strategy.evaluate( + orderedProviders, + FLAG_KEY, + DEFAULT_STRING, + null, + p -> p.getStringEvaluation(FLAG_KEY, DEFAULT_STRING, null)); + + assertEquals(ErrorCode.GENERAL, result.getErrorCode()); + + MultiProviderEvaluation multiResult = assertInstanceOf(MultiProviderEvaluation.class, result); + List errors = multiResult.getProviderErrors(); + assertNotNull(errors); + assertEquals(3, errors.size()); + + // First provider threw GeneralError + assertEquals("provider1", errors.get(0).getProviderName()); + assertEquals(ErrorCode.GENERAL, errors.get(0).getErrorCode()); + assertEquals("connection timeout", errors.get(0).getErrorMessage()); + assertNotNull(errors.get(0).getException()); + + // Second provider returned error-coded result + assertEquals("provider2", errors.get(1).getProviderName()); + assertEquals(ErrorCode.PARSE_ERROR, errors.get(1).getErrorCode()); + assertEquals("bad json", errors.get(1).getErrorMessage()); + assertNull(errors.get(1).getException()); + + // Third provider threw RuntimeException (non-OpenFeatureError) + assertEquals("provider3", errors.get(2).getProviderName()); + assertEquals(ErrorCode.GENERAL, errors.get(2).getErrorCode()); + assertEquals("unexpected failure", errors.get(2).getErrorMessage()); + assertNotNull(errors.get(2).getException()); + } + + @Test + void shouldReturnMultiProviderEvaluationForNonExistentFlag() { orderedProviders.clear(); orderedProviders.put("old-provider", inMemoryProvider1); orderedProviders.put("new-provider", inMemoryProvider2); - ProviderEvaluation providerEvaluation = strategy.evaluate( + ProviderEvaluation result = strategy.evaluate( + orderedProviders, + FLAG_KEY, + DEFAULT_STRING, + null, + p -> p.getStringEvaluation(FLAG_KEY, DEFAULT_STRING, null)); + + assertEquals(ErrorCode.GENERAL, result.getErrorCode()); + assertTrue(result.getErrorMessage().contains("No provider successfully responded")); + + // InMemoryProvider throws FlagNotFoundError, which should be captured + MultiProviderEvaluation multiResult = assertInstanceOf(MultiProviderEvaluation.class, result); + List errors = multiResult.getProviderErrors(); + assertNotNull(errors); + assertEquals(2, errors.size()); + assertEquals("old-provider", errors.get(0).getProviderName()); + assertEquals("new-provider", errors.get(1).getProviderName()); + } + + @Test + void shouldReturnFirstSuccessEvenAfterErrors() { + setupProviderError(mockProvider1, ErrorCode.PARSE_ERROR); + setupProviderException(mockProvider2, new GeneralError("timeout")); + setupProviderSuccess(mockProvider3, "finally-success"); + + ProviderEvaluation result = strategy.evaluate( + orderedProviders, + FLAG_KEY, + DEFAULT_STRING, + null, + p -> p.getStringEvaluation(FLAG_KEY, DEFAULT_STRING, null)); + + assertNotNull(result); + assertEquals("finally-success", result.getValue()); + assertNull(result.getErrorCode()); + assertTrue(!(result instanceof MultiProviderEvaluation)); + } + + @Test + void shouldIncludeProviderErrorDetailsInErrorMessage() { + setupProviderErrorWithMessage(mockProvider1, ErrorCode.PARSE_ERROR, "parse failed"); + setupProviderErrorWithMessage(mockProvider2, ErrorCode.GENERAL, "timeout"); + setupProviderErrorWithMessage(mockProvider3, ErrorCode.FLAG_NOT_FOUND, "not found"); + + ProviderEvaluation result = strategy.evaluate( orderedProviders, FLAG_KEY, DEFAULT_STRING, null, p -> p.getStringEvaluation(FLAG_KEY, DEFAULT_STRING, null)); - assertEquals(ErrorCode.GENERAL, providerEvaluation.getErrorCode()); - assertEquals("No provider successfully responded", providerEvaluation.getErrorMessage()); + String message = result.getErrorMessage(); + assertTrue(message.contains("provider1")); + assertTrue(message.contains("PARSE_ERROR")); + assertTrue(message.contains("provider2")); + assertTrue(message.contains("GENERAL")); + assertTrue(message.contains("provider3")); + assertTrue(message.contains("FLAG_NOT_FOUND")); } } diff --git a/src/test/java/dev/openfeature/sdk/multiprovider/MultiProviderTest.java b/src/test/java/dev/openfeature/sdk/multiprovider/MultiProviderTest.java index 887b71d0a..d69cd821f 100644 --- a/src/test/java/dev/openfeature/sdk/multiprovider/MultiProviderTest.java +++ b/src/test/java/dev/openfeature/sdk/multiprovider/MultiProviderTest.java @@ -115,7 +115,11 @@ void shouldUseDefaultFirstMatchStrategy() { ProviderEvaluation providerEvaluation = multiProvider.getStringEvaluation("non-existing", "", null); assertEquals(ErrorCode.FLAG_NOT_FOUND, providerEvaluation.getErrorCode()); - assertEquals("Flag not found in any provider", providerEvaluation.getErrorMessage()); + assertEquals( + "Flag not found in any provider. Provider errors: [" + + "old-provider: FLAG_NOT_FOUND (flag non-existing not found), " + + "new-provider: FLAG_NOT_FOUND (flag non-existing not found)]", + providerEvaluation.getErrorMessage()); } @SneakyThrows From db693860dadc89144acee18e0b558382d6aed0d7 Mon Sep 17 00:00:00 2001 From: Parth Suthar Date: Thu, 19 Mar 2026 02:03:37 -0400 Subject: [PATCH 2/2] update to refactor buildAggregateMessage in ProviderError Signed-off-by: Parth Suthar --- .../sdk/multiprovider/FirstMatchStrategy.java | 12 +----------- .../sdk/multiprovider/FirstSuccessfulStrategy.java | 13 ++----------- .../sdk/multiprovider/MultiProviderEvaluation.java | 11 ++++++----- .../sdk/multiprovider/ProviderError.java | 14 ++++++++++++++ 4 files changed, 23 insertions(+), 27 deletions(-) diff --git a/src/main/java/dev/openfeature/sdk/multiprovider/FirstMatchStrategy.java b/src/main/java/dev/openfeature/sdk/multiprovider/FirstMatchStrategy.java index d3b6f6cad..cf54bfdaa 100644 --- a/src/main/java/dev/openfeature/sdk/multiprovider/FirstMatchStrategy.java +++ b/src/main/java/dev/openfeature/sdk/multiprovider/FirstMatchStrategy.java @@ -11,7 +11,6 @@ import java.util.List; import java.util.Map; import java.util.function.Function; -import java.util.stream.Collectors; import lombok.NoArgsConstructor; import lombok.extern.slf4j.Slf4j; @@ -65,19 +64,10 @@ public ProviderEvaluation evaluate( } // All providers either threw or returned FLAG_NOT_FOUND - String aggregateMessage = buildAggregateMessage(collectedErrors); return MultiProviderEvaluation.multiProviderBuilder() - .errorMessage(aggregateMessage) + .errorMessage(ProviderError.buildAggregateMessage("Flag not found in any provider", collectedErrors)) .errorCode(FLAG_NOT_FOUND) .providerErrors(collectedErrors) .build(); } - - private static String buildAggregateMessage(List errors) { - if (errors.isEmpty()) { - return "Flag not found in any provider"; - } - String details = errors.stream().map(ProviderError::toString).collect(Collectors.joining(", ")); - return "Flag not found in any provider. Provider errors: [" + details + "]"; - } } diff --git a/src/main/java/dev/openfeature/sdk/multiprovider/FirstSuccessfulStrategy.java b/src/main/java/dev/openfeature/sdk/multiprovider/FirstSuccessfulStrategy.java index e455c165d..2f4e6d5ba 100644 --- a/src/main/java/dev/openfeature/sdk/multiprovider/FirstSuccessfulStrategy.java +++ b/src/main/java/dev/openfeature/sdk/multiprovider/FirstSuccessfulStrategy.java @@ -8,7 +8,6 @@ import java.util.List; import java.util.Map; import java.util.function.Function; -import java.util.stream.Collectors; import lombok.NoArgsConstructor; import lombok.extern.slf4j.Slf4j; @@ -50,19 +49,11 @@ public ProviderEvaluation evaluate( } } - String aggregateMessage = buildAggregateMessage(collectedErrors); return MultiProviderEvaluation.multiProviderBuilder() - .errorMessage(aggregateMessage) + .errorMessage( + ProviderError.buildAggregateMessage("No provider successfully responded", collectedErrors)) .errorCode(ErrorCode.GENERAL) .providerErrors(collectedErrors) .build(); } - - private static String buildAggregateMessage(List errors) { - if (errors.isEmpty()) { - return "No provider successfully responded"; - } - String details = errors.stream().map(ProviderError::toString).collect(Collectors.joining(", ")); - return "No provider successfully responded. Provider errors: [" + details + "]"; - } } diff --git a/src/main/java/dev/openfeature/sdk/multiprovider/MultiProviderEvaluation.java b/src/main/java/dev/openfeature/sdk/multiprovider/MultiProviderEvaluation.java index 737eb6353..9757604aa 100644 --- a/src/main/java/dev/openfeature/sdk/multiprovider/MultiProviderEvaluation.java +++ b/src/main/java/dev/openfeature/sdk/multiprovider/MultiProviderEvaluation.java @@ -7,12 +7,13 @@ import java.util.List; /** - * A {@link ProviderEvaluation} subtype returned by multi-provider strategies when all providers - * fail to produce a successful result. + * A {@link ProviderEvaluation} subtype returned by multi-provider strategies that carries + * per-provider error details. * - *

    This type carries per-provider error details so that callers can inspect why each individual - * provider failed. It is only returned in error scenarios; successful evaluations always return - * a plain {@link ProviderEvaluation}. + *

    This type can represent both successful and failed evaluations. When a strategy exhausts + * all providers without a successful result, the per-provider errors describe why each provider + * failed. Custom strategies may also use this type for successful results to surface information + * about providers that were skipped or failed before the successful one. * *

    Usage: * 

    {@code
diff --git a/src/main/java/dev/openfeature/sdk/multiprovider/ProviderError.java b/src/main/java/dev/openfeature/sdk/multiprovider/ProviderError.java
index 9d4f63965..921b881fa 100644
--- a/src/main/java/dev/openfeature/sdk/multiprovider/ProviderError.java
+++ b/src/main/java/dev/openfeature/sdk/multiprovider/ProviderError.java
@@ -2,6 +2,8 @@
 
 import dev.openfeature.sdk.ErrorCode;
 import dev.openfeature.sdk.exceptions.OpenFeatureError;
+import java.util.List;
+import java.util.stream.Collectors;
 import lombok.AllArgsConstructor;
 import lombok.Builder;
 import lombok.Data;
@@ -49,6 +51,18 @@ public static ProviderError fromException(String providerName, Exception excepti
         return new ProviderError(providerName, code, exception.getMessage(), exception);
     }
 
+    /**
+     * Build an aggregate error message from a list of provider errors.
+     *
+     * @param baseMessage the base message to use (e.g. "No provider successfully responded")
+     * @param errors      the list of per-provider errors
+     * @return an aggregate message including per-provider details
+     */
+    public static String buildAggregateMessage(String baseMessage, List errors) {
+        String details = errors.stream().map(ProviderError::toString).collect(Collectors.joining(", "));
+        return baseMessage + ". Provider errors: [" + details + "]";
+    }
+
     @Override
     public String toString() {
         return providerName + ": " + errorCode + " (" + (errorMessage != null ? errorMessage : "unknown") + ")";