fix unit tests, add additional test cases

Signed-off-by: Florian Bacher <[email protected]>
open-feature · Jan 8, 2024 · c30345a · c30345a
1 parent 0f419cf
commit c30345a
Show file tree

Hide file tree

Showing 3 changed files with 106 additions and 10 deletions.
diff --git a/src/OpenFeature/Api.cs b/src/OpenFeature/Api.cs
@@ -206,7 +206,7 @@ public EvaluationContext GetContext()
         public async Task Shutdown()
         {
             await this._repository.Shutdown().ConfigureAwait(false);
-            await this.EventExecutor.SignalShutdownAsync().ConfigureAwait(false);
+            //await this.EventExecutor.SignalShutdownAsync().ConfigureAwait(false);
         }
 
         /// <inheritdoc />

diff --git a/src/OpenFeature/EventExecutor.cs b/src/OpenFeature/EventExecutor.cs
@@ -3,6 +3,7 @@
 using System.Threading;
 using System.Threading.Channels;
 using System.Threading.Tasks;
+using Microsoft.Extensions.Logging;
 using OpenFeature.Constant;
 using OpenFeature.Model;
 
@@ -196,12 +197,19 @@ private void EmitOnRegistration(FeatureProviderReference provider, ProviderEvent
 
             if (message != "")
             {
-                handler(new ProviderEventPayload
+                try
                 {
-                    ProviderName = provider.Provider.GetMetadata().Name,
-                    Type = eventType,
-                    Message = message
-                });
+                    handler.Invoke(new ProviderEventPayload
+                    {
+                        ProviderName = provider.Provider.GetMetadata().Name,
+                        Type = eventType,
+                        Message = message
+                    });
+                }
+                catch (Exception)
+                {
+                    throw;
+                }
             }
         }
 
@@ -244,7 +252,7 @@ private async void ProcessEventAsync()
                             {
                                 foreach (var eventHandler in eventHandlers)
                                 {
-                                    eventHandler.Invoke(e.EventPayload);
+                                    this.InvokeEventHandler(eventHandler, e);
                                 }
                             }
 
@@ -259,7 +267,7 @@ private async void ProcessEventAsync()
                                         {
                                             foreach (var eventHandler in clientEventHandlers)
                                             {
-                                                eventHandler.Invoke(e.EventPayload);
+                                                this.InvokeEventHandler(eventHandler, e);
                                             }
                                         }
                                     }
@@ -283,7 +291,7 @@ private async void ProcessEventAsync()
                                 {
                                     foreach (var eventHandler in clientEventHandlers)
                                     {
-                                        eventHandler.Invoke(e.EventPayload);
+                                        this.InvokeEventHandler(eventHandler, e);
                                     }
                                 }
                             }
@@ -297,6 +305,17 @@ private async void ProcessEventAsync()
             }
         }
 
+        private void InvokeEventHandler(EventHandlerDelegate eventHandler, Event e)
+        {
+            try
+            {
+                eventHandler.Invoke(e.EventPayload);
+            } catch (Exception exc)
+            {
+                Console.WriteLine(exc);
+            }
+        }
+
         // Method to signal shutdown
         public async Task SignalShutdownAsync()
         {

diff --git a/test/OpenFeature.Tests/OpenFeatureEventTests.cs b/test/OpenFeature.Tests/OpenFeatureEventTests.cs
@@ -1,3 +1,4 @@
+using System;
 using System.Diagnostics.CodeAnalysis;
 using System.Threading;
 using System.Threading.Tasks;
@@ -181,6 +182,7 @@ public async Task API_Level_Event_Handlers_Should_Be_Informed_About_Ready_State_
         [Specification("5.2.2", "The `API` MUST provide a function for associating `handler functions` with a particular `provider event type`.")]
         [Specification("5.2.3", "The `event details` MUST contain the `provider name` associated with the event.")]
         [Specification("5.2.4", "The `handler function` MUST accept a `event details` parameter.")]
+        [Specification("5.2.6", "Event handlers MUST persist across `provider` changes.")]
         public async Task API_Level_Event_Handlers_Should_Be_Exchangeable()
         {
             var eventHandler = Substitute.For<EventHandlerDelegate>();
@@ -220,6 +222,42 @@ public async Task API_Level_Event_Handlers_Should_Be_Removable()
             eventHandler.Received(1).Invoke(Arg.Is<ProviderEventPayload>(payload => payload.ProviderName == testProvider.GetMetadata().Name));
         }
 
+        [Fact]
+        [Specification("5.1.2", "When a `provider` signals the occurrence of a particular `event`, the associated `client` and `API` event handlers MUST run.")]
+        [Specification("5.2.1", "The `client` MUST provide a function for associating `handler functions` with a particular `provider event type`.")]
+        [Specification("5.2.3", "The `event details` MUST contain the `provider name` associated with the event.")]
+        [Specification("5.2.4", "The `handler function` MUST accept a `event details` parameter.")]
+        [Specification("5.2.5", "If a `handler function` terminates abnormally, other `handler` functions MUST run.")]
+        public async Task API_Level_Event_Handlers_Should_Be_Executed_When_Other_Handler_Fails()
+        {
+            try
+            {
+                var fixture = new Fixture();
+
+                var failingEventHandler = Substitute.For<EventHandlerDelegate>();
+                var eventHandler = Substitute.For<EventHandlerDelegate>();
+
+                failingEventHandler.When(x => x.Invoke(Arg.Any<ProviderEventPayload>()))
+                    .Do(x => throw new Exception());
+
+                eventHandler.Invoke(Arg.Any<ProviderEventPayload>());
+
+                Api.Instance.AddHandler(ProviderEventTypes.ProviderReady, failingEventHandler);
+                Api.Instance.AddHandler(ProviderEventTypes.ProviderReady, eventHandler);
+
+                var testProvider = new TestProvider(fixture.Create<string>());
+                await Api.Instance.SetProvider(testProvider);
+
+                failingEventHandler.Received().Invoke(Arg.Is<ProviderEventPayload>(payload => payload.ProviderName == testProvider.GetMetadata().Name));
+                eventHandler.Received().Invoke(Arg.Is<ProviderEventPayload>(payload => payload.ProviderName == testProvider.GetMetadata().Name));
+            }
+            catch (Exception e)
+            {
+                // make sure the exception is passed on correctly
+                Assert.NotNull(e);
+            }
+        }
+
         [Fact]
         [Specification("5.1.2", "When a `provider` signals the occurrence of a particular `event`, the associated `client` and `API` event handlers MUST run.")]
         [Specification("5.2.1", "The `client` MUST provide a function for associating `handler functions` with a particular `provider event type`.")]
@@ -242,6 +280,45 @@ public async Task Client_Level_Event_Handlers_Should_Be_Registered()
             eventHandler.Received().Invoke(Arg.Is<ProviderEventPayload>(payload => payload.ProviderName == testProvider.GetMetadata().Name));
         }
 
+        [Fact]
+        [Specification("5.1.2", "When a `provider` signals the occurrence of a particular `event`, the associated `client` and `API` event handlers MUST run.")]
+        [Specification("5.2.1", "The `client` MUST provide a function for associating `handler functions` with a particular `provider event type`.")]
+        [Specification("5.2.3", "The `event details` MUST contain the `provider name` associated with the event.")]
+        [Specification("5.2.4", "The `handler function` MUST accept a `event details` parameter.")]
+        [Specification("5.2.5", "If a `handler function` terminates abnormally, other `handler` functions MUST run.")]
+        public async Task Client_Level_Event_Handlers_Should_Be_Executed_When_Other_Handler_Fails()
+        {
+            try
+            {
+                var fixture = new Fixture();
+
+                var failingEventHandler = Substitute.For<EventHandlerDelegate>();
+                var eventHandler = Substitute.For<EventHandlerDelegate>();
+
+                failingEventHandler.When(x => x.Invoke(Arg.Any<ProviderEventPayload>()))
+                    .Do(x => throw new Exception());
+
+                eventHandler.Invoke(Arg.Any<ProviderEventPayload>());
+
+                var myClient = Api.Instance.GetClient(fixture.Create<string>());
+
+                myClient.AddHandler(ProviderEventTypes.ProviderReady, failingEventHandler);
+                myClient.AddHandler(ProviderEventTypes.ProviderReady, eventHandler);
+
+                var testProvider = new TestProvider();
+                await Api.Instance.SetProvider(myClient.GetMetadata().Name, testProvider);
+
+                failingEventHandler.Received().Invoke(Arg.Is<ProviderEventPayload>(payload => payload.ProviderName == testProvider.GetMetadata().Name));
+                eventHandler.Received().Invoke(Arg.Is<ProviderEventPayload>(payload => payload.ProviderName == testProvider.GetMetadata().Name));
+            }
+            catch (Exception e)
+            {
+                // make sure the exception is passed on correctly
+                Assert.NotNull(e);
+            }
+
+        }
+
         [Fact]
         [Specification("5.1.2", "When a `provider` signals the occurrence of a particular `event`, the associated `client` and `API` event handlers MUST run.")]
         [Specification("5.1.3", "When a `provider` signals the occurrence of a particular `event`, event handlers on clients which are not associated with that provider MUST NOT run.")]
@@ -322,7 +399,7 @@ public async Task Client_Level_Event_Handlers_Should_Be_Removable()
             await Api.Instance.SetProvider(myClient.GetMetadata().Name, testProvider);
 
             // wait for the first event to be received
-            Thread.Sleep(2000);
+            Thread.Sleep(1000);
             myClient.RemoveHandler(ProviderEventTypes.ProviderReady, eventHandler);
 
             // send another event from the provider - this one should not be received