Skip to content

Commit f531ded

Browse files
committed
fix: efcore forward look at query shapes
1 parent 8c54423 commit f531ded

12 files changed

Lines changed: 1004 additions & 7 deletions

File tree

src/core/Flowthru.Core/Flows/Flow.cs

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
using Flowthru.Core.Graph.Meta.Models;
88
using Flowthru.Core.Graph.Scheduling;
99
using Flowthru.Core.Graph.Validation;
10+
using Flowthru.Core.Results;
1011
using Microsoft.Extensions.Logging;
1112

1213
namespace Flowthru.Core.Flows;
@@ -772,12 +773,37 @@ CancellationToken cancellationToken
772773

773774
return FlowResult.CreateSuccess(stopwatch.Elapsed, stepResults, Name);
774775
}
775-
catch (OperationCanceledException)
776+
catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested)
776777
{
777-
// Re-throw — cancellation is not a failure but a requested abort.
778+
// Caller-initiated abort — re-throw cleanly so hosts can distinguish a
779+
// requested cancellation from a runtime failure.
778780
stopwatch.Stop();
779781
throw;
780782
}
783+
catch (OperationCanceledException ex)
784+
{
785+
// Cancellation propagated past the executor without the caller asking
786+
// for it — i.e., an internal stop-on-first-error cascade leaked the
787+
// cancel out instead of cleanly returning partial results. By the
788+
// FlowResult contract this is an unexpected escape, so wrap it in
789+
// FlowExecutionEscapedException so the runtime-error classifier reports
790+
// it as a possible framework bug instead of an external cancellation.
791+
stopwatch.Stop();
792+
var wrapped = new FlowExecutionEscapedException(
793+
"Flow execution aborted by an unexpected cancellation that was not "
794+
+ "caller-initiated. This indicates a possible bug in Flowthru's "
795+
+ "execution path — the original cancellation is preserved as the "
796+
+ "inner exception.",
797+
ex
798+
);
799+
Logger?.LogError(wrapped, "Flow execution aborted unexpectedly: {ErrorMessage}", ex.Message);
800+
return FlowResult.CreateFailure(
801+
stopwatch.Elapsed,
802+
wrapped,
803+
new Dictionary<string, StepResult>(),
804+
Name
805+
);
806+
}
781807
catch (Exception ex)
782808
{
783809
stopwatch.Stop();
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
namespace Flowthru.Core.Results;
2+
3+
/// <summary>
4+
/// Marker exception that wraps any failure which escapes the normal
5+
/// <see cref="Flows.FlowResult"/> contract — i.e., a runtime failure that
6+
/// surfaced as a thrown exception rather than a structured step failure.
7+
/// </summary>
8+
/// <remarks>
9+
/// <para>
10+
/// Flowthru's design invariant (see <c>CONTRIBUTING.md</c>) is that a flow which
11+
/// passes pre-flight checks should always complete successfully, with any
12+
/// step failure captured in <see cref="Flows.FlowResult"/>. When something
13+
/// throws past that boundary — for example, an internal cancellation cascade
14+
/// that leaks out of the executor before partial step results can be
15+
/// gathered — that's an unexpected escape of the contract and almost always
16+
/// indicates a Flowthru framework bug.
17+
/// </para>
18+
/// <para>
19+
/// Wrapping such a failure in <c>FlowExecutionEscapedException</c> lets the
20+
/// runtime-error reporting pipeline classify it correctly as
21+
/// <see cref="ErrorClassification.PossibleFrameworkBug"/> regardless of the
22+
/// inner exception's type — including allowlisted types like
23+
/// <see cref="OperationCanceledException"/> that would otherwise be mistaken
24+
/// for environmental failures.
25+
/// </para>
26+
/// </remarks>
27+
public sealed class FlowExecutionEscapedException : Exception
28+
{
29+
/// <summary>
30+
/// Creates a new <see cref="FlowExecutionEscapedException"/> wrapping an
31+
/// underlying failure that escaped the FlowResult contract.
32+
/// </summary>
33+
/// <param name="message">Human-readable description of the escape.</param>
34+
/// <param name="innerException">The original failure being wrapped.</param>
35+
public FlowExecutionEscapedException(string message, Exception innerException)
36+
: base(message, innerException) { }
37+
}

src/core/Flowthru.Core/Results/RuntimeErrorClassifier.cs

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,20 @@ public static class RuntimeErrorClassifier
2626
/// <remarks>
2727
/// Walks the exception type's inheritance chain and checks inner exceptions.
2828
/// Any match against known external/environmental exception types produces
29-
/// <see cref="ErrorClassification.ExternalError"/>.
29+
/// <see cref="ErrorClassification.ExternalError"/>. A
30+
/// <see cref="FlowExecutionEscapedException"/> short-circuits the inner-walk
31+
/// and is always classified as <see cref="ErrorClassification.PossibleFrameworkBug"/>,
32+
/// since by definition it represents a failure that escaped the FlowResult
33+
/// contract — even if its inner exception happens to be allowlisted.
3034
/// </remarks>
3135
public static ErrorClassification Classify(Exception exception)
3236
{
37+
// Explicit framework-bug marker takes precedence over any inner-exception
38+
// heuristic. Without this short-circuit a leaked OperationCanceledException
39+
// wrapped here would still classify as ExternalError via the inner walk.
40+
if (exception is FlowExecutionEscapedException)
41+
return ErrorClassification.PossibleFrameworkBug;
42+
3343
if (IsExternal(exception))
3444
return ErrorClassification.ExternalError;
3545

src/core/Flowthru.Core/Services/FlowthruService.cs

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
using Flowthru.Core.Graph.Validation;
88
using Flowthru.Core.Meta;
99
using Flowthru.Core.Meta.Providers;
10+
using Flowthru.Core.Results;
1011
using Flowthru.Core.Services.Models;
1112
using Microsoft.Extensions.DependencyInjection;
1213
using Microsoft.Extensions.Logging;
@@ -223,8 +224,43 @@ public async Task<FlowResult> ExecuteFlowAsync(
223224
_logger.LogInformation("════════════════════════════════════════");
224225
_logger.LogInformation("");
225226

226-
// Execute merged pipeline
227-
var result = await mergedPipeline.RunAsync(options, cancellationToken);
227+
// Execute merged pipeline.
228+
//
229+
// Service-level safety net: anything that escapes Flow.RunAsync past the
230+
// FlowResult contract is, by definition, an unexpected runtime escape. The
231+
// wrap below ensures the user-facing formatter still gets a chance to fire
232+
// (with the "please file an issue" framing) before the exception reaches
233+
// the host. Caller-initiated cancellation propagates clean — that's a
234+
// user-requested abort, not a Flowthru failure.
235+
FlowResult result;
236+
try
237+
{
238+
result = await mergedPipeline.RunAsync(options, cancellationToken);
239+
}
240+
catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested)
241+
{
242+
throw;
243+
}
244+
catch (Exception ex)
245+
{
246+
var wrapped =
247+
ex as FlowExecutionEscapedException
248+
?? new FlowExecutionEscapedException(
249+
"Flow execution failed with an exception that escaped the FlowResult "
250+
+ "contract. This is unexpected — pre-flight should have caught the "
251+
+ "underlying issue before runtime. The original exception is "
252+
+ "preserved as the inner exception.",
253+
ex
254+
);
255+
var synthetic = FlowResult.CreateFailure(
256+
executionTime: totalStopwatch.Elapsed,
257+
exception: wrapped,
258+
stepResults: new Dictionary<string, StepResult>(),
259+
flowName: "Pipeline"
260+
);
261+
options.GetFormatter().Format(synthetic, _logger);
262+
throw;
263+
}
228264

229265
// Export post-run metadata — fires only after real executions (dry run returns above)
230266
if (_metadataBuilder != null)

0 commit comments

Comments
 (0)