From e4ee081bf59ebf584d219e55fa5cc6fb424cc656 Mon Sep 17 00:00:00 2001 From: Yu Chou Date: Tue, 18 Aug 2026 14:50:15 +0800 Subject: [PATCH] fix: carry errorCode through provider error events A handler registered with OpenFeatureAPI.onProviderError always saw getErrorCode() as null. Two places dropped it. EventDetails.fromProviderEventDetails, the only path from a provider-emitted ProviderEventDetails to the EventDetails handed to API-level handlers, copied flagsChanged, eventMetadata and message but not errorCode. It failed silently: EventDetails extends ProviderEventDetails, so getErrorCode() compiles and returns null rather than failing to compile, and there is no public way to observe a provider's events directly to work around it. OpenFeatureAPI.emitError, which raises PROVIDER_ERROR when a provider fails to initialise, built its ProviderEventDetails from the exception's message only, even though it holds an OpenFeatureError that exposes getErrorCode(). Errors the SDK raises itself therefore reached handlers with no code either, which is what spec 5.1.5 asks for. errorCode was the only field missing from the conversion: ProviderEventDetails declares four fields and the other three were already copied. emitReady still sets no error code, so PROVIDER_READY events are unchanged. EventsTest.shouldHaveAllProperties now asserts errorCode alongside the other fields, since that is the test that should have caught this. Fixes #2014 Signed-off-by: Yu Chou --- .../dev/openfeature/sdk/EventDetails.java | 1 + .../dev/openfeature/sdk/OpenFeatureAPI.java | 5 +- .../dev/openfeature/sdk/EventDetailsTest.java | 53 +++++++++++++++++++ .../java/dev/openfeature/sdk/EventsTest.java | 22 +++++++- 4 files changed, 79 insertions(+), 2 deletions(-) create mode 100644 src/test/java/dev/openfeature/sdk/EventDetailsTest.java diff --git a/src/main/java/dev/openfeature/sdk/EventDetails.java b/src/main/java/dev/openfeature/sdk/EventDetails.java index c75b046e0..fca5d4114 100644 --- a/src/main/java/dev/openfeature/sdk/EventDetails.java +++ b/src/main/java/dev/openfeature/sdk/EventDetails.java @@ -26,6 +26,7 @@ static EventDetails fromProviderEventDetails( .flagsChanged(providerEventDetails.getFlagsChanged()) .eventMetadata(providerEventDetails.getEventMetadata()) .message(providerEventDetails.getMessage()) + .errorCode(providerEventDetails.getErrorCode()) .build(); } } diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java index 4f8d7d6e4..06cb366e7 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java @@ -317,7 +317,10 @@ private void emitError(FeatureProvider provider, OpenFeatureError exception) { runHandlersForProvider( provider, ProviderEvent.PROVIDER_ERROR, - ProviderEventDetails.builder().message(exception.getMessage()).build()); + ProviderEventDetails.builder() + .message(exception.getMessage()) + .errorCode(exception.getErrorCode()) + .build()); } private void emitErrorAndThrow(FeatureProvider provider, OpenFeatureError exception) throws OpenFeatureError { diff --git a/src/test/java/dev/openfeature/sdk/EventDetailsTest.java b/src/test/java/dev/openfeature/sdk/EventDetailsTest.java new file mode 100644 index 000000000..63a7f5dbf --- /dev/null +++ b/src/test/java/dev/openfeature/sdk/EventDetailsTest.java @@ -0,0 +1,53 @@ +package dev.openfeature.sdk; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.Arrays; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +class EventDetailsTest { + + @Test + @DisplayName("should carry the error code a provider set, so API-level handlers can see it") + void shouldCopyErrorCode() { + ProviderEventDetails providerEventDetails = ProviderEventDetails.builder() + .errorCode(ErrorCode.PROVIDER_NOT_READY) + .build(); + + EventDetails details = EventDetails.fromProviderEventDetails(providerEventDetails, "provider"); + + assertThat(details.getErrorCode()).isEqualTo(ErrorCode.PROVIDER_NOT_READY); + } + + @Test + @DisplayName("should leave the error code unset when the provider did not set one") + void shouldLeaveErrorCodeUnsetWhenAbsent() { + EventDetails details = EventDetails.fromProviderEventDetails( + ProviderEventDetails.builder().build(), "provider"); + + assertThat(details.getErrorCode()).isNull(); + } + + @Test + @DisplayName("should carry every field of the provider event details") + void shouldCopyEveryField() { + ImmutableMetadata metadata = + ImmutableMetadata.builder().addString("key", "value").build(); + ProviderEventDetails providerEventDetails = ProviderEventDetails.builder() + .flagsChanged(Arrays.asList("flag1", "flag2")) + .message("message") + .eventMetadata(metadata) + .errorCode(ErrorCode.GENERAL) + .build(); + + EventDetails details = EventDetails.fromProviderEventDetails(providerEventDetails, "provider", "domain"); + + assertThat(details.getFlagsChanged()).isEqualTo(providerEventDetails.getFlagsChanged()); + assertThat(details.getMessage()).isEqualTo(providerEventDetails.getMessage()); + assertThat(details.getEventMetadata()).isEqualTo(providerEventDetails.getEventMetadata()); + assertThat(details.getErrorCode()).isEqualTo(providerEventDetails.getErrorCode()); + assertThat(details.getProviderName()).isEqualTo("provider"); + assertThat(details.getDomain()).isEqualTo("domain"); + } +} diff --git a/src/test/java/dev/openfeature/sdk/EventsTest.java b/src/test/java/dev/openfeature/sdk/EventsTest.java index b3cd2a05d..e010007f2 100644 --- a/src/test/java/dev/openfeature/sdk/EventsTest.java +++ b/src/test/java/dev/openfeature/sdk/EventsTest.java @@ -538,6 +538,22 @@ void handlersRunIfOneThrows() { verify(lastHandler, timeout(TIMEOUT)).accept(any()); } + @Test + @DisplayName("errors the SDK raises itself must carry the error code") + @Specification( + number = "5.1.5", + text = "`PROVIDER_ERROR` events SHOULD populate the `provider event details`'s `error code` field.") + void errorsRaisedBySdkMustCarryErrorCode() { + final Consumer handler = mockHandler(); + api.onProviderError(handler); + + api.setProvider( + "errorsRaisedBySdkMustCarryErrorCode", TestProvider.builder().initsToFatal()); + + verify(handler, timeout(TIMEOUT)) + .accept(argThat((EventDetails details) -> ErrorCode.PROVIDER_FATAL.equals(details.getErrorCode()))); + } + @Test @DisplayName("should have all properties") @Specification(number = "5.2.4", text = "The handler function MUST accept a event details parameter.") @@ -561,10 +577,12 @@ void shouldHaveAllProperties() { ImmutableMetadata metadata = ImmutableMetadata.builder().addInteger("int", 1).build(); String message = "a message"; + ErrorCode errorCode = ErrorCode.GENERAL; ProviderEventDetails details = ProviderEventDetails.builder() .eventMetadata(metadata) .flagsChanged(flagsChanged) .message(message) + .errorCode(errorCode) .build(); provider.emit(ProviderEvent.PROVIDER_CONFIGURATION_CHANGED, details); @@ -574,12 +592,14 @@ void shouldHaveAllProperties() { return metadata.equals(eventDetails.getEventMetadata()) // TODO: issue for client name in events && flagsChanged.equals(eventDetails.getFlagsChanged()) - && message.equals(eventDetails.getMessage()); + && message.equals(eventDetails.getMessage()) + && errorCode.equals(eventDetails.getErrorCode()); })); verify(handler2, timeout(TIMEOUT)).accept(argThat((EventDetails eventDetails) -> { return metadata.equals(eventDetails.getEventMetadata()) && flagsChanged.equals(eventDetails.getFlagsChanged()) && message.equals(eventDetails.getMessage()) + && errorCode.equals(eventDetails.getErrorCode()) && name.equals(eventDetails.getDomain()); })); }