feat!: internally maintain provider status (open-feature#276)

This PR implements a few things from spec 0.8.0: - implements internal provider status (already implemented in JS) - the provider no longer updates its status to READY/ERROR, etc after init (the SDK does this automatically) - the provider's state is updated according to the last event it fired - adds `PROVIDER_FATAL` error and code - adds "short circuit" feature when evaluations are skipped if provider is `NOT_READY` or `FATAL` - removes some deprecations that were making the work harder since we already have pending breaking changes. Fixes: open-feature#250 --------- Signed-off-by: Todd Baert <[email protected]> Co-authored-by: André Silva <[email protected]> Co-authored-by: Ryan Lamb <[email protected]> Signed-off-by: Artyom Tonoyan <[email protected]>
arttonoyan · Nov 17, 2024 · 8020219 · 8020219
1 parent a123541
commit 8020219
Show file tree

Hide file tree

Showing 16 changed files with 365 additions and 338 deletions.
diff --git a/src/OpenFeature/Api.cs b/src/OpenFeature/Api.cs
@@ -6,6 +6,7 @@
 using System.Threading.Tasks;
 using Microsoft.Extensions.Logging;
 using OpenFeature.Constant;
+using OpenFeature.Error;
 using OpenFeature.Model;
 
 namespace OpenFeature
@@ -37,18 +38,17 @@ static Api() { }
         private Api() { }
 
         /// <summary>
-        /// Sets the feature provider. In order to wait for the provider to be set, and initialization to complete,
+        /// Sets the default feature provider. In order to wait for the provider to be set, and initialization to complete,
         /// await the returned task.
         /// </summary>
         /// <remarks>The provider cannot be set to null. Attempting to set the provider to null has no effect.</remarks>
         /// <param name="featureProvider">Implementation of <see cref="FeatureProvider"/></param>
         public async Task SetProviderAsync(FeatureProvider featureProvider)
         {
             this._eventExecutor.RegisterDefaultFeatureProvider(featureProvider);
-            await this._repository.SetProviderAsync(featureProvider, this.GetContext()).ConfigureAwait(false);
+            await this._repository.SetProviderAsync(featureProvider, this.GetContext(), AfterInitialization, AfterError).ConfigureAwait(false);
         }
 
-
         /// <summary>
         /// Sets the feature provider to given clientName. In order to wait for the provider to be set, and
         /// initialization to complete, await the returned task.
@@ -62,7 +62,7 @@ public async Task SetProviderAsync(string clientName, FeatureProvider featurePro
                 throw new ArgumentNullException(nameof(clientName));
             }
             this._eventExecutor.RegisterClientFeatureProvider(clientName, featureProvider);
-            await this._repository.SetProviderAsync(clientName, featureProvider, this.GetContext()).ConfigureAwait(false);
+            await this._repository.SetProviderAsync(clientName, featureProvider, this.GetContext(), AfterInitialization, AfterError).ConfigureAwait(false);
         }
 
         /// <summary>
@@ -121,7 +121,7 @@ public FeatureProvider GetProvider(string clientName)
         /// <returns><see cref="FeatureClient"/></returns>
         public FeatureClient GetClient(string? name = null, string? version = null, ILogger? logger = null,
             EvaluationContext? context = null) =>
-            new FeatureClient(name, version, logger, context);
+            new FeatureClient(() => _repository.GetProvider(name), name, version, logger, context);
 
         /// <summary>
         /// Appends list of hooks to global hooks list
@@ -258,12 +258,46 @@ public void RemoveHandler(ProviderEventTypes type, EventHandlerDelegate handler)
         public void SetLogger(ILogger logger)
         {
             this._eventExecutor.SetLogger(logger);
+            this._repository.SetLogger(logger);
         }
 
         internal void AddClientHandler(string client, ProviderEventTypes eventType, EventHandlerDelegate handler)
             => this._eventExecutor.AddClientHandler(client, eventType, handler);
 
         internal void RemoveClientHandler(string client, ProviderEventTypes eventType, EventHandlerDelegate handler)
             => this._eventExecutor.RemoveClientHandler(client, eventType, handler);
+
+        /// <summary>
+        /// Update the provider state to READY and emit a READY event after successful init.
+        /// </summary>
+        private async Task AfterInitialization(FeatureProvider provider)
+        {
+            provider.Status = ProviderStatus.Ready;
+            var eventPayload = new ProviderEventPayload
+            {
+                Type = ProviderEventTypes.ProviderReady,
+                Message = "Provider initialization complete",
+                ProviderName = provider.GetMetadata().Name,
+            };
+
+            await this._eventExecutor.EventChannel.Writer.WriteAsync(new Event { Provider = provider, EventPayload = eventPayload }).ConfigureAwait(false);
+        }
+
+        /// <summary>
+        /// Update the provider state to ERROR and emit an ERROR after failed init.
+        /// </summary>
+        private async Task AfterError(FeatureProvider provider, Exception ex)
+
+        {
+            provider.Status = typeof(ProviderFatalException) == ex.GetType() ? ProviderStatus.Fatal : ProviderStatus.Error;
+            var eventPayload = new ProviderEventPayload
+            {
+                Type = ProviderEventTypes.ProviderError,
+                Message = $"Provider initialization error: {ex?.Message}",
+                ProviderName = provider.GetMetadata()?.Name,
+            };
+
+            await this._eventExecutor.EventChannel.Writer.WriteAsync(new Event { Provider = provider, EventPayload = eventPayload }).ConfigureAwait(false);
+        }
     }
 }
diff --git a/src/OpenFeature/Constant/ErrorType.cs b/src/OpenFeature/Constant/ErrorType.cs
@@ -47,5 +47,10 @@ public enum ErrorType
         /// Context does not contain a targeting key and the provider requires one.
         /// </summary>
         [Description("TARGETING_KEY_MISSING")] TargetingKeyMissing,
+
+        /// <summary>
+        /// The provider has entered an irrecoverable error state.
+        /// </summary>
+        [Description("PROVIDER_FATAL")] ProviderFatal,
     }
 }
diff --git a/src/OpenFeature/Constant/ProviderStatus.cs b/src/OpenFeature/Constant/ProviderStatus.cs
@@ -26,6 +26,11 @@ public enum ProviderStatus
         /// <summary>
         /// The provider is in an error state and unable to evaluate flags.
         /// </summary>
-        [Description("ERROR")] Error
+        [Description("ERROR")] Error,
+
+        /// <summary>
+        /// The provider has entered an irrecoverable error state.
+        /// </summary>
+        [Description("FATAL")] Fatal,
     }
 }
diff --git a/src/OpenFeature/Error/ProviderFatalException.cs b/src/OpenFeature/Error/ProviderFatalException.cs
@@ -0,0 +1,23 @@
+using System;
+using System.Diagnostics.CodeAnalysis;
+using OpenFeature.Constant;
+
+namespace OpenFeature.Error
+{
+    /// <summary> the
+    /// An exception that signals the provider has entered an irrecoverable error state.
+    /// </summary>
+    [ExcludeFromCodeCoverage]
+    public class ProviderFatalException : FeatureProviderException
+    {
+        /// <summary>
+        /// Initialize a new instance of the <see cref="ProviderFatalException"/> class
+        /// </summary>
+        /// <param name="message">Exception message</param>
+        /// <param name="innerException">Optional inner exception</param>
+        public ProviderFatalException(string? message = null, Exception? innerException = null)
+            : base(ErrorType.ProviderFatal, message, innerException)
+        {
+        }
+    }
+}
diff --git a/src/OpenFeature/Error/ProviderNotReadyException.cs b/src/OpenFeature/Error/ProviderNotReadyException.cs
@@ -5,7 +5,7 @@
 namespace OpenFeature.Error
 {
     /// <summary>
-    /// Provider has yet been initialized when evaluating a flag.
+    /// Provider has not yet been initialized when evaluating a flag.
     /// </summary>
     [ExcludeFromCodeCoverage]
     public class ProviderNotReadyException : FeatureProviderException

diff --git a/src/OpenFeature/EventExecutor.cs b/src/OpenFeature/EventExecutor.cs
@@ -184,7 +184,7 @@ private void EmitOnRegistration(FeatureProvider? provider, ProviderEventTypes ev
             {
                 return;
             }
-            var status = provider.GetStatus();
+            var status = provider.Status;
 
             var message = "";
             if (status == ProviderStatus.Ready && eventType == ProviderEventTypes.ProviderReady)
@@ -234,6 +234,7 @@ private async void ProcessFeatureProviderEventsAsync(object? providerRef)
                 switch (item)
                 {
                     case ProviderEventPayload eventPayload:
+                        this.UpdateProviderStatus(typedProviderRef, eventPayload);
                         await this.EventChannel.Writer.WriteAsync(new Event { Provider = typedProviderRef, EventPayload = eventPayload }).ConfigureAwait(false);
                         break;
                 }
@@ -307,6 +308,24 @@ private async void ProcessEventAsync()
             }
         }
 
+        // map events to provider status as per spec: https://openfeature.dev/specification/sections/events/#requirement-535
+        private void UpdateProviderStatus(FeatureProvider provider, ProviderEventPayload eventPayload)
+        {
+            switch (eventPayload.Type)
+            {
+                case ProviderEventTypes.ProviderReady:
+                    provider.Status = ProviderStatus.Ready;
+                    break;
+                case ProviderEventTypes.ProviderStale:
+                    provider.Status = ProviderStatus.Stale;
+                    break;
+                case ProviderEventTypes.ProviderError:
+                    provider.Status = eventPayload.ErrorType == ErrorType.ProviderFatal ? ProviderStatus.Fatal : ProviderStatus.Error;
+                    break;
+                default: break;
+            }
+        }
+
         private void InvokeEventHandler(EventHandlerDelegate eventHandler, Event e)
         {
             try

diff --git a/src/OpenFeature/FeatureProvider.cs b/src/OpenFeature/FeatureProvider.cs
@@ -1,10 +1,12 @@
 using System.Collections.Immutable;
+using System.Runtime.CompilerServices;
 using System.Threading;
 using System.Threading.Channels;
 using System.Threading.Tasks;
 using OpenFeature.Constant;
 using OpenFeature.Model;
 
+[assembly: InternalsVisibleTo("DynamicProxyGenAssembly2")] // required to allow NSubstitute mocking of internal methods
 namespace OpenFeature
 {
     /// <summary>
@@ -94,35 +96,25 @@ public abstract Task<ResolutionDetails<Value>> ResolveStructureValueAsync(string
             EvaluationContext? context = null, CancellationToken cancellationToken = default);
 
         /// <summary>
-        /// Get the status of the provider.
+        /// Internally-managed provider status.
+        /// The SDK uses this field to track the status of the provider.
+        /// Not visible outside OpenFeature assembly
         /// </summary>
-        /// <returns>The current <see cref="ProviderStatus"/></returns>
-        /// <remarks>
-        /// If a provider does not override this method, then its status will be assumed to be
-        /// <see cref="ProviderStatus.Ready"/>. If a provider implements this method, and supports initialization,
-        /// then it should start in the <see cref="ProviderStatus.NotReady"/>status . If the status is
-        /// <see cref="ProviderStatus.NotReady"/>, then the Api will call the <see cref="InitializeAsync" /> when the
-        /// provider is set.
-        /// </remarks>
-        public virtual ProviderStatus GetStatus() => ProviderStatus.Ready;
+        internal virtual ProviderStatus Status { get; set; } = ProviderStatus.NotReady;
 
         /// <summary>
         /// <para>
         /// This method is called before a provider is used to evaluate flags. Providers can overwrite this method,
         /// if they have special initialization needed prior being called for flag evaluation.
+        /// When this method completes, the provider will be considered ready for use.
         /// </para>
         /// </summary>
         /// <param name="context"><see cref="EvaluationContext"/></param>
         /// <param name="cancellationToken">The <see cref="CancellationToken"/> to cancel any async side effects.</param>
         /// <returns>A task that completes when the initialization process is complete.</returns>
         /// <remarks>
         /// <para>
-        /// A provider which supports initialization should override this method as well as
-        /// <see cref="GetStatus"/>.
-        /// </para>
-        /// <para>
-        /// The provider should return <see cref="ProviderStatus.Ready"/> or <see cref="ProviderStatus.Error"/> from
-        /// the <see cref="GetStatus"/> method after initialization is complete.
+        /// Providers not implementing this method will be considered ready immediately.
         /// </para>
         /// </remarks>
         public virtual Task InitializeAsync(EvaluationContext context, CancellationToken cancellationToken = default)

diff --git a/src/OpenFeature/IFeatureClient.cs b/src/OpenFeature/IFeatureClient.cs
@@ -1,6 +1,7 @@
 using System.Collections.Generic;
 using System.Threading;
 using System.Threading.Tasks;
+using OpenFeature.Constant;
 using OpenFeature.Model;
 
 namespace OpenFeature
@@ -53,6 +54,12 @@ public interface IFeatureClient : IEventBus
         /// <returns>Client metadata <see cref="ClientMetadata"/></returns>
         ClientMetadata GetMetadata();
 
+        /// <summary>
+        /// Returns the current status of the associated provider.
+        /// </summary>
+        /// <returns><see cref="ProviderStatus"/></returns>
+        ProviderStatus ProviderStatus { get; }
+
         /// <summary>
         /// Resolves a boolean feature flag
         /// </summary>

diff --git a/src/OpenFeature/Model/ProviderEvents.cs b/src/OpenFeature/Model/ProviderEvents.cs
@@ -28,6 +28,11 @@ public class ProviderEventPayload
         /// </summary>
         public string? Message { get; set; }
 
+        /// <summary>
+        /// Optional error associated with the event. 
+        /// </summary>
+        public ErrorType? ErrorType { get; set; }
+
         /// <summary>
         /// A List of flags that have been changed.
         /// </summary>

diff --git a/src/OpenFeature/OpenFeatureClient.cs b/src/OpenFeature/OpenFeatureClient.cs
@@ -21,6 +21,7 @@ public sealed partial class FeatureClient : IFeatureClient
         private readonly ClientMetadata _metadata;
         private readonly ConcurrentStack<Hook> _hooks = new ConcurrentStack<Hook>();
         private readonly ILogger _logger;
+        private readonly Func<FeatureProvider> _providerAccessor;
         private EvaluationContext _evaluationContext;
 
         private readonly object _evaluationContextLock = new object();
@@ -48,6 +49,9 @@ public sealed partial class FeatureClient : IFeatureClient
             return (method(provider), provider);
         }
 
+        /// <inheritdoc />
+        public ProviderStatus ProviderStatus => this._providerAccessor.Invoke().Status;
+
         /// <inheritdoc />
         public EvaluationContext GetContext()
         {
@@ -69,16 +73,18 @@ public void SetContext(EvaluationContext? context)
         /// <summary>
         /// Initializes a new instance of the <see cref="FeatureClient"/> class.
         /// </summary>
+        /// <param name="providerAccessor">Function to retrieve current provider</param>
         /// <param name="name">Name of client <see cref="ClientMetadata"/></param>
         /// <param name="version">Version of client <see cref="ClientMetadata"/></param>
         /// <param name="logger">Logger used by client</param>
         /// <param name="context">Context given to this client</param>
         /// <exception cref="ArgumentNullException">Throws if any of the required parameters are null</exception>
-        public FeatureClient(string? name, string? version, ILogger? logger = null, EvaluationContext? context = null)
+        internal FeatureClient(Func<FeatureProvider> providerAccessor, string? name, string? version, ILogger? logger = null, EvaluationContext? context = null)
         {
             this._metadata = new ClientMetadata(name, version);
             this._logger = logger ?? NullLogger<FeatureClient>.Instance;
             this._evaluationContext = context ?? EvaluationContext.Empty;
+            this._providerAccessor = providerAccessor;
         }
 
         /// <inheritdoc />
@@ -246,6 +252,16 @@ private async Task<FlagEvaluationDetails<T>> EvaluateFlagAsync<T>(
             {
                 var contextFromHooks = await this.TriggerBeforeHooksAsync(allHooks, hookContext, options, cancellationToken).ConfigureAwait(false);
 
+                // short circuit evaluation entirely if provider is in a bad state
+                if (provider.Status == ProviderStatus.NotReady)
+                {
+                    throw new ProviderNotReadyException("Provider has not yet completed initialization.");
+                }
+                else if (provider.Status == ProviderStatus.Fatal)
+                {
+                    throw new ProviderFatalException("Provider is in an irrecoverable error state.");
+                }
+
                 evaluation =
                     (await resolveValueDelegate.Invoke(flagKey, defaultValue, contextFromHooks.EvaluationContext, cancellationToken).ConfigureAwait(false))
                     .ToFlagEvaluationDetails();