From bf9de4e177a4963340278854a25dd355f95dfc51 Mon Sep 17 00:00:00 2001 From: chrfwow Date: Tue, 3 Dec 2024 17:06:25 +0100 Subject: [PATCH] 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 --- src/OpenFeature/OpenFeatureClient.cs | 18 +++++++++- .../OpenFeatureClientTests.cs | 36 +++++++++++++++++++ test/OpenFeature.Tests/TestImplementations.cs | 29 ++++++++++++--- 3 files changed, 78 insertions(+), 5 deletions(-) diff --git a/src/OpenFeature/OpenFeatureClient.cs b/src/OpenFeature/OpenFeatureClient.cs index 08e29533..c2621785 100644 --- a/src/OpenFeature/OpenFeatureClient.cs +++ b/src/OpenFeature/OpenFeatureClient.cs @@ -263,7 +263,23 @@ private async Task> EvaluateFlagAsync( (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 index ce3e9e93..ee9eee0c 100644 --- 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()); } + [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(); + var clientVersion = fixture.Create(); + var flagName = fixture.Create(); + var defaultValue = fixture.Create(); + const string testMessage = "Couldn't parse flag data."; + + var featureProviderMock = Substitute.For(); + featureProviderMock.ResolveStructureValueAsync(flagName, defaultValue, Arg.Any()) + .Returns(Task.FromResult(new ResolutionDetails(flagName, defaultValue, ErrorType.ParseError, + "ERROR", null, testMessage))); + featureProviderMock.GetMetadata().Returns(new Metadata(fixture.Create())); + featureProviderMock.GetProviderHooks().Returns(ImmutableList.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()); + + 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(flagName, defaultString, ErrorType.None, cancelledReason); }); featureProviderMock.GetMetadata().Returns(new Metadata(fixture.Create())); diff --git a/test/OpenFeature.Tests/TestImplementations.cs b/test/OpenFeature.Tests/TestImplementations.cs index 7a1dff10..ea35b870 100644 --- 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 BeforeAsync(HookContext context, IReadOnlyDictionary? 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 BeforeAsync(HookContext context, + IReadOnlyDictionary? hints = null, CancellationToken cancellationToken = default) { + Interlocked.Increment(ref this._beforeCallCount); return new ValueTask(EvaluationContext.Empty); } public override ValueTask AfterAsync(HookContext context, FlagEvaluationDetails details, IReadOnlyDictionary? hints = null, CancellationToken cancellationToken = default) { + Interlocked.Increment(ref this._afterCallCount); return new ValueTask(); } - public override ValueTask ErrorAsync(HookContext context, Exception error, IReadOnlyDictionary? hints = null, CancellationToken cancellationToken = default) + public override ValueTask ErrorAsync(HookContext context, Exception error, + IReadOnlyDictionary? hints = null, CancellationToken cancellationToken = default) { + Interlocked.Increment(ref this._errorCallCount); return new ValueTask(); } - public override ValueTask FinallyAsync(HookContext context, IReadOnlyDictionary? hints = null, CancellationToken cancellationToken = default) + public override ValueTask FinallyAsync(HookContext context, + IReadOnlyDictionary? hints = null, CancellationToken cancellationToken = default) { + Interlocked.Increment(ref this._finallyCallCount); return new ValueTask(); } }