Skip to content

Commit 87a0d14

Browse files
thomhurstclaude
andcommitted
refactor: clean separation of output and exception handling
Redesign ExecuteInternal to use a linear three-step flow: 1. Execute pipeline, capture any exception (don't rethrow yet) 2. Always print summary exactly once via PrintSummary() 3. Handle exceptions: rethrow original OR throw PipelineFailedException This replaces the previous try/catch structure which had a bug where OnEnd could execute twice if it threw PipelineFailedException (caught by the catch block, which would call OnEnd again). Key changes: - Rename OnEnd to PrintSummary (single responsibility - only output) - PrintSummary never throws, just prints and returns - Use ExceptionDispatchInfo to preserve original stack trace - Exception decision logic is now linear and explicit Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent b1fe79c commit 87a0d14

1 file changed

Lines changed: 38 additions & 23 deletions

File tree

src/ModularPipelines/Engine/Executors/ExecutionOrchestrator.cs

Lines changed: 38 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
using System.Diagnostics;
2+
using System.Runtime.ExceptionServices;
23
using Microsoft.Extensions.Logging;
34
using Microsoft.Extensions.Options;
45
using ModularPipelines.Enums;
@@ -115,34 +116,59 @@ private async Task<PipelineSummary> ExecuteInternal(CancellationToken cancellati
115116
var start = DateTimeOffset.UtcNow;
116117
var stopWatch = Stopwatch.StartNew();
117118

119+
// Step 1: Execute pipeline, capture any exception (don't rethrow yet)
120+
PipelineSummary? summary = null;
121+
Exception? caughtException = null;
118122
try
119123
{
120-
var result = await ExecutePipeline(runnableModules, organizedModules).ConfigureAwait(false);
124+
summary = await ExecutePipeline(runnableModules, organizedModules).ConfigureAwait(false);
125+
}
126+
catch (Exception ex)
127+
{
128+
caughtException = ex;
129+
}
121130

122-
return await OnEnd(organizedModules, stopWatch, start, result, throwOnFailure: true).ConfigureAwait(false);
131+
// Step 2: Always print summary (exactly once)
132+
summary = await PrintSummary(organizedModules, stopWatch, start, summary).ConfigureAwait(false);
133+
134+
// Step 3: Handle exceptions - rethrow original if present, otherwise check for pipeline failure
135+
if (caughtException != null)
136+
{
137+
ExceptionDispatchInfo.Capture(caughtException).Throw();
123138
}
124-
catch
139+
140+
if (summary.Status == Status.Failed && _options.Value.ThrowOnPipelineFailure)
125141
{
126-
// Don't throw PipelineFailedException here - we want to rethrow the original exception
127-
await OnEnd(organizedModules, stopWatch, start, null, throwOnFailure: false).ConfigureAwait(false);
128-
throw;
142+
var failedModules = summary.GetFailedModuleResults()
143+
.Select(r => r.ModuleName)
144+
.ToList();
145+
146+
throw new PipelineFailedException(summary, failedModules);
129147
}
148+
149+
return summary;
130150
}
131151

132-
private async Task<PipelineSummary> OnEnd(OrganizedModules organizedModules, Stopwatch stopWatch, DateTimeOffset start,
133-
PipelineSummary? pipelineSummary, bool throwOnFailure)
152+
/// <summary>
153+
/// Prints the pipeline summary and flushes output. Does not throw exceptions.
154+
/// </summary>
155+
private async Task<PipelineSummary> PrintSummary(
156+
OrganizedModules organizedModules,
157+
Stopwatch stopWatch,
158+
DateTimeOffset start,
159+
PipelineSummary? existingSummary)
134160
{
135161
var end = DateTimeOffset.UtcNow;
136-
pipelineSummary ??= _pipelineSummaryFactory.Create(organizedModules.AllModules, stopWatch.Elapsed, start, end);
162+
var summary = existingSummary ?? _pipelineSummaryFactory.Create(organizedModules.AllModules, stopWatch.Elapsed, start, end);
137163

138-
_outputCoordinator.PrintResults(pipelineSummary);
164+
_outputCoordinator.PrintResults(summary);
139165

140166
await System.Console.Out.FlushAsync().ConfigureAwait(false);
141167

142168
// Flush any buffered exceptions after the results table has been printed
143169
_outputCoordinator.FlushExceptions();
144170

145-
// Check for original exception before logging cancellation reason
171+
// Log cancellation/failure info (informational only)
146172
if (_engineCancellationToken.OriginalException != null)
147173
{
148174
_logger.LogInformation("Pipeline Failed: {ExceptionType}",
@@ -154,18 +180,7 @@ private async Task<PipelineSummary> OnEnd(OrganizedModules organizedModules, Sto
154180
_engineCancellationToken.Reason);
155181
}
156182

157-
// Throw exception if pipeline failed - this ensures non-zero exit code in CI
158-
// Only throw when throwOnFailure is true (success path), not when handling an existing exception
159-
if (throwOnFailure && pipelineSummary.Status == Status.Failed && _options.Value.ThrowOnPipelineFailure)
160-
{
161-
var failedModules = pipelineSummary.GetFailedModuleResults()
162-
.Select(r => r.ModuleName)
163-
.ToList();
164-
165-
throw new PipelineFailedException(pipelineSummary, failedModules);
166-
}
167-
168-
return pipelineSummary;
183+
return summary;
169184
}
170185

171186
private async Task<PipelineSummary> ExecutePipeline(List<IModule> runnableModules, OrganizedModules organizedModules)

0 commit comments

Comments
 (0)