diff --git a/src/modules/Elsa.Workflows.Core/Signals/FaultSignal.cs b/src/modules/Elsa.Workflows.Core/Signals/FaultSignal.cs index ff6305340..50c839117 100644 --- a/src/modules/Elsa.Workflows.Core/Signals/FaultSignal.cs +++ b/src/modules/Elsa.Workflows.Core/Signals/FaultSignal.cs @@ -65,6 +65,24 @@ namespace Elsa.Workflows.Signals; /// /// /// +/// Completing the faulted activity takes one extra step. CompleteActivityAsync returns immediately unless +/// the activity is , and throughout the handler it is still +/// , 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: +/// +/// +/// context.StopPropagation(); +/// signal.FaultedContext.TransitionTo(ActivityStatus.Running); +/// await signal.FaultedContext.CompleteActivityAsync(substituteResult); +/// +/// +/// This is not licence to call RecoverFromFault(), 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 CancelActivityAsync already accepts a +/// faulted activity, and rescheduling needs none either. +/// +/// /// That last rule is not stylistic. RecoverFromFault() is asymmetric: it sets the faulting context's /// to zero, which is idempotent, but decrements the /// count on every ancestor, which is not. A second call is therefore harmless for the faulting context and harmful for diff --git a/test/integration/Elsa.Workflows.IntegrationTests/Scenarios/FaultSignals/FaultSignalTests.cs b/test/integration/Elsa.Workflows.IntegrationTests/Scenarios/FaultSignals/FaultSignalTests.cs index 6552a7dff..962e72a62 100644 --- a/test/integration/Elsa.Workflows.IntegrationTests/Scenarios/FaultSignals/FaultSignalTests.cs +++ b/test/integration/Elsa.Workflows.IntegrationTests/Scenarios/FaultSignals/FaultSignalTests.cs @@ -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() {