elsa-workflows / elsa-workflows/elsa-foundation
[Tech debt] Mediator/Events: pipeline-ordering bug, discarded parallel-handler exceptions, and builder duplication
- Dominant language
- C#
- Stars
- 5
- Forks
- 1
- Avg merge
- 3h 52m
- Merged PRs (30d)
- 212
Description
**Severity: medium | Ease: easy-medium** — batched findings from `src/Elsa/Mediator/**` and `src/Elsa/Events/**`.
1. **`CommandPipeline.cs:19`:** middleware registration order is backwards — `CommandLoggingMiddleware` is registered after `CommandHandlerInvokerMiddleware`, so "Invoking {CommandName}" is logged only *after* the handler has already fully executed, making timing-based diagnostics misleading.
2. **`ParallelProcessingStrategy.cs:8-15`:** `Task.WhenAll(tasks)` is awaited directly, so when multiple handlers throw concurrently, only the first exception (by array order) propagates — other handlers' failures are silently discarded even though `Task.WhenAll`'s `Exception` holds all of them.
3. **`CommandHandlerInvokerMiddleware`/`RequestHandlerInvokerMiddleware`** duplicate ~25 lines of near-identical reflection/dispatch logic — the exact duplication responsible for issue #393 (AmbiguousMatchException) existing in only one of the two copies.
4. **Three near-identical pipeline-builder classes** (`CommandPipelineBuilder`, `RequestPipelineBuilder`, `EventPipelineBuilder`) duplicate `Properties`/`Use`/`Build` scaffolding; `ICommandPipelineBuilder` exposes `Use(int,...)`/`Remove`/`RemoveAt`/`Clear` with zero call sites anywhere in the repo.
5. **Dead code:** non-generic `CommandContext`/`RequestContext` records are never instantiated anywhere (only the generic ``/`` versions are used); `CommandHandlerInvokerMiddleware.cs:38-41` has commented-out code referencing types that no longer exist.
6. **`RequestPipeline.cs:13`:** `_pipeline` lazy-init field is not `volatile`, unlike the equivalent field in `EventPipeline.cs` — inconsistent thread-safety posture for the same pattern.
**Proposed fix:** Swap the middleware registration order in (1); aggregate all faulted tasks in (2); extract a shared generic dispatch helper for (3); introduce a shared `MiddlewarePipelineBuilder` base for (4) and drop the unused members; delete the dead code in (5); mark `_pipeline` `volatile` in (6).
_Found via automated codebase quality audit._
Contributor guide
No contributing guide indexed for this repository
Research direction
Start with CommandPipeline.cs, ParallelProcessingStrategy.cs, the command/request invoker middleware, the three pipeline builders, the context records, and RequestPipeline.cs. Trace the existing middleware, dispatch, builder, and lazy-initialization patterns before changing them. Done means all six findings are addressed without leaving unused members or duplicate behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp
- Domain
- backend
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100