feat: Support Returning Error Resolutions from Providers (#323)

When provider resolutions with error code set other than `None` are returned, the provider acts as if an error was thrown. Signed-off-by: christian.lutnik <[email protected]> Signed-off-by: Artyom Tonoyan <[email protected]>
open-feature · Dec 4, 2024 · e383697 · e383697
1 parent 480c790
commit e383697
Show file tree

Hide file tree

Showing 3 changed files with 78 additions and 5 deletions.
diff --git a/src/OpenFeature/OpenFeatureClient.cs b/src/OpenFeature/OpenFeatureClient.cs
@@ -263,7 +263,23 @@ private async Task<FlagEvaluationDetails<T>> EvaluateFlagAsync<T>(
                     (await resolveValueDelegate.Invoke(flagKey, defaultValue, contextFromHooks.EvaluationContext, cancellationToken).ConfigureAwait(false))
                     .ToFlagEvaluationDetails();
 
-                await this.TriggerAfterHooksAsync(allHooksReversed, hookContext, evaluation, options, cancellationToken).ConfigureAwait(false);
+                if (evaluation.ErrorType == ErrorType.None)
+                {
+                    await this.TriggerAfterHooksAsync(
+                        allHooksReversed,
+                        hookContext,
+                        evaluation,
+                        options,
+                        cancellationToken
+                    ).ConfigureAwait(false);
+                }
+                else
+                {
+                    var exception = new FeatureProviderException(evaluation.ErrorType, evaluation.ErrorMessage);
+                    this.FlagEvaluationErrorWithDescription(flagKey, evaluation.ErrorType.GetDescription(), exception);
+                    await this.TriggerErrorHooksAsync(allHooksReversed, hookContext, exception, options, cancellationToken)
+                        .ConfigureAwait(false);
+                }
             }
             catch (FeatureProviderException ex)
             {

diff --git a/test/OpenFeature.Tests/OpenFeatureClientTests.cs b/test/OpenFeature.Tests/OpenFeatureClientTests.cs
@@ -433,6 +433,41 @@ public async Task When_Exception_Occurs_During_Evaluation_Should_Return_Error()
             _ = featureProviderMock.Received(1).ResolveStructureValueAsync(flagName, defaultValue, Arg.Any<EvaluationContext>());
         }
 
+        [Fact]
+        public async Task When_Error_Is_Returned_From_Provider_Should_Not_Run_After_Hook_But_Error_Hook()
+        {
+            var fixture = new Fixture();
+            var domain = fixture.Create<string>();
+            var clientVersion = fixture.Create<string>();
+            var flagName = fixture.Create<string>();
+            var defaultValue = fixture.Create<Value>();
+            const string testMessage = "Couldn't parse flag data.";
+
+            var featureProviderMock = Substitute.For<FeatureProvider>();
+            featureProviderMock.ResolveStructureValueAsync(flagName, defaultValue, Arg.Any<EvaluationContext>())
+                .Returns(Task.FromResult(new ResolutionDetails<Value>(flagName, defaultValue, ErrorType.ParseError,
+                    "ERROR", null, testMessage)));
+            featureProviderMock.GetMetadata().Returns(new Metadata(fixture.Create<string>()));
+            featureProviderMock.GetProviderHooks().Returns(ImmutableList<Hook>.Empty);
+
+            await Api.Instance.SetProviderAsync(featureProviderMock);
+            var client = Api.Instance.GetClient(domain, clientVersion);
+            var testHook = new TestHook();
+            client.AddHooks(testHook);
+            var response = await client.GetObjectDetailsAsync(flagName, defaultValue);
+
+            response.ErrorType.Should().Be(ErrorType.ParseError);
+            response.Reason.Should().Be(Reason.Error);
+            response.ErrorMessage.Should().Be(testMessage);
+            _ = featureProviderMock.Received(1)
+                .ResolveStructureValueAsync(flagName, defaultValue, Arg.Any<EvaluationContext>());
+
+            Assert.Equal(1, testHook.BeforeCallCount);
+            Assert.Equal(0, testHook.AfterCallCount);
+            Assert.Equal(1, testHook.ErrorCallCount);
+            Assert.Equal(1, testHook.FinallyCallCount);
+        }
+
         [Fact]
         public async Task Cancellation_Token_Added_Is_Passed_To_Provider()
         {
@@ -454,6 +489,7 @@ public async Task Cancellation_Token_Added_Is_Passed_To_Provider()
                 {
                     await Task.Delay(10); // artificially delay until cancelled
                 }
+
                 return new ResolutionDetails<string>(flagName, defaultString, ErrorType.None, cancelledReason);
             });
             featureProviderMock.GetMetadata().Returns(new Metadata(fixture.Create<string>()));

diff --git a/test/OpenFeature.Tests/TestImplementations.cs b/test/OpenFeature.Tests/TestImplementations.cs
@@ -8,28 +8,49 @@
 
 namespace OpenFeature.Tests
 {
-    public class TestHookNoOverride : Hook { }
+    public class TestHookNoOverride : Hook
+    {
+    }
 
     public class TestHook : Hook
     {
-        public override ValueTask<EvaluationContext> BeforeAsync<T>(HookContext<T> context, IReadOnlyDictionary<string, object>? hints = null, CancellationToken cancellationToken = default)
+        private int _beforeCallCount;
+        public int BeforeCallCount { get => this._beforeCallCount; }
+
+        private int _afterCallCount;
+        public int AfterCallCount { get => this._afterCallCount; }
+
+        private int _errorCallCount;
+        public int ErrorCallCount { get => this._errorCallCount; }
+
+        private int _finallyCallCount;
+        public int FinallyCallCount { get => this._finallyCallCount; }
+
+        public override ValueTask<EvaluationContext> BeforeAsync<T>(HookContext<T> context,
+            IReadOnlyDictionary<string, object>? hints = null, CancellationToken cancellationToken = default)
         {
+            Interlocked.Increment(ref this._beforeCallCount);
             return new ValueTask<EvaluationContext>(EvaluationContext.Empty);
         }
 
         public override ValueTask AfterAsync<T>(HookContext<T> context, FlagEvaluationDetails<T> details,
             IReadOnlyDictionary<string, object>? hints = null, CancellationToken cancellationToken = default)
         {
+            Interlocked.Increment(ref this._afterCallCount);
             return new ValueTask();
         }
 
-        public override ValueTask ErrorAsync<T>(HookContext<T> context, Exception error, IReadOnlyDictionary<string, object>? hints = null, CancellationToken cancellationToken = default)
+        public override ValueTask ErrorAsync<T>(HookContext<T> context, Exception error,
+            IReadOnlyDictionary<string, object>? hints = null, CancellationToken cancellationToken = default)
         {
+            Interlocked.Increment(ref this._errorCallCount);
             return new ValueTask();
         }
 
-        public override ValueTask FinallyAsync<T>(HookContext<T> context, IReadOnlyDictionary<string, object>? hints = null, CancellationToken cancellationToken = default)
+        public override ValueTask FinallyAsync<T>(HookContext<T> context,
+            IReadOnlyDictionary<string, object>? hints = null, CancellationToken cancellationToken = default)
         {
+            Interlocked.Increment(ref this._finallyCallCount);
             return new ValueTask();
         }
     }