From 3c02309f104d46e8436cb2ffd97b38d6325d8c4e Mon Sep 17 00:00:00 2001 From: Jimmy Date: Sun, 9 Jun 2024 00:35:54 +0800 Subject: [PATCH 1/9] catch plugin exceptions. --- src/Neo/Ledger/Blockchain.cs | 63 ++++++++++++++++++++++++++++++++++-- src/Neo/Plugins/Plugin.cs | 10 +++++- 2 files changed, 70 insertions(+), 3 deletions(-) diff --git a/src/Neo/Ledger/Blockchain.cs b/src/Neo/Ledger/Blockchain.cs index ff9c8e29d8..620a6a1956 100644 --- a/src/Neo/Ledger/Blockchain.cs +++ b/src/Neo/Ledger/Blockchain.cs @@ -16,6 +16,7 @@ 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; @@ -468,10 +469,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); + InvokeCommitting(system, block, snapshot, all_application_executed); snapshot.Commit(); } - Committed?.Invoke(system, block); + InvokeCommitted(system, block); system.MemPool.UpdatePoolForBlockPersisted(block, system.StoreView); extensibleWitnessWhiteList = null; block_cache.Remove(block.PrevHash); @@ -480,6 +481,64 @@ private void Persist(Block block) Debug.Assert(header.Index == block.Index); } + private void InvokeCommitting(NeoSystem system, Block block, DataCache snapshot, IReadOnlyList applicationExecutedList) + { + var handlers = Committing?.GetInvocationList(); + if (handlers == null) return; + + foreach (var @delegate in handlers) + { + var handler = (CommittingHandler)@delegate; + try + { + handler(system, block, snapshot, applicationExecutedList); + } + catch (Exception ex) + { + if (handler.Target is Plugin) + { + // Log the exception and continue with the next handler + Utility.Log(nameof(handler.Target), LogLevel.Error, ex); + } + else + { + // Rethrow the exception if the handler is not from a plugin + Console.WriteLine($"Exception in committing handler: {ex.Message}"); + throw; + } + } + } + } + + private void InvokeCommitted(NeoSystem system, Block block) + { + var handlers = Committed?.GetInvocationList(); + if (handlers == null) return; + + foreach (var @delegate in handlers) + { + var handler = (CommittedHandler)@delegate; + try + { + handler(system, block); + } + catch (Exception ex) + { + if (handler.Target is Plugin) + { + // Log the exception and continue with the next handler + Utility.Log(nameof(handler.Target), LogLevel.Error, ex); + } + else + { + // Rethrow the exception if the handler is not from a plugin + Console.WriteLine($"Exception in committed handler: {ex.Message}"); + throw; + } + } + } + } + /// /// Gets a object used for creating the actor. /// diff --git a/src/Neo/Plugins/Plugin.cs b/src/Neo/Plugins/Plugin.cs index 248301af56..3eefd4e7da 100644 --- a/src/Neo/Plugins/Plugin.cs +++ b/src/Neo/Plugins/Plugin.cs @@ -229,7 +229,15 @@ protected internal virtual void OnSystemLoaded(NeoSystem system) /// if the is handled by a plugin; otherwise, . 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; + } } } } From e426ed6f755801ba12e4d2ee5ef0835c145c19e2 Mon Sep 17 00:00:00 2001 From: Jimmy Date: Sun, 9 Jun 2024 00:50:11 +0800 Subject: [PATCH 2/9] add UT test --- src/Neo/Ledger/Blockchain.cs | 4 ++-- tests/Neo.UnitTests/Plugins/TestPlugin.cs | 21 ++++++++++++++++++++- tests/Neo.UnitTests/Plugins/UT_Plugin.cs | 9 +++++++++ 3 files changed, 31 insertions(+), 3 deletions(-) diff --git a/src/Neo/Ledger/Blockchain.cs b/src/Neo/Ledger/Blockchain.cs index 620a6a1956..c2979b67a9 100644 --- a/src/Neo/Ledger/Blockchain.cs +++ b/src/Neo/Ledger/Blockchain.cs @@ -481,7 +481,7 @@ private void Persist(Block block) Debug.Assert(header.Index == block.Index); } - private void InvokeCommitting(NeoSystem system, Block block, DataCache snapshot, IReadOnlyList applicationExecutedList) + internal static void InvokeCommitting(NeoSystem system, Block block, DataCache snapshot, IReadOnlyList applicationExecutedList) { var handlers = Committing?.GetInvocationList(); if (handlers == null) return; @@ -510,7 +510,7 @@ private void InvokeCommitting(NeoSystem system, Block block, DataCache snapshot, } } - private void InvokeCommitted(NeoSystem system, Block block) + internal static void InvokeCommitted(NeoSystem system, Block block) { var handlers = Committed?.GetInvocationList(); if (handlers == null) return; diff --git a/tests/Neo.UnitTests/Plugins/TestPlugin.cs b/tests/Neo.UnitTests/Plugins/TestPlugin.cs index dde500b927..59d8ed98a6 100644 --- a/tests/Neo.UnitTests/Plugins/TestPlugin.cs +++ b/tests/Neo.UnitTests/Plugins/TestPlugin.cs @@ -10,13 +10,22 @@ // 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 TestPlugin : Plugin { - public TestPlugin() : base() { } + public TestPlugin() : base() + { + Blockchain.Committing += OnCommitting; + Blockchain.Committed += OnCommitted; + } protected override void Configure() { } @@ -36,5 +45,15 @@ public IConfigurationSection TestGetConfiguration() } protected override bool OnMessage(object message) => true; + + private void OnCommitting(NeoSystem system, Block block, DataCache snapshot, IReadOnlyList applicationExecutedList) + { + throw new NotImplementedException(); + } + + private void OnCommitted(NeoSystem system, Block block) + { + throw new NotImplementedException(); + } } } diff --git a/tests/Neo.UnitTests/Plugins/UT_Plugin.cs b/tests/Neo.UnitTests/Plugins/UT_Plugin.cs index e5e8cb6e49..7fc272d17f 100644 --- a/tests/Neo.UnitTests/Plugins/UT_Plugin.cs +++ b/tests/Neo.UnitTests/Plugins/UT_Plugin.cs @@ -11,6 +11,7 @@ using FluentAssertions; using Microsoft.VisualStudio.TestTools.UnitTesting; +using Neo.Ledger; using Neo.Plugins; using System; @@ -63,5 +64,13 @@ public void TestGetConfiguration() var pp = new TestPlugin(); pp.TestGetConfiguration().Key.Should().Be("PluginConfiguration"); } + + [TestMethod] + public void TestOnException() + { + var pp = new TestPlugin(); + + Blockchain.InvokeCommitted(null,null); + } } } From 5d6fa28e65ff05e0a794bb5e050bc260376efe24 Mon Sep 17 00:00:00 2001 From: Jimmy Date: Sun, 9 Jun 2024 00:52:55 +0800 Subject: [PATCH 3/9] udpate format --- tests/Neo.UnitTests/Plugins/UT_Plugin.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Neo.UnitTests/Plugins/UT_Plugin.cs b/tests/Neo.UnitTests/Plugins/UT_Plugin.cs index 7fc272d17f..ed82e41cb5 100644 --- a/tests/Neo.UnitTests/Plugins/UT_Plugin.cs +++ b/tests/Neo.UnitTests/Plugins/UT_Plugin.cs @@ -70,7 +70,7 @@ public void TestOnException() { var pp = new TestPlugin(); - Blockchain.InvokeCommitted(null,null); + Blockchain.InvokeCommitted(null, null); } } } From 439f504f2154c41cc250ed38c46384fc4536b1b8 Mon Sep 17 00:00:00 2001 From: Jimmy Date: Sun, 9 Jun 2024 00:58:50 +0800 Subject: [PATCH 4/9] make the test more complete --- tests/Neo.UnitTests/Plugins/UT_Plugin.cs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/tests/Neo.UnitTests/Plugins/UT_Plugin.cs b/tests/Neo.UnitTests/Plugins/UT_Plugin.cs index ed82e41cb5..46e2e4fb03 100644 --- a/tests/Neo.UnitTests/Plugins/UT_Plugin.cs +++ b/tests/Neo.UnitTests/Plugins/UT_Plugin.cs @@ -70,7 +70,15 @@ public void TestOnException() { var pp = new TestPlugin(); - Blockchain.InvokeCommitted(null, null); + // Call the InvokeCommitted method and ensure no exception is thrown + try + { + Blockchain.InvokeCommitted(null, null); + } + catch (Exception ex) + { + Assert.Fail($"InvokeCommitted threw an exception: {ex.Message}"); + } } } } From e569fed499b52726143339fc7dadcd0b311cc6b6 Mon Sep 17 00:00:00 2001 From: Jimmy Date: Sun, 9 Jun 2024 00:59:38 +0800 Subject: [PATCH 5/9] complete the ut test --- tests/Neo.UnitTests/Plugins/UT_Plugin.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/Neo.UnitTests/Plugins/UT_Plugin.cs b/tests/Neo.UnitTests/Plugins/UT_Plugin.cs index 46e2e4fb03..fdf33ddfb6 100644 --- a/tests/Neo.UnitTests/Plugins/UT_Plugin.cs +++ b/tests/Neo.UnitTests/Plugins/UT_Plugin.cs @@ -73,6 +73,7 @@ public void TestOnException() // Call the InvokeCommitted method and ensure no exception is thrown try { + Blockchain.InvokeCommitting(null, null,null,null); Blockchain.InvokeCommitted(null, null); } catch (Exception ex) From d701dd35999f2001522d57cfdd4ee16978e5859a Mon Sep 17 00:00:00 2001 From: Jimmy Date: Sun, 9 Jun 2024 01:01:13 +0800 Subject: [PATCH 6/9] format --- tests/Neo.UnitTests/Plugins/UT_Plugin.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Neo.UnitTests/Plugins/UT_Plugin.cs b/tests/Neo.UnitTests/Plugins/UT_Plugin.cs index fdf33ddfb6..2492eebdbe 100644 --- a/tests/Neo.UnitTests/Plugins/UT_Plugin.cs +++ b/tests/Neo.UnitTests/Plugins/UT_Plugin.cs @@ -73,7 +73,7 @@ public void TestOnException() // Call the InvokeCommitted method and ensure no exception is thrown try { - Blockchain.InvokeCommitting(null, null,null,null); + Blockchain.InvokeCommitting(null, null, null, null); Blockchain.InvokeCommitted(null, null); } catch (Exception ex) From db1c7f01699c21084c917bb93a968ba61239af33 Mon Sep 17 00:00:00 2001 From: Jimmy Date: Sun, 9 Jun 2024 01:10:59 +0800 Subject: [PATCH 7/9] complete UT tests with NonPlugin case --- tests/Neo.UnitTests/Plugins/TestPlugin.cs | 19 +++++++++++++++++++ tests/Neo.UnitTests/Plugins/UT_Plugin.cs | 11 ++++++++++- 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/tests/Neo.UnitTests/Plugins/TestPlugin.cs b/tests/Neo.UnitTests/Plugins/TestPlugin.cs index 59d8ed98a6..e5400350cc 100644 --- a/tests/Neo.UnitTests/Plugins/TestPlugin.cs +++ b/tests/Neo.UnitTests/Plugins/TestPlugin.cs @@ -19,6 +19,25 @@ namespace Neo.UnitTests.Plugins { + public class TestNonPlugin + { + public TestNonPlugin() : base() + { + Blockchain.Committing += OnCommitting; + Blockchain.Committed += OnCommitted; + } + + private void OnCommitting(NeoSystem system, Block block, DataCache snapshot, IReadOnlyList applicationExecutedList) + { + throw new NotImplementedException(); + } + + private void OnCommitted(NeoSystem system, Block block) + { + throw new NotImplementedException(); + } + } + public class TestPlugin : Plugin { public TestPlugin() : base() diff --git a/tests/Neo.UnitTests/Plugins/UT_Plugin.cs b/tests/Neo.UnitTests/Plugins/UT_Plugin.cs index 2492eebdbe..124ef21474 100644 --- a/tests/Neo.UnitTests/Plugins/UT_Plugin.cs +++ b/tests/Neo.UnitTests/Plugins/UT_Plugin.cs @@ -68,7 +68,7 @@ public void TestGetConfiguration() [TestMethod] public void TestOnException() { - var pp = new TestPlugin(); + _ = new TestPlugin(); // Call the InvokeCommitted method and ensure no exception is thrown try @@ -80,6 +80,15 @@ public void TestOnException() { Assert.Fail($"InvokeCommitted threw an exception: {ex.Message}"); } + + _ = new TestNonPlugin(); + + // Call the InvokeCommitted method and ensure exception is thrown + Assert.ThrowsException(() => + { + Blockchain.InvokeCommitting(null, null, null, null); + Blockchain.InvokeCommitted(null, null); + }); } } } From 17f74efb6e879085db5bcc4bfe2bcb276a4642ac Mon Sep 17 00:00:00 2001 From: Jimmy Date: Sun, 9 Jun 2024 08:15:28 +0800 Subject: [PATCH 8/9] async invoke --- src/Neo/Ledger/Blockchain.cs | 59 +++++++++-------------- tests/Neo.UnitTests/Plugins/TestPlugin.cs | 7 +-- tests/Neo.UnitTests/Plugins/UT_Plugin.cs | 26 +++++----- 3 files changed, 42 insertions(+), 50 deletions(-) diff --git a/src/Neo/Ledger/Blockchain.cs b/src/Neo/Ledger/Blockchain.cs index c2979b67a9..0d6eb4fc40 100644 --- a/src/Neo/Ledger/Blockchain.cs +++ b/src/Neo/Ledger/Blockchain.cs @@ -12,6 +12,7 @@ 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; @@ -21,10 +22,12 @@ 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 { @@ -469,10 +472,10 @@ private void Persist(Block block) Context.System.EventStream.Publish(application_executed); all_application_executed.Add(application_executed); } - InvokeCommitting(system, block, snapshot, all_application_executed); + _ = InvokeCommittingAsync(system, block, snapshot, all_application_executed); snapshot.Commit(); } - InvokeCommitted(system, block); + _ = InvokeCommittedAsync(system, block); system.MemPool.UpdatePoolForBlockPersisted(block, system.StoreView); extensibleWitnessWhiteList = null; block_cache.Remove(block.PrevHash); @@ -481,64 +484,48 @@ private void Persist(Block block) Debug.Assert(header.Index == block.Index); } - internal static void InvokeCommitting(NeoSystem system, Block block, DataCache snapshot, IReadOnlyList applicationExecutedList) + internal static async Task InvokeCommittingAsync(NeoSystem system, Block block, DataCache snapshot, IReadOnlyList applicationExecutedList) { - var handlers = Committing?.GetInvocationList(); - if (handlers == null) return; + await InvokeHandlersAsync(Committing?.GetInvocationList(), h => ((CommittingHandler)h)(system, block, snapshot, applicationExecutedList)); + } - foreach (var @delegate in handlers) - { - var handler = (CommittingHandler)@delegate; - try - { - handler(system, block, snapshot, applicationExecutedList); - } - catch (Exception ex) - { - if (handler.Target is Plugin) - { - // Log the exception and continue with the next handler - Utility.Log(nameof(handler.Target), LogLevel.Error, ex); - } - else - { - // Rethrow the exception if the handler is not from a plugin - Console.WriteLine($"Exception in committing handler: {ex.Message}"); - throw; - } - } - } + internal static async Task InvokeCommittedAsync(NeoSystem system, Block block) + { + await InvokeHandlersAsync(Committed?.GetInvocationList(), h => ((CommittedHandler)h)(system, block)); } - internal static void InvokeCommitted(NeoSystem system, Block block) + private static async Task InvokeHandlersAsync(Delegate[] handlers, Action handlerAction) { - var handlers = Committed?.GetInvocationList(); if (handlers == null) return; - foreach (var @delegate in handlers) + var exceptions = new ConcurrentBag(); + var tasks = handlers.Select(handler => Task.Run(() => { - var handler = (CommittedHandler)@delegate; try { - handler(system, block); + handlerAction(handler); } catch (Exception ex) { if (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); } else { - // Rethrow the exception if the handler is not from a plugin - Console.WriteLine($"Exception in committed handler: {ex.Message}"); - throw; + exceptions.Add(ex); } } - } + })).ToList(); + + await Task.WhenAll(tasks); + + exceptions.ForEach(e => throw e); } + /// /// Gets a object used for creating the actor. /// diff --git a/tests/Neo.UnitTests/Plugins/TestPlugin.cs b/tests/Neo.UnitTests/Plugins/TestPlugin.cs index e5400350cc..8bc058c2a2 100644 --- a/tests/Neo.UnitTests/Plugins/TestPlugin.cs +++ b/tests/Neo.UnitTests/Plugins/TestPlugin.cs @@ -21,7 +21,7 @@ namespace Neo.UnitTests.Plugins { public class TestNonPlugin { - public TestNonPlugin() : base() + public TestNonPlugin() { Blockchain.Committing += OnCommitting; Blockchain.Committed += OnCommitted; @@ -29,15 +29,16 @@ public TestNonPlugin() : base() private void OnCommitting(NeoSystem system, Block block, DataCache snapshot, IReadOnlyList applicationExecutedList) { - throw new NotImplementedException(); + throw new NotImplementedException("Test exception from OnCommitting"); } private void OnCommitted(NeoSystem system, Block block) { - throw new NotImplementedException(); + throw new NotImplementedException("Test exception from OnCommitted"); } } + public class TestPlugin : Plugin { public TestPlugin() : base() diff --git a/tests/Neo.UnitTests/Plugins/UT_Plugin.cs b/tests/Neo.UnitTests/Plugins/UT_Plugin.cs index 124ef21474..1ad286ae9a 100644 --- a/tests/Neo.UnitTests/Plugins/UT_Plugin.cs +++ b/tests/Neo.UnitTests/Plugins/UT_Plugin.cs @@ -14,6 +14,7 @@ using Neo.Ledger; using Neo.Plugins; using System; +using System.Threading.Tasks; namespace Neo.UnitTests.Plugins { @@ -66,28 +67,31 @@ public void TestGetConfiguration() } [TestMethod] - public void TestOnException() + public async Task TestOnException() { - _ = new TestPlugin(); - - // Call the InvokeCommitted method and ensure no exception is thrown + // Ensure no exception is thrown try { - Blockchain.InvokeCommitting(null, null, null, null); - Blockchain.InvokeCommitted(null, null); + await Blockchain.InvokeCommittingAsync(null, null, null, null); + await Blockchain.InvokeCommittedAsync(null, null); } catch (Exception ex) { - Assert.Fail($"InvokeCommitted threw an exception: {ex.Message}"); + Assert.Fail($"InvokeCommitting or InvokeCommitted threw an exception: {ex.Message}"); } + // Register TestNonPlugin that throws exceptions _ = new TestNonPlugin(); - // Call the InvokeCommitted method and ensure exception is thrown - Assert.ThrowsException(() => + // Ensure exception is thrown + await Assert.ThrowsExceptionAsync(async () => + { + await Blockchain.InvokeCommittingAsync(null, null, null, null); + }); + + await Assert.ThrowsExceptionAsync(async () => { - Blockchain.InvokeCommitting(null, null, null, null); - Blockchain.InvokeCommitted(null, null); + await Blockchain.InvokeCommittedAsync(null, null); }); } } From 6d06a53a81957ae1f3b914695537390e5b990afc Mon Sep 17 00:00:00 2001 From: Jimmy Date: Sun, 9 Jun 2024 14:59:03 +0800 Subject: [PATCH 9/9] Update src/Neo/Ledger/Blockchain.cs Co-authored-by: Christopher Schuchardt --- src/Neo/Ledger/Blockchain.cs | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/src/Neo/Ledger/Blockchain.cs b/src/Neo/Ledger/Blockchain.cs index 0d6eb4fc40..0b53d47c18 100644 --- a/src/Neo/Ledger/Blockchain.cs +++ b/src/Neo/Ledger/Blockchain.cs @@ -505,18 +505,15 @@ private static async Task InvokeHandlersAsync(Delegate[] handlers, Action