diff --git a/src/Application/Compendium.Application/CQRS/CommandDispatcher.cs b/src/Application/Compendium.Application/CQRS/CommandDispatcher.cs index 1f5abfc..ec41a26 100644 --- a/src/Application/Compendium.Application/CQRS/CommandDispatcher.cs +++ b/src/Application/Compendium.Application/CQRS/CommandDispatcher.cs @@ -2,6 +2,8 @@ using Compendium.Application.CQRS.Behaviors; using Compendium.Core.Telemetry; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; namespace Compendium.Application.CQRS; @@ -40,15 +42,23 @@ Task> DispatchAsync(TCommand command, Cancell public sealed class CommandDispatcher : ICommandDispatcher { private readonly IServiceProvider _serviceProvider; + private readonly ILogger _logger; /// /// Initializes a new instance of the class. /// /// The service provider for resolving handlers and behaviors. /// Thrown when serviceProvider is null. + /// + /// P0-02: The logger is resolved from rather than injected + /// directly so that the existing public single-parameter constructor remains binary- and + /// source-compatible for downstream consumers. When the container has no logging configured + /// (e.g. in lightweight unit tests) a is used as a safe fallback. + /// public CommandDispatcher(IServiceProvider serviceProvider) { _serviceProvider = serviceProvider ?? throw new ArgumentNullException(nameof(serviceProvider)); + _logger = _serviceProvider.GetService>() ?? NullLogger.Instance; } /// @@ -139,7 +149,20 @@ public async Task DispatchAsync(TCommand command, Cancellation activity?.SetTag("exception.type", ex.GetType().FullName); activity?.SetTag("exception.message", ex.Message); - return Result.Failure(Error.Failure("Command.ExecutionFailed", ex.Message)); + // P0-02: Log the swallowed exception with stack trace BEFORE wrapping it into a + // Result.Failure. Without this, production debugging is blind — the exception is + // converted to a failure Result and the only trace is on OTel spans. + _logger.LogError( + ex, + "Command handler for {CommandType} threw an unhandled exception after {ElapsedMs}ms; converting to Result.Failure({ErrorCode})", + typeof(TCommand).Name, + sw.Elapsed.TotalMilliseconds, + "Command.ExecutionFailed"); + + return Result.Failure(Error.Failure( + "Command.ExecutionFailed", + ex.Message, + BuildExceptionMetadata(ex))); } } @@ -232,10 +255,39 @@ public async Task> DispatchAsync(TCommand com activity?.SetTag("exception.type", ex.GetType().FullName); activity?.SetTag("exception.message", ex.Message); - return Result.Failure(Error.Failure("Command.ExecutionFailed", ex.Message)); + // P0-02: Log the swallowed exception with stack trace BEFORE wrapping it into a + // Result.Failure. Without this, production debugging is blind — the exception is + // converted to a failure Result and the only trace is on OTel spans. + _logger.LogError( + ex, + "Command handler for {CommandType} returning {ResultType} threw an unhandled exception after {ElapsedMs}ms; converting to Result.Failure({ErrorCode})", + typeof(TCommand).Name, + typeof(TResult).Name, + sw.Elapsed.TotalMilliseconds, + "Command.ExecutionFailed"); + + return Result.Failure(Error.Failure( + "Command.ExecutionFailed", + ex.Message, + BuildExceptionMetadata(ex))); } } + /// + /// Builds the error metadata dictionary attached to an execution-failure error, capturing the + /// exception type so downstream consumers can discriminate the underlying failure cause without + /// parsing the message string. P0-02. + /// + /// The exception that was thrown by the handler pipeline. + /// A read-only metadata dictionary containing the exception type name. + private static IReadOnlyDictionary BuildExceptionMetadata(Exception exception) + { + return new Dictionary + { + ["exceptionType"] = exception.GetType().FullName ?? exception.GetType().Name, + }; + } + /// /// Executes the command handler for commands that don't return a value. /// diff --git a/src/Application/Compendium.Application/CQRS/QueryDispatcher.cs b/src/Application/Compendium.Application/CQRS/QueryDispatcher.cs index c2353ce..f35768a 100644 --- a/src/Application/Compendium.Application/CQRS/QueryDispatcher.cs +++ b/src/Application/Compendium.Application/CQRS/QueryDispatcher.cs @@ -1,6 +1,8 @@ using System.Diagnostics; using Compendium.Core.Telemetry; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; namespace Compendium.Application.CQRS; @@ -29,15 +31,23 @@ Task> DispatchAsync(TQuery query, CancellationT public sealed class QueryDispatcher : IQueryDispatcher { private readonly IServiceProvider _serviceProvider; + private readonly ILogger _logger; /// /// Initializes a new instance of the class. /// /// The service provider for resolving handlers. /// Thrown when serviceProvider is null. + /// + /// P0-02: The logger is resolved from rather than injected + /// directly so that the existing public single-parameter constructor remains binary- and + /// source-compatible for downstream consumers. When the container has no logging configured + /// (e.g. in lightweight unit tests) a is used as a safe fallback. + /// public QueryDispatcher(IServiceProvider serviceProvider) { _serviceProvider = serviceProvider ?? throw new ArgumentNullException(nameof(serviceProvider)); + _logger = _serviceProvider.GetService>() ?? NullLogger.Instance; } /// @@ -116,7 +126,36 @@ public async Task> DispatchAsync(TQuery query, activity?.SetTag("exception.type", ex.GetType().FullName); activity?.SetTag("exception.message", ex.Message); - return Result.Failure(Error.Failure("Query.ExecutionFailed", ex.Message)); + // P0-02: Log the swallowed exception with stack trace BEFORE wrapping it into a + // Result.Failure. Without this, production debugging is blind — the exception is + // converted to a failure Result and the only trace is on OTel spans. + _logger.LogError( + ex, + "Query handler for {QueryType} returning {ResultType} threw an unhandled exception after {ElapsedMs}ms; converting to Result.Failure({ErrorCode})", + typeof(TQuery).Name, + typeof(TResult).Name, + sw.Elapsed.TotalMilliseconds, + "Query.ExecutionFailed"); + + return Result.Failure(Error.Failure( + "Query.ExecutionFailed", + ex.Message, + BuildExceptionMetadata(ex))); } } + + /// + /// Builds the error metadata dictionary attached to an execution-failure error, capturing the + /// exception type so downstream consumers can discriminate the underlying failure cause without + /// parsing the message string. P0-02. + /// + /// The exception that was thrown by the handler. + /// A read-only metadata dictionary containing the exception type name. + private static IReadOnlyDictionary BuildExceptionMetadata(Exception exception) + { + return new Dictionary + { + ["exceptionType"] = exception.GetType().FullName ?? exception.GetType().Name, + }; + } } diff --git a/tests/Unit/Compendium.Application.Tests/CQRS/CapturingLogger.cs b/tests/Unit/Compendium.Application.Tests/CQRS/CapturingLogger.cs new file mode 100644 index 0000000..e8b2b93 --- /dev/null +++ b/tests/Unit/Compendium.Application.Tests/CQRS/CapturingLogger.cs @@ -0,0 +1,63 @@ +// ----------------------------------------------------------------------- +// +// Copyright (c) 2026 Sassy Solutions. Licensed under the MIT License. +// See LICENSE in the project root for license information. +// +// ----------------------------------------------------------------------- + +using Microsoft.Extensions.Logging; + +namespace Compendium.Application.Tests.CQRS; + +/// +/// A minimal in-memory implementation that records every log entry it +/// receives. Used to assert that the CQRS dispatchers log swallowed handler exceptions (P0-02) +/// before wrapping them into a failure. Avoids taking a dependency on +/// Microsoft.Extensions.Diagnostics.Testing just for the FakeLogger type. +/// +/// The category type for the logger. +public sealed class CapturingLogger : ILogger +{ + private readonly List _entries = []; + + /// + /// Gets the captured log entries in the order they were recorded. + /// + public IReadOnlyList Entries => _entries; + + /// + public IDisposable BeginScope(TState state) + where TState : notnull => NullScope.Instance; + + /// + public bool IsEnabled(LogLevel logLevel) => true; + + /// + public void Log( + LogLevel logLevel, + EventId eventId, + TState state, + Exception? exception, + Func formatter) + { + _entries.Add(new CapturedLogEntry(logLevel, exception, formatter(state, exception))); + } + + /// + /// Represents a single captured log entry. + /// + /// The log level. + /// The exception associated with the entry, if any. + /// The rendered log message. + public sealed record CapturedLogEntry(LogLevel Level, Exception? Exception, string Message); + + private sealed class NullScope : IDisposable + { + public static readonly NullScope Instance = new(); + + public void Dispose() + { + // No-op. + } + } +} diff --git a/tests/Unit/Compendium.Application.Tests/CQRS/CommandDispatcherTests.cs b/tests/Unit/Compendium.Application.Tests/CQRS/CommandDispatcherTests.cs index 341cd87..649e15e 100644 --- a/tests/Unit/Compendium.Application.Tests/CQRS/CommandDispatcherTests.cs +++ b/tests/Unit/Compendium.Application.Tests/CQRS/CommandDispatcherTests.cs @@ -8,6 +8,7 @@ using Compendium.Application.CQRS; using Compendium.Application.CQRS.Behaviors; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; namespace Compendium.Application.Tests.CQRS; @@ -133,6 +134,116 @@ public async Task DispatchAsync_WhenHandlerThrows_ReturnsExecutionFailedFailure( result.Error.Message.Should().Contain("kaboom"); } + [Fact] + public async Task DispatchAsync_WhenHandlerThrows_LogsExceptionBeforeWrappingInFailure() + { + // Arrange + var thrown = new InvalidOperationException("kaboom"); + var handler = Substitute.For>(); + handler.HandleAsync(Arg.Any(), Arg.Any()) + .Returns>(_ => throw thrown); + + var logger = new CapturingLogger(); + var sp = new ServiceCollection() + .AddSingleton(handler) + .AddSingleton>(logger) + .BuildServiceProvider(); + var dispatcher = new CommandDispatcher(sp); + + // Act + var result = await dispatcher.DispatchAsync(new TestCommand(), CancellationToken.None); + + // Assert — the Result contract is preserved (P0-02: never rethrow) + result.IsFailure.Should().BeTrue(); + result.Error.Code.Should().Be("Command.ExecutionFailed"); + + // Assert — the exception was logged with its stack trace, not silently swallowed + var errorEntry = logger.Entries.Should().ContainSingle(e => e.Level == LogLevel.Error).Subject; + errorEntry.Exception.Should().BeSameAs(thrown); + errorEntry.Message.Should().Contain(nameof(TestCommand)); + + // Assert — the exception type is preserved in the error metadata (non-breaking enrichment) + result.Error.Metadata.Should().ContainKey("exceptionType"); + result.Error.Metadata["exceptionType"].Should().Be(typeof(InvalidOperationException).FullName); + } + + [Fact] + public async Task DispatchAsync_WhenNoLoggerRegistered_StillReturnsFailureWithoutThrowing() + { + // Arrange — no ILogger registered; dispatcher must fall back to NullLogger and not throw + var handler = Substitute.For>(); + handler.HandleAsync(Arg.Any(), Arg.Any()) + .Returns>(_ => throw new InvalidOperationException("kaboom")); + + var sp = new ServiceCollection() + .AddSingleton(handler) + .BuildServiceProvider(); + var dispatcher = new CommandDispatcher(sp); + + // Act + var result = await dispatcher.DispatchAsync(new TestCommand(), CancellationToken.None); + + // Assert + result.IsFailure.Should().BeTrue(); + result.Error.Code.Should().Be("Command.ExecutionFailed"); + } + + [Fact] + public async Task DispatchAsyncTResult_WhenHandlerThrows_LogsExceptionBeforeWrappingInFailure() + { + // Arrange + var thrown = new InvalidOperationException("oops"); + var handler = Substitute.For>(); + handler.HandleAsync(Arg.Any(), Arg.Any()) + .Returns>>(_ => throw thrown); + + var logger = new CapturingLogger(); + var sp = new ServiceCollection() + .AddSingleton(handler) + .AddSingleton>(logger) + .BuildServiceProvider(); + var dispatcher = new CommandDispatcher(sp); + + // Act + var result = await dispatcher.DispatchAsync( + new TestCommandWithResult { Value = 1 }, + CancellationToken.None); + + // Assert + result.IsFailure.Should().BeTrue(); + result.Error.Code.Should().Be("Command.ExecutionFailed"); + + var errorEntry = logger.Entries.Should().ContainSingle(e => e.Level == LogLevel.Error).Subject; + errorEntry.Exception.Should().BeSameAs(thrown); + errorEntry.Message.Should().Contain(nameof(TestCommandWithResult)); + + result.Error.Metadata.Should().ContainKey("exceptionType"); + result.Error.Metadata["exceptionType"].Should().Be(typeof(InvalidOperationException).FullName); + } + + [Fact] + public async Task DispatchAsync_WhenHandlerSucceeds_DoesNotLogError() + { + // Arrange + var handler = Substitute.For>(); + handler.HandleAsync(Arg.Any(), Arg.Any()) + .Returns(Task.FromResult(Result.Success())); + + var logger = new CapturingLogger(); + var sp = new ServiceCollection() + .AddSingleton(handler) + .AddSingleton>(logger) + .BuildServiceProvider(); + var dispatcher = new CommandDispatcher(sp); + + // Act + var result = await dispatcher.DispatchAsync(new TestCommand(), CancellationToken.None); + + // Assert + result.IsSuccess.Should().BeTrue(); + logger.Entries.Should().NotContain(e => e.Level == LogLevel.Error); + } + [Fact] public async Task DispatchAsync_WhenBehaviorsRegistered_InvokesPipelineInRegistrationOrder() { diff --git a/tests/Unit/Compendium.Application.Tests/CQRS/QueryDispatcherTests.cs b/tests/Unit/Compendium.Application.Tests/CQRS/QueryDispatcherTests.cs index e15c4bd..765cd51 100644 --- a/tests/Unit/Compendium.Application.Tests/CQRS/QueryDispatcherTests.cs +++ b/tests/Unit/Compendium.Application.Tests/CQRS/QueryDispatcherTests.cs @@ -7,6 +7,7 @@ using Compendium.Application.CQRS; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; namespace Compendium.Application.Tests.CQRS; @@ -134,4 +135,87 @@ public async Task DispatchAsync_WhenHandlerThrows_ReturnsExecutionFailedFailure( result.Error.Code.Should().Be("Query.ExecutionFailed"); result.Error.Message.Should().Contain("query bad"); } + + [Fact] + public async Task DispatchAsync_WhenHandlerThrows_LogsExceptionBeforeWrappingInFailure() + { + // Arrange + var thrown = new InvalidOperationException("query bad"); + var handler = Substitute.For>(); + handler.HandleAsync(Arg.Any(), Arg.Any()) + .Returns>>(_ => throw thrown); + + var logger = new CapturingLogger(); + var sp = new ServiceCollection() + .AddSingleton(handler) + .AddSingleton>(logger) + .BuildServiceProvider(); + var dispatcher = new QueryDispatcher(sp); + + // Act + var result = await dispatcher.DispatchAsync( + new TestQuery(), + CancellationToken.None); + + // Assert — the Result contract is preserved (P0-02: never rethrow) + result.IsFailure.Should().BeTrue(); + result.Error.Code.Should().Be("Query.ExecutionFailed"); + + // Assert — the exception was logged with its stack trace, not silently swallowed + var errorEntry = logger.Entries.Should().ContainSingle(e => e.Level == LogLevel.Error).Subject; + errorEntry.Exception.Should().BeSameAs(thrown); + errorEntry.Message.Should().Contain(nameof(TestQuery)); + + // Assert — the exception type is preserved in the error metadata (non-breaking enrichment) + result.Error.Metadata.Should().ContainKey("exceptionType"); + result.Error.Metadata["exceptionType"].Should().Be(typeof(InvalidOperationException).FullName); + } + + [Fact] + public async Task DispatchAsync_WhenNoLoggerRegistered_StillReturnsFailureWithoutThrowing() + { + // Arrange — no ILogger registered; dispatcher must fall back to NullLogger and not throw + var handler = Substitute.For>(); + handler.HandleAsync(Arg.Any(), Arg.Any()) + .Returns>>(_ => throw new InvalidOperationException("query bad")); + + var sp = new ServiceCollection() + .AddSingleton(handler) + .BuildServiceProvider(); + var dispatcher = new QueryDispatcher(sp); + + // Act + var result = await dispatcher.DispatchAsync( + new TestQuery(), + CancellationToken.None); + + // Assert + result.IsFailure.Should().BeTrue(); + result.Error.Code.Should().Be("Query.ExecutionFailed"); + } + + [Fact] + public async Task DispatchAsync_WhenHandlerSucceeds_DoesNotLogError() + { + // Arrange + var handler = Substitute.For>(); + handler.HandleAsync(Arg.Any(), Arg.Any()) + .Returns(Task.FromResult(Result.Success("hello"))); + + var logger = new CapturingLogger(); + var sp = new ServiceCollection() + .AddSingleton(handler) + .AddSingleton>(logger) + .BuildServiceProvider(); + var dispatcher = new QueryDispatcher(sp); + + // Act + var result = await dispatcher.DispatchAsync( + new TestQuery(), + CancellationToken.None); + + // Assert + result.IsSuccess.Should().BeTrue(); + logger.Entries.Should().NotContain(e => e.Level == LogLevel.Error); + } }