elsa-workflows / elsa-workflows/elsa-foundation

[Tech debt] Mediator/Events: pipeline-ordering bug, discarded parallel-handler exceptions, and builder duplication

Open
#419 1 comment 0 reactions 0 assignees View on GitHub
needs-triage tech-debt
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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.