From 9b8eea622afee9c330f70855c5353c493b9b7f87 Mon Sep 17 00:00:00 2001 From: Oleksii Shmalko Date: Mon, 16 Feb 2026 18:20:54 +0200 Subject: [PATCH 1/4] fix: make SetProviderAndWait behavior consistent with SetProvider MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit OpenFeature specification defines SetProviderAndWait as a waiting version of SetProvider, or as a shortcut for waiting on provider ready event. However, currently SetProvider and SetProviderAndWait exhibit non-trivial behavior differences besides waiting. SetProvider runs initialization asynchronously and potentially concurrently with shutdown of the old provider. The API is not blocked and the application author may initialize other providers concurrently, run evaluations, etc. 🐛: in this mode, the error from initializer is ignored when updating the provider state, so fatal/error states may be not set properly. SetProviderAndWait runs initialization synchronously while holding exclusive api.mu lock. This almost completely locks OpenFeature SDK: the application author cannot initialize other providers (for different domains), configure context or hooks, evaluate feature flags, or shutdown SDK. If a provider initialization blocks forever'ish, the SDK remains unusable and is unrecoverable. Another difference is that old provider is shutdown only after new provider has successfully initialized. 🐛: if the new provider fails to initialize, the old provider is already unset in API but will never be shutdown, and the new provider is not registered with api.eventExecutor (so if it comes back online after some time, nobody listens to its events, and the state will go out of sync if old provider continues emitting events). 🐛: in both modes, given that shutdown is run concurrently with updating subscriptions in eventExecutor, it is possible for the old provider to override the state of the new provider: 1. init finishes, emits provider ready event (directly from goroutine), updates state 2. old provider emits some event during shutdown (e.g., PROVIDER_ERROR or PROVIDER_STALE), eventExecutor receives the event and updates the state to error/stale 3. new provider is registered with eventExecutor but the state is already wrong. This PR introduces a couple of changes: Make initialization flow consistent across both modes: always initialize async but make "AndWait" methods wait for initialization outside of critical section. Make init respect returned error. Always call shutdown on old provider (if it is no longer used). Always register new provider with event executor. Do this before we start init/shutdown, so the old provider cannot influence state of the new provider. Make event executor registration non-erroring by making shutdown channel buffered (there were no good way to recover from registration error). Signed-off-by: Oleksii Shmalko --- openfeature/event_executor.go | 36 +++-- openfeature/event_executor_test.go | 76 +++-------- openfeature/openfeature_api.go | 202 +++++++++++------------------ openfeature/reference.go | 2 +- 4 files changed, 120 insertions(+), 196 deletions(-) diff --git a/openfeature/event_executor.go b/openfeature/event_executor.go index 01fa1718..9673fc1a 100644 --- a/openfeature/event_executor.go +++ b/openfeature/event_executor.go @@ -1,12 +1,10 @@ package openfeature import ( - "fmt" "log/slog" "maps" "slices" "sync" - "time" ) const defaultDomain = "" @@ -192,7 +190,7 @@ func (e *eventExecutor) State(domain string) State { } // registerDefaultProvider registers the default FeatureProvider and remove the old default provider if available -func (e *eventExecutor) registerDefaultProvider(provider FeatureProvider) error { +func (e *eventExecutor) registerDefaultProvider(provider FeatureProvider) { e.mu.Lock() defer e.mu.Unlock() @@ -200,11 +198,11 @@ func (e *eventExecutor) registerDefaultProvider(provider FeatureProvider) error oldProvider := e.defaultProviderReference e.defaultProviderReference = newProvider - return e.startListeningAndShutdownOld(newProvider, oldProvider) + e.startListeningAndShutdownOld(newProvider, oldProvider) } // registerNamedEventingProvider registers a named FeatureProvider and remove event listener for old named provider -func (e *eventExecutor) registerNamedEventingProvider(associatedClient string, provider FeatureProvider) error { +func (e *eventExecutor) registerNamedEventingProvider(associatedClient string, provider FeatureProvider) { e.mu.Lock() defer e.mu.Unlock() newProvider := newProviderRef(provider) @@ -212,12 +210,12 @@ func (e *eventExecutor) registerNamedEventingProvider(associatedClient string, p oldProvider := e.namedProviderReference[associatedClient] e.namedProviderReference[associatedClient] = newProvider - return e.startListeningAndShutdownOld(newProvider, oldProvider) + e.startListeningAndShutdownOld(newProvider, oldProvider) } // startListeningAndShutdownOld is a helper to start concurrent listening to new provider events and invoke shutdown // hook of the old provider if it's not bound by another subscription -func (e *eventExecutor) startListeningAndShutdownOld(newProvider providerReference, oldReference providerReference) error { +func (e *eventExecutor) startListeningAndShutdownOld(newProvider providerReference, oldReference providerReference) { // check if this provider already actively handled - 1:N binding capability if !isRunning(newProvider, e.activeSubscriptions) { e.activeSubscriptions = append(e.activeSubscriptions, newProvider) @@ -247,7 +245,7 @@ func (e *eventExecutor) startListeningAndShutdownOld(newProvider providerReferen // check if this provider is still bound - 1:N binding capability if isBound(oldReference, e.defaultProviderReference, slices.Collect(maps.Values(e.namedProviderReference))) { - return nil + return } // drop from active references @@ -258,16 +256,28 @@ func (e *eventExecutor) startListeningAndShutdownOld(newProvider providerReferen _, ok := oldReference.featureProvider.(EventHandler) if !ok { // no shutdown for non event handling provider - return nil + return } // avoid shutdown lockouts select { case oldReference.shutdownSemaphore <- "": - return nil - case <-time.After(200 * time.Millisecond): - return fmt.Errorf("old event handler %s timeout waiting for handler shutdown", - oldReference.featureProvider.Metadata().Name) + default: + // This should never happen: + // + // providerReference.shutdownSemaphore is created with + // a buffer size of 1, so it should allow sending at + // least one shutdown signal without blocking. Locking + // should prevent us from sending more than one + // signal. + // + // In the unlikely case that it does not, this + // shouldn't be a big deal: we have already swapped + // references in eventExecutor and openfeatureAPI, and + // the handler should be able to receive at least one + // shutdown signal later. + slog.Info("OF BUG: failed to send shutdown to old event handler", + "provider", oldReference.featureProvider.Metadata().Name) } } diff --git a/openfeature/event_executor_test.go b/openfeature/event_executor_test.go index 3e2a2030..fe6d99af 100644 --- a/openfeature/event_executor_test.go +++ b/openfeature/event_executor_test.go @@ -30,19 +30,13 @@ func TestEventHandler_RegisterUnregisterEventProvider(t *testing.T) { } executor := newEventExecutor() - err := executor.registerDefaultProvider(eventingProvider) - if err != nil { - t.Fatal(err) - } + executor.registerDefaultProvider(eventingProvider) if executor.defaultProviderReference.featureProvider != eventingProvider { t.Error("implementation should register default eventing provider") } - err = executor.registerNamedEventingProvider("domain", eventingProvider) - if err != nil { - t.Fatal(err) - } + executor.registerNamedEventingProvider("domain", eventingProvider) if _, ok := executor.namedProviderReference["domain"]; !ok { t.Errorf("implementation should register named eventing provider") @@ -966,7 +960,6 @@ func TestEventHandler_HandlersRunImmediately(t *testing.T) { eventingImpl := &ProviderEventing{ c: make(chan Event, 1), } - eventingImpl.Invoke(Event{EventType: ProviderError}) provider := struct { FeatureProvider @@ -990,6 +983,9 @@ func TestEventHandler_HandlersRunImmediately(t *testing.T) { AddHandler(ProviderStale, &callback) AddHandler(ProviderConfigChange, &callback) + // Emit error event from the provider + eventingImpl.Invoke(Event{EventType: ProviderError}) + // assert client transitioned to ERROR eventually(t, func() bool { return NewDefaultClient().State() == ErrorState @@ -1009,7 +1005,6 @@ func TestEventHandler_HandlersRunImmediately(t *testing.T) { eventingImpl := &ProviderEventing{ c: make(chan Event, 1), } - eventingImpl.Invoke(Event{EventType: ProviderStale}) provider := struct { FeatureProvider @@ -1033,6 +1028,9 @@ func TestEventHandler_HandlersRunImmediately(t *testing.T) { AddHandler(ProviderError, &callback) AddHandler(ProviderConfigChange, &callback) + // Emit stale event from the provider. + eventingImpl.Invoke(Event{EventType: ProviderStale}) + // assert client transitioned to STALE eventually(t, func() bool { return NewDefaultClient().State() == StaleState @@ -1198,25 +1196,16 @@ func TestEventHandler_1ToNMapping(t *testing.T) { executor := newEventExecutor() - err := executor.registerDefaultProvider(eventingProvider) - if err != nil { - t.Fatal(err) - } + executor.registerDefaultProvider(eventingProvider) if len(executor.activeSubscriptions) != 1 && executor.activeSubscriptions[0].featureProvider != eventingProvider.FeatureProvider { t.Fatal("provider not added to active provider subscriptions") } - err = executor.registerNamedEventingProvider("clientA", eventingProvider) - if err != nil { - t.Fatal(err) - } + executor.registerNamedEventingProvider("clientA", eventingProvider) - err = executor.registerNamedEventingProvider("clientB", eventingProvider) - if err != nil { - t.Fatal(err) - } + executor.registerNamedEventingProvider("clientB", eventingProvider) if len(executor.activeSubscriptions) != 1 { t.Fatal("multiple provided in active subscriptions") @@ -1236,15 +1225,9 @@ func TestEventHandler_1ToNMapping(t *testing.T) { executor := newEventExecutor() - err := executor.registerDefaultProvider(eventingProvider) - if err != nil { - t.Fatal(err) - } + executor.registerDefaultProvider(eventingProvider) - err = executor.registerNamedEventingProvider("clientA", eventingProvider) - if err != nil { - t.Fatal(err) - } + executor.registerNamedEventingProvider("clientA", eventingProvider) overridingProvider := struct { FeatureProvider @@ -1256,10 +1239,7 @@ func TestEventHandler_1ToNMapping(t *testing.T) { }, } - err = executor.registerNamedEventingProvider("clientA", overridingProvider) - if err != nil { - t.Fatal(err) - } + executor.registerNamedEventingProvider("clientA", overridingProvider) if len(executor.activeSubscriptions) != 2 { t.Fatal("error with active provider subscriptions") @@ -1279,15 +1259,9 @@ func TestEventHandler_1ToNMapping(t *testing.T) { executor := newEventExecutor() - err := executor.registerNamedEventingProvider("clientA", eventingProvider) - if err != nil { - t.Fatal(err) - } + executor.registerNamedEventingProvider("clientA", eventingProvider) - err = executor.registerNamedEventingProvider("clientB", eventingProvider) - if err != nil { - t.Fatal(err) - } + executor.registerNamedEventingProvider("clientB", eventingProvider) overridingProvider := struct { FeatureProvider @@ -1299,10 +1273,7 @@ func TestEventHandler_1ToNMapping(t *testing.T) { }, } - err = executor.registerNamedEventingProvider("clientA", overridingProvider) - if err != nil { - t.Fatal(err) - } + executor.registerNamedEventingProvider("clientA", overridingProvider) if len(executor.activeSubscriptions) != 2 { t.Fatal("error with active provider subscriptions") @@ -1322,10 +1293,7 @@ func TestEventHandler_1ToNMapping(t *testing.T) { executor := newEventExecutor() - err := executor.registerNamedEventingProvider("clientA", eventingProvider) - if err != nil { - t.Fatal(err) - } + executor.registerNamedEventingProvider("clientA", eventingProvider) overridingProvider := struct { FeatureProvider @@ -1337,10 +1305,7 @@ func TestEventHandler_1ToNMapping(t *testing.T) { }, } - err = executor.registerNamedEventingProvider("clientA", overridingProvider) - if err != nil { - t.Fatal(err) - } + executor.registerNamedEventingProvider("clientA", overridingProvider) if len(executor.activeSubscriptions) != 1 && executor.activeSubscriptions[0].featureProvider != overridingProvider.FeatureProvider { @@ -1560,8 +1525,7 @@ func TestEventHandler_ChannelClosure(t *testing.T) { executor := newEventExecutor() - err := executor.registerDefaultProvider(eventingProvider) - require.NoError(t, err) + executor.registerDefaultProvider(eventingProvider) require.Len(t, executor.activeSubscriptions, 1) var eventCount atomic.Int64 diff --git a/openfeature/openfeature_api.go b/openfeature/openfeature_api.go index 715a4ca0..13118700 100644 --- a/openfeature/openfeature_api.go +++ b/openfeature/openfeature_api.go @@ -34,163 +34,120 @@ func newEvaluationAPI(eventExecutor *eventExecutor) *evaluationAPI { } func (api *evaluationAPI) SetProvider(provider FeatureProvider) error { - return api.setProvider(provider, true) + return api.SetProviderWithContext(context.Background(), provider) } func (api *evaluationAPI) SetProviderAndWait(provider FeatureProvider) error { - return api.setProvider(provider, false) + return api.SetProviderWithContextAndWait(context.Background(), provider) } -// GetProviderMetadata returns the default FeatureProvider's metadata -func (api *evaluationAPI) GetProviderMetadata() Metadata { - api.mu.RLock() - defer api.mu.RUnlock() +// SetProviderWithContext sets the default FeatureProvider with context-aware initialization. +func (api *evaluationAPI) SetProviderWithContext(ctx context.Context, provider FeatureProvider) error { + if provider == nil { + return errors.New("default provider cannot be set to nil") + } + api.setProviderWithContext(ctx, provider) + return nil +} - return api.defaultProvider.Metadata() +// SetProviderWithContextAndWait sets the default FeatureProvider with context-aware initialization and waits for completion. +func (api *evaluationAPI) SetProviderWithContextAndWait(ctx context.Context, provider FeatureProvider) error { + if provider == nil { + return errors.New("default provider cannot be set to nil") + } + initCh := api.setProviderWithContext(ctx, provider) + return <-initCh } // SetNamedProvider sets a provider with client name. Returns an error if FeatureProvider is nil func (api *evaluationAPI) SetNamedProvider(clientName string, provider FeatureProvider, async bool) error { - api.mu.Lock() - defer api.mu.Unlock() + return api.SetNamedProviderWithContext(context.Background(), clientName, provider, async) +} +// SetNamedProviderWithContext sets a provider with client name using context-aware initialization. +func (api *evaluationAPI) SetNamedProviderWithContext(ctx context.Context, clientName string, provider FeatureProvider, async bool) error { if provider == nil { return errors.New("provider cannot be set to nil") } - // Initialize new named provider and Shutdown the old one - // Provider update must be non-blocking, hence initialization & Shutdown happens concurrently - oldProvider := api.namedProviders[clientName] - api.namedProviders[clientName] = provider - - err := api.initNewAndShutdownOld(context.Background(), clientName, provider, oldProvider, async) - if err != nil { - return err - } - - err = api.eventExecutor.registerNamedEventingProvider(clientName, provider) - if err != nil { - return err - } - - return nil -} - -// GetNamedProviderMetadata returns the default FeatureProvider's metadata -func (api *evaluationAPI) GetNamedProviderMetadata(name string) Metadata { - api.mu.RLock() - defer api.mu.RUnlock() + initCh := api.setNamedProviderWithContext(ctx, clientName, provider) - provider, ok := api.namedProviders[name] - if !ok { - return ProviderMetadata() + if async { + return nil } - return provider.Metadata() -} - -// Context-aware provider setup methods - -// SetProviderWithContext sets the default FeatureProvider with context-aware initialization. -func (api *evaluationAPI) SetProviderWithContext(ctx context.Context, provider FeatureProvider) error { - return api.setProviderWithContext(ctx, provider, true) + return <-initCh } -// SetProviderWithContextAndWait sets the default FeatureProvider with context-aware initialization and waits for completion. -func (api *evaluationAPI) SetProviderWithContextAndWait(ctx context.Context, provider FeatureProvider) error { - return api.setProviderWithContext(ctx, provider, false) +// SetNamedProviderWithContextAndWait sets a provider with client name using context-aware initialization and waits for completion. +func (api *evaluationAPI) SetNamedProviderWithContextAndWait(ctx context.Context, clientName string, provider FeatureProvider) error { + return api.SetNamedProviderWithContext(ctx, clientName, provider, false) } // setProviderWithContext sets the default FeatureProvider of the evaluationAPI with context-aware initialization. -func (api *evaluationAPI) setProviderWithContext(ctx context.Context, provider FeatureProvider, async bool) error { +func (api *evaluationAPI) setProviderWithContext(ctx context.Context, provider FeatureProvider) <-chan error { api.mu.Lock() defer api.mu.Unlock() - if provider == nil { - return errors.New("default provider cannot be set to nil") - } - oldProvider := api.defaultProvider api.defaultProvider = provider - err := api.initNewAndShutdownOld(ctx, "", provider, oldProvider, async) - if err != nil { - return fmt.Errorf("failed to initialize default provider %q: %w", provider.Metadata().Name, err) - } + api.eventExecutor.registerDefaultProvider(provider) - err = api.eventExecutor.registerDefaultProvider(provider) - if err != nil { - return fmt.Errorf("failed to register default provider %q: %w", provider.Metadata().Name, err) - } + api.shutdownOld(ctx, oldProvider) - return nil + return api.initNew(ctx, "", provider) } -// SetNamedProviderWithContext sets a provider with client name using context-aware initialization. -func (api *evaluationAPI) SetNamedProviderWithContext(ctx context.Context, clientName string, provider FeatureProvider, async bool) error { +func (api *evaluationAPI) setNamedProviderWithContext(ctx context.Context, clientName string, provider FeatureProvider) <-chan error { api.mu.Lock() defer api.mu.Unlock() - if provider == nil { - return errors.New("provider cannot be set to nil") - } - // Initialize new named provider and Shutdown the old one oldProvider := api.namedProviders[clientName] api.namedProviders[clientName] = provider - err := api.initNewAndShutdownOld(ctx, clientName, provider, oldProvider, async) - if err != nil { - return fmt.Errorf("failed to initialize named provider %q for domain %q: %w", provider.Metadata().Name, clientName, err) - } + api.eventExecutor.registerNamedEventingProvider(clientName, provider) - err = api.eventExecutor.registerNamedEventingProvider(clientName, provider) - if err != nil { - return fmt.Errorf("failed to register named provider %q for domain %q: %w", provider.Metadata().Name, clientName, err) - } + api.shutdownOld(ctx, oldProvider) - return nil + return api.initNew(ctx, clientName, provider) } -// SetNamedProviderWithContextAndWait sets a provider with client name using context-aware initialization and waits for completion. -func (api *evaluationAPI) SetNamedProviderWithContextAndWait(ctx context.Context, clientName string, provider FeatureProvider) error { - return api.SetNamedProviderWithContext(ctx, clientName, provider, false) -} +func (api *evaluationAPI) initNew(ctx context.Context, clientName string, newProvider FeatureProvider) <-chan error { + errCh := make(chan error, 1) + + // Initialize new provider async. The caller may wait on the channel. + go func(executor *eventExecutor, evalCtx EvaluationContext, ctx context.Context, provider FeatureProvider, clientName string) { + event, err := initializerWithContext(ctx, provider, evalCtx) + executor.triggerEvent(event, provider) -// initNewAndShutdownOld is the main helper to initialise new FeatureProvider and Shutdown the old FeatureProvider. -// Always uses the context-aware initializer with the provided context. -// -// When shutting down old providers that implement ContextAwareStateHandler, a 10-second timeout -// is applied to prevent hanging if the provider becomes unresponsive during shutdown. -func (api *evaluationAPI) initNewAndShutdownOld(ctx context.Context, clientName string, newProvider FeatureProvider, oldProvider FeatureProvider, async bool) error { - if async { - go func(executor *eventExecutor, evalCtx EvaluationContext, ctx context.Context, provider FeatureProvider, clientName string) { - // for async initialization, error is conveyed as an event - event, _ := initializerWithContext(ctx, provider, evalCtx) - executor.states.Store(clientName, stateFromEventOrError(event, nil)) - executor.triggerEvent(event, provider) - }(api.eventExecutor, api.evalCtx, ctx, newProvider, clientName) - } else { - event, err := initializerWithContext(ctx, newProvider, api.evalCtx) - api.eventExecutor.states.Store(clientName, stateFromEventOrError(event, err)) - api.eventExecutor.triggerEvent(event, newProvider) if err != nil { - return err + if clientName == "" { + err = fmt.Errorf("failed to initialize default provider %q: %w", provider.Metadata().Name, err) + } else { + err = fmt.Errorf("failed to initialize named provider %q for domain %q: %w", provider.Metadata().Name, clientName, err) + } } - } + errCh <- err + }(api.eventExecutor, api.evalCtx, ctx, newProvider, clientName) + return errCh +} + +func (api *evaluationAPI) shutdownOld(ctx context.Context, oldProvider FeatureProvider) { v, ok := oldProvider.(StateHandler) // oldProvider can be nil or without state handling capability if oldProvider == nil || !ok { - return nil + return } namedProviders := slices.Collect(maps.Values(api.namedProviders)) // check for multiple bindings if oldProvider == api.defaultProvider || slices.Contains(namedProviders, oldProvider) { - return nil + return } go func(forShutdown StateHandler, parentCtx context.Context) { @@ -203,8 +160,27 @@ func (api *evaluationAPI) initNewAndShutdownOld(ctx context.Context, clientName forShutdown.Shutdown() } }(v, ctx) +} - return nil +// GetProviderMetadata returns the default FeatureProvider's metadata +func (api *evaluationAPI) GetProviderMetadata() Metadata { + api.mu.RLock() + defer api.mu.RUnlock() + + return api.defaultProvider.Metadata() +} + +// GetNamedProviderMetadata returns the default FeatureProvider's metadata +func (api *evaluationAPI) GetNamedProviderMetadata(name string) Metadata { + api.mu.RLock() + defer api.mu.RUnlock() + + provider, ok := api.namedProviders[name] + if !ok { + return ProviderMetadata() + } + + return provider.Metadata() } // GetNamedProviders returns named providers map. @@ -324,32 +300,6 @@ func (api *evaluationAPI) GetProvider() FeatureProvider { return api.defaultProvider } -// SetProvider sets the default FeatureProvider of the evaluationAPI. -// Returns an error if provider registration cause an error -func (api *evaluationAPI) setProvider(provider FeatureProvider, async bool) error { - api.mu.Lock() - defer api.mu.Unlock() - - if provider == nil { - return errors.New("default provider cannot be set to nil") - } - - oldProvider := api.defaultProvider - api.defaultProvider = provider - - err := api.initNewAndShutdownOld(context.Background(), "", provider, oldProvider, async) - if err != nil { - return err - } - - err = api.eventExecutor.registerDefaultProvider(provider) - if err != nil { - return err - } - - return nil -} - // initializerWithContext is a context-aware helper to execute provider initialization and generate appropriate event for the initialization // If the provider implements ContextAwareStateHandler, InitWithContext is called; otherwise, Init is called for backward compatibility. // It also returns an error if the initialization resulted in an error or if the context is cancelled. diff --git a/openfeature/reference.go b/openfeature/reference.go index ab04ca35..18c7da6b 100644 --- a/openfeature/reference.go +++ b/openfeature/reference.go @@ -9,7 +9,7 @@ func newProviderRef(provider FeatureProvider) providerReference { return providerReference{ featureProvider: provider, kind: reflect.TypeOf(provider).Kind(), - shutdownSemaphore: make(chan any), + shutdownSemaphore: make(chan any, 1), } } From e8efbb52ad4747860312da38e64d88605ee6880b Mon Sep 17 00:00:00 2001 From: Oleksii Shmalko Date: Wed, 18 Feb 2026 23:09:24 +0200 Subject: [PATCH 2/4] test: fix tests relying on incorrect event ordering Signed-off-by: Oleksii Shmalko --- openfeature/event_executor_test.go | 80 +++++++++++++++--------------- 1 file changed, 39 insertions(+), 41 deletions(-) diff --git a/openfeature/event_executor_test.go b/openfeature/event_executor_test.go index fe6d99af..41ba4286 100644 --- a/openfeature/event_executor_test.go +++ b/openfeature/event_executor_test.go @@ -438,7 +438,6 @@ func TestEventHandler_InitOfProvider(t *testing.T) { eventingImpl := &ProviderEventing{ c: make(chan Event, 1), } - eventingImpl.Invoke(Event{EventType: ProviderConfigChange}) provider := struct { FeatureProvider @@ -457,11 +456,13 @@ func TestEventHandler_InitOfProvider(t *testing.T) { client := NewClient("someClient") client.AddHandler(ProviderReady, &callback) - err := SetNamedProvider("providerWithoutClient", provider) + err := SetNamedProviderAndWait("providerWithoutClient", provider) if err != nil { t.Fatal(err) } + eventingImpl.Invoke(Event{EventType: ProviderConfigChange}) + select { case <-rsp: t.Errorf("event must have not emitted") @@ -476,17 +477,18 @@ func TestEventHandler_InitOfProviderError(t *testing.T) { t.Run("for default provider in global scope", func(t *testing.T) { t.Cleanup(initSingleton) - eventingImpl := &ProviderEventing{ - c: make(chan Event, 1), - } - eventingImpl.Invoke(Event{EventType: ProviderError}) - - provider := struct { + initFailingProvider := struct { FeatureProvider + StateHandler EventHandler }{ NoopProvider{}, - eventingImpl, + &stateHandlerForTests{ + initF: func(e EvaluationContext) error { + return errors.New("init failed") + }, + }, + &ProviderEventing{c: make(chan Event, 1)}, } // callback @@ -496,10 +498,7 @@ func TestEventHandler_InitOfProviderError(t *testing.T) { } AddHandler(ProviderError, &callback) - err := SetProvider(provider) - if err != nil { - t.Fatal(err) - } + _ = SetProviderAndWait(initFailingProvider) // init fails; we only care that ProviderError handler runs select { case <-rsp: @@ -512,17 +511,18 @@ func TestEventHandler_InitOfProviderError(t *testing.T) { t.Run("for default provider with unassociated client handler", func(t *testing.T) { t.Cleanup(initSingleton) - eventingImpl := &ProviderEventing{ - c: make(chan Event, 1), - } - eventingImpl.Invoke(Event{EventType: ProviderError}) - - provider := struct { + initFailingProvider := struct { FeatureProvider + StateHandler EventHandler }{ NoopProvider{}, - eventingImpl, + &stateHandlerForTests{ + initF: func(e EvaluationContext) error { + return errors.New("init failed") + }, + }, + &ProviderEventing{c: make(chan Event, 1)}, } // callback @@ -534,10 +534,7 @@ func TestEventHandler_InitOfProviderError(t *testing.T) { client := NewClient("clientWithNoProviderAssociation") client.AddHandler(ProviderError, &callback) - err := SetProvider(provider) - if err != nil { - t.Fatal(err) - } + _ = SetProviderAndWait(initFailingProvider) // init fails; we only care that ProviderError handler runs select { case <-rsp: @@ -550,17 +547,18 @@ func TestEventHandler_InitOfProviderError(t *testing.T) { t.Run("for named provider in client scope", func(t *testing.T) { t.Cleanup(initSingleton) - eventingImpl := &ProviderEventing{ - c: make(chan Event, 1), - } - eventingImpl.Invoke(Event{EventType: ProviderError}) - - provider := struct { + initFailingProvider := struct { FeatureProvider + StateHandler EventHandler }{ NoopProvider{}, - eventingImpl, + &stateHandlerForTests{ + initF: func(e EvaluationContext) error { + return errors.New("init failed") + }, + }, + &ProviderEventing{c: make(chan Event, 1)}, } // callback @@ -572,10 +570,7 @@ func TestEventHandler_InitOfProviderError(t *testing.T) { client := NewClient("someClient") client.AddHandler(ProviderError, &callback) - err := SetNamedProvider("someClient", provider) - if err != nil { - t.Fatal(err) - } + _ = SetNamedProviderAndWait("someClient", initFailingProvider) // init fails; we only care that ProviderError handler runs select { case <-rsp: @@ -591,7 +586,6 @@ func TestEventHandler_InitOfProviderError(t *testing.T) { eventingImpl := &ProviderEventing{ c: make(chan Event, 1), } - eventingImpl.Invoke(Event{EventType: ProviderError}) provider := struct { FeatureProvider @@ -610,11 +604,13 @@ func TestEventHandler_InitOfProviderError(t *testing.T) { client := NewClient("provider") client.AddHandler(ProviderError, &callback) - err := SetNamedProvider("someClient", provider) + err := SetNamedProviderAndWait("someClient", provider) if err != nil { t.Fatal(err) } + eventingImpl.Invoke(Event{EventType: ProviderError}) + select { case <-rsp: t.Errorf("event must have not emitted") @@ -854,7 +850,6 @@ func TestEventHandler_HandlersRunImmediately(t *testing.T) { eventingImpl := &ProviderEventing{ c: make(chan Event, 1), } - eventingImpl.Invoke(Event{EventType: ProviderError}) provider := struct { FeatureProvider @@ -864,7 +859,7 @@ func TestEventHandler_HandlersRunImmediately(t *testing.T) { eventingImpl, } - if err := SetProvider(provider); err != nil { + if err := SetProviderAndWait(provider); err != nil { t.Fatal(err) } @@ -875,6 +870,8 @@ func TestEventHandler_HandlersRunImmediately(t *testing.T) { AddHandler(ProviderError, &callback) + eventingImpl.Invoke(Event{EventType: ProviderError}) + select { case <-rsp: break @@ -889,7 +886,6 @@ func TestEventHandler_HandlersRunImmediately(t *testing.T) { eventingImpl := &ProviderEventing{ c: make(chan Event, 1), } - eventingImpl.Invoke(Event{EventType: ProviderStale}) provider := struct { FeatureProvider @@ -899,10 +895,12 @@ func TestEventHandler_HandlersRunImmediately(t *testing.T) { eventingImpl, } - if err := SetProvider(provider); err != nil { + if err := SetProviderAndWait(provider); err != nil { t.Fatal(err) } + eventingImpl.Invoke(Event{EventType: ProviderStale}) + rsp := make(chan EventDetails, 1) callback := func(e EventDetails) { rsp <- e From b6e4bfa478b3138c22627dd25ab2d35d955efe3f Mon Sep 17 00:00:00 2001 From: Todd Baert Date: Sat, 28 Feb 2026 15:12:40 -0500 Subject: [PATCH 3/4] fixup: remove dead code Signed-off-by: Todd Baert --- openfeature/openfeature_api.go | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/openfeature/openfeature_api.go b/openfeature/openfeature_api.go index 13118700..39a12ff8 100644 --- a/openfeature/openfeature_api.go +++ b/openfeature/openfeature_api.go @@ -368,27 +368,9 @@ var statesMap = map[EventType]func(ProviderEventDetails) State{ }, } -func stateFromEventOrError(event Event, err error) State { - if err != nil { - return stateFromError(err) - } - return stateFromEvent(event) -} - func stateFromEvent(event Event) State { if stateFn, ok := statesMap[event.EventType]; ok { return stateFn(event.ProviderEventDetails) } return NotReadyState // default } - -func stateFromError(err error) State { - var e *ProviderInitError - switch { - case errors.As(err, &e): - if e.ErrorCode == ProviderFatalCode { - return FatalState - } - } - return ErrorState // default -} From 929042218c198abc722012da26475214cb362063 Mon Sep 17 00:00:00 2001 From: Todd Baert Date: Mon, 2 Mar 2026 06:46:35 -0500 Subject: [PATCH 4/4] fixup: test 533 Signed-off-by: Todd Baert --- openfeature/event_executor_test.go | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/openfeature/event_executor_test.go b/openfeature/event_executor_test.go index 41ba4286..a86e8386 100644 --- a/openfeature/event_executor_test.go +++ b/openfeature/event_executor_test.go @@ -863,6 +863,13 @@ func TestEventHandler_HandlersRunImmediately(t *testing.T) { t.Fatal(err) } + // Emit error event and wait for state transition before registering handler, + // so that AddHandler fires via emitOnRegistration (spec 5.3.3). + eventingImpl.Invoke(Event{EventType: ProviderError}) + eventually(t, func() bool { + return NewDefaultClient().State() == ErrorState + }, time.Second, time.Millisecond*100, "provider did not transition to ERROR state") + rsp := make(chan EventDetails, 1) callback := func(e EventDetails) { rsp <- e @@ -870,8 +877,6 @@ func TestEventHandler_HandlersRunImmediately(t *testing.T) { AddHandler(ProviderError, &callback) - eventingImpl.Invoke(Event{EventType: ProviderError}) - select { case <-rsp: break @@ -899,7 +904,12 @@ func TestEventHandler_HandlersRunImmediately(t *testing.T) { t.Fatal(err) } + // Emit stale event and wait for state transition before registering handler, + // so that AddHandler fires via emitOnRegistration (spec 5.3.3). eventingImpl.Invoke(Event{EventType: ProviderStale}) + eventually(t, func() bool { + return NewDefaultClient().State() == StaleState + }, time.Second, time.Millisecond*100, "provider did not transition to STALE state") rsp := make(chan EventDetails, 1) callback := func(e EventDetails) {