From 219b352cd8c88e9fb833303d6d851c489d7871fe Mon Sep 17 00:00:00 2001 From: Avinesh Singh Date: Wed, 28 Jan 2026 23:23:51 +0530 Subject: [PATCH 1/4] Propagate exceptions from command handlers correctly This is to make `CommandHandlerInvokerMiddleware` correctly 'await' the task before accessing its result, similar to how this is done in `RequestHandlerInvokerMiddleware`. Without this change, attempting to access result of a cancelled task would result in TargetInvocationException / AggregateException which breaks the task processing in `BackgroundCommandSenderHostedService` due to unhandled exception, thus breaking the background queue and particularly in bulk dispatch workflow causing remaining workflows to not get dispatched --- .../Extensions/HandlerExtensions.cs | 21 ++- .../CommandHandlerInvokerMiddleware.cs | 5 +- .../CommandCancellationBehaviorTests.cs | 131 ++++++++++++++++++ .../Elsa.Mediator.UnitTests.csproj | 11 ++ 4 files changed, 163 insertions(+), 5 deletions(-) create mode 100644 test/unit/Elsa.Mediator.UnitTests/CommandCancellationBehaviorTests.cs create mode 100644 test/unit/Elsa.Mediator.UnitTests/Elsa.Mediator.UnitTests.csproj diff --git a/src/common/Elsa.Mediator/Extensions/HandlerExtensions.cs b/src/common/Elsa.Mediator/Extensions/HandlerExtensions.cs index 695268c58b..17204949d1 100644 --- a/src/common/Elsa.Mediator/Extensions/HandlerExtensions.cs +++ b/src/common/Elsa.Mediator/Extensions/HandlerExtensions.cs @@ -47,7 +47,15 @@ public static Task InvokeAsync(this INotificationHandler handler, MethodBase han { var notification = notificationContext.Notification; var cancellationToken = notificationContext.CancellationToken; - return (Task)handleMethod.Invoke(handler, [notification, cancellationToken])!; + + try + { + return (Task)handleMethod.Invoke(handler, [notification, cancellationToken])!; + } + catch (TargetInvocationException ex) when (ex.InnerException is not null) + { + throw ex.InnerException; + } } /// @@ -60,7 +68,14 @@ public static Task InvokeAsync(this ICommandHandler handler, M { var command = commandContext.Command; var cancellationToken = commandContext.CancellationToken; - var task = (Task)handleMethod.Invoke(handler, [command, cancellationToken])!; - return task; + + try + { + return (Task)handleMethod.Invoke(handler, [command, cancellationToken])!; + } + catch (TargetInvocationException ex) when (ex.InnerException is not null) + { + throw ex.InnerException; + } } } \ No newline at end of file diff --git a/src/common/Elsa.Mediator/Middleware/Command/Components/CommandHandlerInvokerMiddleware.cs b/src/common/Elsa.Mediator/Middleware/Command/Components/CommandHandlerInvokerMiddleware.cs index 645181b95c..c8d1fb0567 100644 --- a/src/common/Elsa.Mediator/Middleware/Command/Components/CommandHandlerInvokerMiddleware.cs +++ b/src/common/Elsa.Mediator/Middleware/Command/Components/CommandHandlerInvokerMiddleware.cs @@ -39,9 +39,10 @@ public async ValueTask InvokeAsync(CommandContext context) var executeMethodWithReturnType = executeMethod.MakeGenericMethod(resultType); // Execute command. - var task = executeMethodWithReturnType.Invoke(strategy, [strategyContext]); + var task = (Task)executeMethodWithReturnType.Invoke(strategy, [strategyContext])!; + await task; - // Get the result of the task. + // Get the result of the completed task. var taskWithReturnType = typeof(Task<>).MakeGenericType(resultType); var resultProperty = taskWithReturnType.GetProperty(nameof(Task.Result))!; context.Result = resultProperty.GetValue(task); diff --git a/test/unit/Elsa.Mediator.UnitTests/CommandCancellationBehaviorTests.cs b/test/unit/Elsa.Mediator.UnitTests/CommandCancellationBehaviorTests.cs new file mode 100644 index 0000000000..69a2dfc64a --- /dev/null +++ b/test/unit/Elsa.Mediator.UnitTests/CommandCancellationBehaviorTests.cs @@ -0,0 +1,131 @@ +using Elsa.Mediator.Contracts; +using Elsa.Mediator.Models; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; + +namespace Elsa.Mediator.UnitTests; + +public class CommandCancellationBehaviorTests +{ + [Fact] + public async Task SendAsync_WithSuccessfulCommand_ReturnsResult() + { + // Arrange + var commandSender = CreateCommandSender(); + + // Act + var result = await commandSender.SendAsync(new EchoCommand("Hello")); + + // Assert + Assert.Equal("Hello", result); + } + + [Fact] + public async Task SendAsync_WithCancelledToken_ThrowsOperationCanceledException() + { + // Arrange + var commandSender = CreateCommandSender(); + using var cts = new CancellationTokenSource(); + cts.Cancel(); + + // Act & Assert + await Assert.ThrowsAnyAsync( + () => commandSender.SendAsync(new SlowCommand(), cts.Token)); + } + + [Fact] + public async Task SendAsync_WithTimeout_ThrowsOperationCanceledException() + { + // Arrange + var commandSender = CreateCommandSender(); + using var cts = new CancellationTokenSource(TimeSpan.FromMilliseconds(50)); + + // Act & Assert + await Assert.ThrowsAnyAsync( + () => commandSender.SendAsync(new SlowCommand(), cts.Token)); + } + + [Fact] + public async Task SendAsync_WithSelfCancellingHandler_ThrowsTaskCanceledException() + { + // Arrange + var commandSender = CreateCommandSender(); + using var cts = new CancellationTokenSource(); + + // Act & Assert + await Assert.ThrowsAnyAsync( + () => commandSender.SendAsync(new SelfCancellingCommand(cts))); + } + + [Fact] + public async Task SendAsync_WithFailingHandler_ThrowsOriginalException() + { + // Arrange + var commandSender = CreateCommandSender(); + + // Act & Assert + var ex = await Assert.ThrowsAsync( + () => commandSender.SendAsync(new FailingCommand("Test error"))); + + Assert.Equal("Test error", ex.Message); + } + + #region Helpers + + private static ICommandSender CreateCommandSender() where THandler : class, ICommandHandler + { + var services = new ServiceCollection(); + services.AddLogging(b => b.SetMinimumLevel(LogLevel.Warning)); + services.AddMediator(); + services.AddCommandHandler(); + + var provider = services.BuildServiceProvider(); + return provider.CreateScope().ServiceProvider.GetRequiredService(); + } + + #endregion + + #region Test Commands + + public record EchoCommand(string Message) : ICommand; + public record SlowCommand : ICommand; + public record SelfCancellingCommand(CancellationTokenSource Cts) : ICommand; + public record FailingCommand(string ErrorMessage) : ICommand; + + #endregion + + #region Test Handlers + + public class EchoCommandHandler : ICommandHandler + { + public Task HandleAsync(EchoCommand command, CancellationToken cancellationToken) + => Task.FromResult(command.Message); + } + + public class SlowCommandHandler : ICommandHandler + { + public async Task HandleAsync(SlowCommand command, CancellationToken cancellationToken) + { + await Task.Delay(TimeSpan.FromSeconds(10), cancellationToken); + return Unit.Instance; + } + } + + public class SelfCancellingCommandHandler : ICommandHandler + { + public async Task HandleAsync(SelfCancellingCommand command, CancellationToken cancellationToken) + { + await command.Cts.CancelAsync(); + await Task.Delay(1000, command.Cts.Token); + return Unit.Instance; + } + } + + public class FailingCommandHandler : ICommandHandler + { + public Task HandleAsync(FailingCommand command, CancellationToken cancellationToken) + => throw new InvalidOperationException(command.ErrorMessage); + } + + #endregion +} diff --git a/test/unit/Elsa.Mediator.UnitTests/Elsa.Mediator.UnitTests.csproj b/test/unit/Elsa.Mediator.UnitTests/Elsa.Mediator.UnitTests.csproj new file mode 100644 index 0000000000..6c549b588d --- /dev/null +++ b/test/unit/Elsa.Mediator.UnitTests/Elsa.Mediator.UnitTests.csproj @@ -0,0 +1,11 @@ + + + + [Elsa.Mediator]* + + + + + + + From 9583a9e12283c63ae474044ba68071d73f10b26c Mon Sep 17 00:00:00 2001 From: Avinesh Singh Date: Mon, 9 Feb 2026 13:23:00 +0530 Subject: [PATCH 2/4] Address review comments Use ExceptionDispatchInfo to prevent misleading stacktrace. As well use disposable fixture to actually dispose CommandSender, --- .../Extensions/HandlerExtensions.cs | 7 +++- .../CommandCancellationBehaviorTests.cs | 40 ++++++++++++------- 2 files changed, 31 insertions(+), 16 deletions(-) diff --git a/src/common/Elsa.Mediator/Extensions/HandlerExtensions.cs b/src/common/Elsa.Mediator/Extensions/HandlerExtensions.cs index 17204949d1..4a414b2595 100644 --- a/src/common/Elsa.Mediator/Extensions/HandlerExtensions.cs +++ b/src/common/Elsa.Mediator/Extensions/HandlerExtensions.cs @@ -1,5 +1,6 @@ using System.Diagnostics.CodeAnalysis; using System.Reflection; +using System.Runtime.ExceptionServices; using Elsa.Mediator.Contracts; using Elsa.Mediator.Middleware.Command; using Elsa.Mediator.Middleware.Notification; @@ -54,7 +55,8 @@ public static Task InvokeAsync(this INotificationHandler handler, MethodBase han } catch (TargetInvocationException ex) when (ex.InnerException is not null) { - throw ex.InnerException; + ExceptionDispatchInfo.Capture(ex.InnerException).Throw(); + throw; // Unreachable, but required for compiler } } @@ -75,7 +77,8 @@ public static Task InvokeAsync(this ICommandHandler handler, M } catch (TargetInvocationException ex) when (ex.InnerException is not null) { - throw ex.InnerException; + ExceptionDispatchInfo.Capture(ex.InnerException).Throw(); + throw; // Unreachable, but required for compiler } } } \ No newline at end of file diff --git a/test/unit/Elsa.Mediator.UnitTests/CommandCancellationBehaviorTests.cs b/test/unit/Elsa.Mediator.UnitTests/CommandCancellationBehaviorTests.cs index 69a2dfc64a..6f347aac19 100644 --- a/test/unit/Elsa.Mediator.UnitTests/CommandCancellationBehaviorTests.cs +++ b/test/unit/Elsa.Mediator.UnitTests/CommandCancellationBehaviorTests.cs @@ -11,10 +11,10 @@ public class CommandCancellationBehaviorTests public async Task SendAsync_WithSuccessfulCommand_ReturnsResult() { // Arrange - var commandSender = CreateCommandSender(); + using var fixture = CreateCommandSender(); // Act - var result = await commandSender.SendAsync(new EchoCommand("Hello")); + var result = await fixture.CommandSender.SendAsync(new EchoCommand("Hello")); // Assert Assert.Equal("Hello", result); @@ -24,55 +24,55 @@ public async Task SendAsync_WithSuccessfulCommand_ReturnsResult() public async Task SendAsync_WithCancelledToken_ThrowsOperationCanceledException() { // Arrange - var commandSender = CreateCommandSender(); + using var fixture = CreateCommandSender(); using var cts = new CancellationTokenSource(); cts.Cancel(); // Act & Assert await Assert.ThrowsAnyAsync( - () => commandSender.SendAsync(new SlowCommand(), cts.Token)); + () => fixture.CommandSender.SendAsync(new SlowCommand(), cts.Token)); } [Fact] public async Task SendAsync_WithTimeout_ThrowsOperationCanceledException() { // Arrange - var commandSender = CreateCommandSender(); + using var fixture = CreateCommandSender(); using var cts = new CancellationTokenSource(TimeSpan.FromMilliseconds(50)); // Act & Assert await Assert.ThrowsAnyAsync( - () => commandSender.SendAsync(new SlowCommand(), cts.Token)); + () => fixture.CommandSender.SendAsync(new SlowCommand(), cts.Token)); } [Fact] public async Task SendAsync_WithSelfCancellingHandler_ThrowsTaskCanceledException() { // Arrange - var commandSender = CreateCommandSender(); + using var fixture = CreateCommandSender(); using var cts = new CancellationTokenSource(); // Act & Assert - await Assert.ThrowsAnyAsync( - () => commandSender.SendAsync(new SelfCancellingCommand(cts))); + await Assert.ThrowsAsync( + () => fixture.CommandSender.SendAsync(new SelfCancellingCommand(cts))); } [Fact] public async Task SendAsync_WithFailingHandler_ThrowsOriginalException() { // Arrange - var commandSender = CreateCommandSender(); + using var fixture = CreateCommandSender(); // Act & Assert var ex = await Assert.ThrowsAsync( - () => commandSender.SendAsync(new FailingCommand("Test error"))); + () => fixture.CommandSender.SendAsync(new FailingCommand("Test error"))); Assert.Equal("Test error", ex.Message); } #region Helpers - private static ICommandSender CreateCommandSender() where THandler : class, ICommandHandler + private static CommandSenderFixture CreateCommandSender() where THandler : class, ICommandHandler { var services = new ServiceCollection(); services.AddLogging(b => b.SetMinimumLevel(LogLevel.Warning)); @@ -80,7 +80,19 @@ private static ICommandSender CreateCommandSender() where THandler : c services.AddCommandHandler(); var provider = services.BuildServiceProvider(); - return provider.CreateScope().ServiceProvider.GetRequiredService(); + var scope = provider.CreateScope(); + return new CommandSenderFixture(provider, scope); + } + + private sealed class CommandSenderFixture(ServiceProvider provider, IServiceScope scope) : IDisposable + { + public ICommandSender CommandSender => scope.ServiceProvider.GetRequiredService(); + + public void Dispose() + { + scope.Dispose(); + provider.Dispose(); + } } #endregion @@ -106,7 +118,7 @@ public class SlowCommandHandler : ICommandHandler { public async Task HandleAsync(SlowCommand command, CancellationToken cancellationToken) { - await Task.Delay(TimeSpan.FromSeconds(10), cancellationToken); + await Task.Delay(TimeSpan.FromMilliseconds(500), cancellationToken); return Unit.Instance; } } From 7d571f9fc63640c8f19ef700e9a466e2c1bcf17a Mon Sep 17 00:00:00 2001 From: Avinesh Singh Date: Mon, 9 Feb 2026 14:24:39 +0530 Subject: [PATCH 3/4] Address review comments - Refactor duplicated unwrapping logic into shared InvokeAndUnwrap helper method - Simplify CommandHandlerInvokerMiddleware to be consistent with RequestHandlerInvokerMiddleware (remove redundant await) - Add Threshold property to test project for consistency --- .../Extensions/HandlerExtensions.cs | 20 +++++++++---------- .../CommandHandlerInvokerMiddleware.cs | 4 +--- .../Elsa.Mediator.UnitTests.csproj | 1 + 3 files changed, 11 insertions(+), 14 deletions(-) diff --git a/src/common/Elsa.Mediator/Extensions/HandlerExtensions.cs b/src/common/Elsa.Mediator/Extensions/HandlerExtensions.cs index 4a414b2595..044970500e 100644 --- a/src/common/Elsa.Mediator/Extensions/HandlerExtensions.cs +++ b/src/common/Elsa.Mediator/Extensions/HandlerExtensions.cs @@ -48,16 +48,7 @@ public static Task InvokeAsync(this INotificationHandler handler, MethodBase han { var notification = notificationContext.Notification; var cancellationToken = notificationContext.CancellationToken; - - try - { - return (Task)handleMethod.Invoke(handler, [notification, cancellationToken])!; - } - catch (TargetInvocationException ex) when (ex.InnerException is not null) - { - ExceptionDispatchInfo.Capture(ex.InnerException).Throw(); - throw; // Unreachable, but required for compiler - } + return InvokeAndUnwrap(handleMethod, handler, [notification, cancellationToken]); } /// @@ -70,10 +61,17 @@ public static Task InvokeAsync(this ICommandHandler handler, M { var command = commandContext.Command; var cancellationToken = commandContext.CancellationToken; + return InvokeAndUnwrap>(handleMethod, handler, [command, cancellationToken]); + } + /// + /// Invokes a method via reflection and unwraps any TargetInvocationException to preserve the original exception's stack trace. + /// + private static T InvokeAndUnwrap(MethodBase method, object target, object[] args) where T : Task + { try { - return (Task)handleMethod.Invoke(handler, [command, cancellationToken])!; + return (T)method.Invoke(target, args)!; } catch (TargetInvocationException ex) when (ex.InnerException is not null) { diff --git a/src/common/Elsa.Mediator/Middleware/Command/Components/CommandHandlerInvokerMiddleware.cs b/src/common/Elsa.Mediator/Middleware/Command/Components/CommandHandlerInvokerMiddleware.cs index 092ae361b3..f36b6995dc 100644 --- a/src/common/Elsa.Mediator/Middleware/Command/Components/CommandHandlerInvokerMiddleware.cs +++ b/src/common/Elsa.Mediator/Middleware/Command/Components/CommandHandlerInvokerMiddleware.cs @@ -42,10 +42,8 @@ public async ValueTask InvokeAsync(CommandContext context) var task = (Task)executeMethodWithReturnType.Invoke(strategy, [strategyContext])!; await task; - // Await the task to get the result without blocking. + // Get result of task. var taskWithReturnType = typeof(Task<>).MakeGenericType(resultType); - var taskInstance = (Task)task!; - await taskInstance.ConfigureAwait(false); var resultProperty = taskWithReturnType.GetProperty(nameof(Task.Result))!; context.Result = resultProperty.GetValue(task); diff --git a/test/unit/Elsa.Mediator.UnitTests/Elsa.Mediator.UnitTests.csproj b/test/unit/Elsa.Mediator.UnitTests/Elsa.Mediator.UnitTests.csproj index 6c549b588d..431a924caa 100644 --- a/test/unit/Elsa.Mediator.UnitTests/Elsa.Mediator.UnitTests.csproj +++ b/test/unit/Elsa.Mediator.UnitTests/Elsa.Mediator.UnitTests.csproj @@ -2,6 +2,7 @@ [Elsa.Mediator]* + 0 From 9fa9cac75407704683289fec6ad2e9564b6493cc Mon Sep 17 00:00:00 2001 From: Avinesh Singh Date: Mon, 9 Feb 2026 14:44:06 +0530 Subject: [PATCH 4/4] add ConfigureAwait(false) consistently to middlewares - Add ConfigureAwait(false) to await statements in both CommandHandlerInvokerMiddleware and RequestHandlerInvokerMiddleware for consistency and to follow library code best practices - Increase test timeout from 50ms to 100ms for better CI reliability while still maintaining adequate margin (handler has 500ms delay) --- .../Command/Components/CommandHandlerInvokerMiddleware.cs | 4 ++-- .../Request/Components/RequestHandlerInvokerMiddleware.cs | 4 ++-- .../CommandCancellationBehaviorTests.cs | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/common/Elsa.Mediator/Middleware/Command/Components/CommandHandlerInvokerMiddleware.cs b/src/common/Elsa.Mediator/Middleware/Command/Components/CommandHandlerInvokerMiddleware.cs index f36b6995dc..96afdc9a72 100644 --- a/src/common/Elsa.Mediator/Middleware/Command/Components/CommandHandlerInvokerMiddleware.cs +++ b/src/common/Elsa.Mediator/Middleware/Command/Components/CommandHandlerInvokerMiddleware.cs @@ -40,7 +40,7 @@ public async ValueTask InvokeAsync(CommandContext context) // Execute command. var task = (Task)executeMethodWithReturnType.Invoke(strategy, [strategyContext])!; - await task; + await task.ConfigureAwait(false); // Get result of task. var taskWithReturnType = typeof(Task<>).MakeGenericType(resultType); @@ -48,6 +48,6 @@ public async ValueTask InvokeAsync(CommandContext context) context.Result = resultProperty.GetValue(task); // Invoke next middleware. - await next(context); + await next(context).ConfigureAwait(false); } } \ No newline at end of file diff --git a/src/common/Elsa.Mediator/Middleware/Request/Components/RequestHandlerInvokerMiddleware.cs b/src/common/Elsa.Mediator/Middleware/Request/Components/RequestHandlerInvokerMiddleware.cs index 2055515f11..b3d47e17c4 100644 --- a/src/common/Elsa.Mediator/Middleware/Request/Components/RequestHandlerInvokerMiddleware.cs +++ b/src/common/Elsa.Mediator/Middleware/Request/Components/RequestHandlerInvokerMiddleware.cs @@ -31,7 +31,7 @@ public async ValueTask InvokeAsync(RequestContext context) var handleMethod = handlerType.GetMethod("HandleAsync")!; var cancellationToken = context.CancellationToken; var task = (Task)handleMethod.Invoke(handler, [request, cancellationToken])!; - await task; + await task.ConfigureAwait(false); // Get result of task. var taskWithReturnType = typeof(Task<>).MakeGenericType(responseType); @@ -39,6 +39,6 @@ public async ValueTask InvokeAsync(RequestContext context) context.Response = resultProperty.GetValue(task)!; // Invoke next middleware. - await next(context); + await next(context).ConfigureAwait(false); } } \ No newline at end of file diff --git a/test/unit/Elsa.Mediator.UnitTests/CommandCancellationBehaviorTests.cs b/test/unit/Elsa.Mediator.UnitTests/CommandCancellationBehaviorTests.cs index 6f347aac19..76e8fa047d 100644 --- a/test/unit/Elsa.Mediator.UnitTests/CommandCancellationBehaviorTests.cs +++ b/test/unit/Elsa.Mediator.UnitTests/CommandCancellationBehaviorTests.cs @@ -38,7 +38,7 @@ public async Task SendAsync_WithTimeout_ThrowsOperationCanceledException() { // Arrange using var fixture = CreateCommandSender(); - using var cts = new CancellationTokenSource(TimeSpan.FromMilliseconds(50)); + using var cts = new CancellationTokenSource(TimeSpan.FromMilliseconds(100)); // Act & Assert await Assert.ThrowsAnyAsync(