fix(application): log swallowed handler exceptions before wrapping in Result.Failure - #122
Merged
Merged
Conversation
… Result.Failure
P0-02: CommandDispatcher and QueryDispatcher converted any exception thrown
in a handler into Result.Failure(Error.Failure("*.ExecutionFailed", ex.Message))
without logging the exception or stack trace. Production debugging was blind —
the only trace was on OTel spans (exception.type/exception.message tags), which
are not always sampled or retained.
Changes:
- Both dispatchers now resolve ILogger<T> from the existing IServiceProvider
(NullLogger<T> fallback) so the public single-arg constructor stays binary-
and source-compatible for downstream consumers.
- Before wrapping the exception into Result.Failure, log it at Error level with
the full exception (stack trace) and the command/query type name.
- Enrich the failure Error with non-breaking metadata { exceptionType } so
consumers can discriminate the underlying cause without parsing the message.
- Result-pattern contract preserved: still returns Result.Failure, never rethrows.
Tests: handler-throws paths now assert Result.Failure is returned AND the logger
received the exception at Error level AND the exceptionType metadata is present,
plus a NullLogger fallback test and success-path no-error-log assertions.
Ref: Nexus bug inventory P0-02; memory project_compendium_dispatcher_silent
There was a problem hiding this comment.
Pull request overview
This PR addresses P0-02 by ensuring exceptions thrown by CQRS command/query handlers are no longer silently swallowed: the dispatchers now log the exception (including stack trace) before converting it into a Result.Failure, and they enrich the failure Error with exceptionType metadata without breaking public APIs.
Changes:
- Add
ILogger<T>resolution (withNullLogger<T>fallback) toCommandDispatcherandQueryDispatcherwithout changing their public constructors. - Log swallowed handler exceptions at
Errorlevel before returningResult.Failure(...), and attach{ exceptionType }metadata to the failure error. - Add unit tests (and a lightweight
CapturingLogger<T>) to verify logging + metadata enrichment + null-logger fallback behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/Application/Compendium.Application/CQRS/CommandDispatcher.cs | Resolve logger via IServiceProvider, log swallowed handler exceptions, and add exceptionType metadata on execution-failure results. |
| src/Application/Compendium.Application/CQRS/QueryDispatcher.cs | Same as command dispatcher: structured error log before wrapping, plus exceptionType metadata enrichment. |
| tests/Unit/Compendium.Application.Tests/CQRS/CapturingLogger.cs | Adds minimal in-memory ILogger<T> implementation for asserting logged entries without extra dependencies. |
| tests/Unit/Compendium.Application.Tests/CQRS/CommandDispatcherTests.cs | Adds coverage for logging-on-throw, null-logger fallback, and “no error logs on success” for command dispatch paths. |
| tests/Unit/Compendium.Application.Tests/CQRS/QueryDispatcherTests.cs | Adds coverage for logging-on-throw, null-logger fallback, and “no error logs on success” for query dispatch path. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This was referenced Jun 15, 2026
Open
Open
Open
Open
Open
Open
Open
Closed
Open
Closed
Closed
Open
Open
This was referenced Jun 22, 2026
Open
Open
Open
Open
Open
Open
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
P0-02 — Dispatcher swallows handler exceptions silently
The Compendium command/query dispatchers convert any exception thrown inside a handler into
Result.Failure(Error.Failure("*.ExecutionFailed", ex.Message))without logging the exception orstack trace. In production this makes debugging blind: the only trace is on OTel spans
(
exception.type/exception.messageactivity tags), which are not always sampled or retained.Refs:
project_compendium_dispatcher_silentProblemDetailsLoggingFilterthat only partially mitigated this (REST surface only — MCP, SDK, background/process-manager dispatch paths still went dark).What changed
Three swallow points fixed (
CommandDispatcher.DispatchAsync<TCommand>,CommandDispatcher.DispatchAsync<TCommand,TResult>,QueryDispatcher.DispatchAsync<TQuery,TResult>):_logger.LogError(ex, "...", commandOrQueryType, ...)— logging the full exception (with stacktrace) at
Errorlevel before building the failureResult.IServiceProvideronly. Rather thanchange the public single-argument constructor (which downstream consumers call directly — see the
existing unit tests), the logger is resolved from the service provider with a
NullLogger<T>fallback when no logging is configured. Binary- and source-compatible.Error.Failure(...)already supports an optionalmetadatadictionary, so the failure error now carries
{ exceptionType }— letting consumers discriminatethe underlying cause without string-parsing
Error.Message.Result.Failure, never rethrows.The pre-existing
LoggingBehavior<TRequest,TResponse>only logs when registered as a pipelinebehavior and rethrows; the dispatcher catch sits outside the behavior pipeline and is the final,
unconditional swallow point — which is exactly what this fixes.
Tests
Added to
CommandDispatcherTests/QueryDispatcherTests(+ a tinyCapturingLogger<T>helper toavoid a new
Microsoft.Extensions.Diagnostics.Testingdependency):Result.Failurereturned AND logger received the exception atErrorlevelAND
exceptionTypemetadata present (command no-result, command-with-result, and query paths)ILoggerregistered → falls back toNullLogger, still returns failure, does not throwError-level log emitteddotnet test Compendium.Application.Tests→ 232 passed. Architecture tests → 37 passed.Notes
[Obsolete]SagaOrchestrator(slated for removal in v1.0) has similar catch-and-wrapblocks but is deprecated dead-path; intentionally out of scope for this focused P0 fix. The live
choreography path (
ChoreographyRouter) already aggregates handler errors with messages.