docs(core): give FaultSignal handlers a working completion path

Review caught that the contract advertised something that cannot work. It
offered handlers three ways to terminalize the faulted activity - cancel,
complete, or reschedule - but CompleteActivityAsync returns immediately unless
the activity is Running, and throughout the handler it is still Faulted, since
recovery runs only after the handler returns. Completing inline did nothing at
all, silently, leaving the child Running.

Measured, same container, handler completing the faulted child:

  inline complete                    -> child Running,   no output, Running/Suspended
  TransitionTo(Running), complete    -> child Completed, "after",   Finished/Finished

So a supported path exists; it just needed writing down. Document it on
FaultSignal, note that it is not licence to call RecoverFromFault (which also
rewrites the fault counts), and note that cancelling and rescheduling need no
equivalent step. Cover it with an integration test asserting that completing the
child with a substitute result fires the container's completion callback and
resumes its sequencing.

Refs #7911

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Sipke Schoorstra 2026-08-11 22:08:44 +02:00
parent d085b536f0
commit a618337923
No known key found for this signature in database
GPG key ID: 5C10502B28A4268F
2 changed files with 50 additions and 0 deletions

View file

@ -65,6 +65,24 @@ namespace Elsa.Workflows.Signals;
/// </item>
/// </list>
/// <para>
/// <b>Completing the faulted activity takes one extra step.</b> <c>CompleteActivityAsync</c> returns immediately unless
/// the activity is <see cref="ActivityStatus.Running"/>, and throughout the handler it is still
/// <see cref="ActivityStatus.Faulted"/>, because recovery runs only once the handler has returned. Completing it inline
/// therefore does nothing at all, silently. A handler that wants to complete the activity — substituting a result for
/// the work that failed, say — has to move it out of the faulted state first:
/// </para>
/// <code>
/// context.StopPropagation();
/// signal.FaultedContext.TransitionTo(ActivityStatus.Running);
/// await signal.FaultedContext.CompleteActivityAsync(substituteResult);
/// </code>
/// <para>
/// This is not licence to call <c>RecoverFromFault()</c>, which also rewrites the fault counts and remains the
/// middleware's job alone. Completing this way fires the enclosing container's completion callback, so the container's
/// normal sequencing resumes. Cancelling needs no equivalent step, because <c>CancelActivityAsync</c> already accepts a
/// faulted activity, and rescheduling needs none either.
/// </para>
/// <para>
/// That last rule is not stylistic. <c>RecoverFromFault()</c> is asymmetric: it <i>sets</i> the faulting context's
/// <see cref="ActivityExecutionContext.AggregateFaultCount"/> to zero, which is idempotent, but <i>decrements</i> the
/// count on every ancestor, which is not. A second call is therefore harmless for the faulting context and harmful for

View file

@ -145,6 +145,38 @@ public class FaultSignalTests(ITestOutputHelper testOutputHelper)
Assert.All(ancestors, x => Assert.Equal(-1, x.AggregateFaultCount));
}
[Fact(DisplayName = "A handler can complete the faulted child with a substitute result")]
public async Task HandlerThatCompletesChildWithSubstituteResult_ResumesTheContainer()
{
// Arrange
var container = new FaultHandlingContainer(async (signal, context) =>
{
context.StopPropagation();
// CompleteActivityAsync no-ops on a non-Running activity, and the middleware recovers only once this
// handler has returned, so the faulted child has to be moved out of Faulted first. This is not the same as
// RecoverFromFault, which would also rewrite the fault counts.
signal.FaultedContext.TransitionTo(ActivityStatus.Running);
await signal.FaultedContext.CompleteActivityAsync("substitute");
})
{
Activities =
{
_faultingActivity,
new WriteLine("after")
}
};
// Act
var result = await RunAsync(container);
// Assert: completing the child fires the container's completion callback, so sequencing resumes.
Assert.Equal(ActivityStatus.Completed, result.GetActivityStatus(_faultingActivity));
Assert.Equal(new[] { "after" }, _fixture.CapturingTextWriter.Lines);
Assert.Equal(WorkflowSubStatus.Finished, result.WorkflowState.SubStatus);
AssertFaultCounts(result, expected: 0);
}
[Fact(DisplayName = "A handler that cancels the faulted child leaves it Canceled, not Running")]
public async Task HandlerThatCancelsChild_LeavesItCanceled()
{