[Neo Core] Part 1. Isolate Plugins Exceptions from the Node.

src/Neo/Ledger/Blockchain.cs

@@ -12,18 +12,22 @@
 using Akka.Actor;
 using Akka.Configuration;
 using Akka.IO;
+using Akka.Util.Internal;
 using Neo.IO.Actors;
 using Neo.Network.P2P;
 using Neo.Network.P2P.Payloads;
 using Neo.Persistence;
+using Neo.Plugins;
 using Neo.SmartContract;
 using Neo.SmartContract.Native;
 using Neo.VM;
 using System;
+using System.Collections.Concurrent;
 using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.Diagnostics;
 using System.Linq;
+using System.Threading.Tasks;
 
 namespace Neo.Ledger
 {
@@ -468,10 +472,10 @@ private void Persist(Block block)
                     Context.System.EventStream.Publish(application_executed);
                     all_application_executed.Add(application_executed);
                 }
-                Committing?.Invoke(system, block, snapshot, all_application_executed);
+                _ = InvokeCommittingAsync(system, block, snapshot, all_application_executed);
                 snapshot.Commit();
             }
-            Committed?.Invoke(system, block);
+            _ = InvokeCommittedAsync(system, block);
             system.MemPool.UpdatePoolForBlockPersisted(block, system.StoreView);
             extensibleWitnessWhiteList = null;
             block_cache.Remove(block.PrevHash);
@@ -480,6 +484,45 @@ private void Persist(Block block)
                 Debug.Assert(header.Index == block.Index);
         }
 
+        internal static async Task InvokeCommittingAsync(NeoSystem system, Block block, DataCache snapshot, IReadOnlyList<ApplicationExecuted> applicationExecutedList)
+        {
+            await InvokeHandlersAsync(Committing?.GetInvocationList(), h => ((CommittingHandler)h)(system, block, snapshot, applicationExecutedList));
+        }
+
+        internal static async Task InvokeCommittedAsync(NeoSystem system, Block block)
+        {
+            await InvokeHandlersAsync(Committed?.GetInvocationList(), h => ((CommittedHandler)h)(system, block));
+        }
+
+        private static async Task InvokeHandlersAsync(Delegate[] handlers, Action<Delegate> handlerAction)
+        {
+            if (handlers == null) return;
+
+            var exceptions = new ConcurrentBag<Exception>();
+            var tasks = handlers.Select(handler => Task.Run(() =>
+            {
+                try
+                {
+                    handlerAction(handler);
+                }
+                catch (Exception ex) when (handler.Target is Plugin)
+                {
+                    // Log the exception and continue with the next handler
+                    // Isolate the plugin exception
+                    Utility.Log(nameof(handler.Target), LogLevel.Error, ex);
+                }
+                catch (Exception ex)
+                {
+                    exceptions.Add(ex);
+                }
+            })).ToList();
+
+            await Task.WhenAll(tasks);
+
+            exceptions.ForEach(e => throw e);
+        }
+
+
         /// <summary>
         /// Gets a <see cref="Akka.Actor.Props"/> object used for creating the <see cref="Blockchain"/> actor.
         /// </summary>

src/Neo/Plugins/Plugin.cs

@@ -229,7 +229,15 @@ protected internal virtual void OnSystemLoaded(NeoSystem system)
         /// <returns><see langword="true"/> if the <paramref name="message"/> is handled by a plugin; otherwise, <see langword="false"/>.</returns>
         public static bool SendMessage(object message)
         {
-            return Plugins.Any(plugin => plugin.OnMessage(message));
+            try
+            {
+                return Plugins.Any(plugin => plugin.OnMessage(message));
+            }
+            catch (Exception ex)
+            {
+                Utility.Log(nameof(Plugin), LogLevel.Error, ex);
+                return false;
+            }
         }
     }
 }

tests/Neo.UnitTests/Plugins/TestPlugin.cs

@@ -10,13 +10,42 @@
 // modifications are permitted.
 
 using Microsoft.Extensions.Configuration;
+using Neo.Ledger;
+using Neo.Network.P2P.Payloads;
+using Neo.Persistence;
 using Neo.Plugins;
+using System;
+using System.Collections.Generic;
 
 namespace Neo.UnitTests.Plugins
 {
+    public class TestNonPlugin
+    {
+        public TestNonPlugin()
+        {
+            Blockchain.Committing += OnCommitting;
+            Blockchain.Committed += OnCommitted;
+        }
+
+        private void OnCommitting(NeoSystem system, Block block, DataCache snapshot, IReadOnlyList<Blockchain.ApplicationExecuted> applicationExecutedList)
+        {
+            throw new NotImplementedException("Test exception from OnCommitting");
+        }
+
+        private void OnCommitted(NeoSystem system, Block block)
+        {
+            throw new NotImplementedException("Test exception from OnCommitted");
+        }
+    }
+
+
     public class TestPlugin : Plugin
     {
-        public TestPlugin() : base() { }
+        public TestPlugin() : base()
+        {
+            Blockchain.Committing += OnCommitting;
+            Blockchain.Committed += OnCommitted;
+        }
 
         protected override void Configure() { }
 
@@ -36,5 +65,15 @@ public IConfigurationSection TestGetConfiguration()
         }
 
         protected override bool OnMessage(object message) => true;
+
+        private void OnCommitting(NeoSystem system, Block block, DataCache snapshot, IReadOnlyList<Blockchain.ApplicationExecuted> applicationExecutedList)
+        {
+            throw new NotImplementedException();
+        }
+
+        private void OnCommitted(NeoSystem system, Block block)
+        {
+            throw new NotImplementedException();
+        }
     }
 }

tests/Neo.UnitTests/Plugins/UT_Plugin.cs

@@ -11,8 +11,10 @@
 
 using FluentAssertions;
 using Microsoft.VisualStudio.TestTools.UnitTesting;
+using Neo.Ledger;
 using Neo.Plugins;
 using System;
+using System.Threading.Tasks;
 
 namespace Neo.UnitTests.Plugins
 {
@@ -63,5 +65,34 @@ public void TestGetConfiguration()
             var pp = new TestPlugin();
             pp.TestGetConfiguration().Key.Should().Be("PluginConfiguration");
         }
+
+        [TestMethod]
+        public async Task TestOnException()
+        {
+            // Ensure no exception is thrown
+            try
+            {
+                await Blockchain.InvokeCommittingAsync(null, null, null, null);
+                await Blockchain.InvokeCommittedAsync(null, null);
+            }
+            catch (Exception ex)
+            {
+                Assert.Fail($"InvokeCommitting or InvokeCommitted threw an exception: {ex.Message}");
+            }
+
+            // Register TestNonPlugin that throws exceptions
+            _ = new TestNonPlugin();
+
+            // Ensure exception is thrown
+            await Assert.ThrowsExceptionAsync<NotImplementedException>(async () =>
+            {
+                await Blockchain.InvokeCommittingAsync(null, null, null, null);
+            });
+
+            await Assert.ThrowsExceptionAsync<NotImplementedException>(async () =>
+            {
+                await Blockchain.InvokeCommittedAsync(null, null);
+            });
+        }
     }
 }