diff --git a/src/modules/Elsa.Workflows.Core/Extensions/ActivityExecutionContextExtensions.cs b/src/modules/Elsa.Workflows.Core/Extensions/ActivityExecutionContextExtensions.cs index 43c92034e..b39f8aa6c 100644 --- a/src/modules/Elsa.Workflows.Core/Extensions/ActivityExecutionContextExtensions.cs +++ b/src/modules/Elsa.Workflows.Core/Extensions/ActivityExecutionContextExtensions.cs @@ -221,6 +221,18 @@ public static partial class ActivityExecutionContextExtensions /// Send a signal up the current hierarchy of ancestors. /// public async ValueTask SendSignalAsync(object signal) + { + await activityExecutionContext.TrySendSignalAsync(signal); + } + + /// + /// Send a signal up the current hierarchy of ancestors and report whether a receiver claimed it. + /// + /// The signal to send. + /// + /// true if a receiver called ; otherwise, false. + /// + internal async ValueTask TrySendSignalAsync(object signal) { var receivingContexts = new[] { @@ -244,9 +256,11 @@ public static partial class ActivityExecutionContextExtensions if (signalContext.StopPropagationRequested) { logger.LogDebug("Propagation of signal {SignalType} on activity {ActivityId} of type {ActivityType} was stopped", signalTypeName, ancestorContext.Activity.Id, ancestorContext.Activity.Type); - return; + return true; } } + + return false; } /// @@ -335,6 +349,12 @@ public static partial class ActivityExecutionContextExtensions /// /// Recovers the current activity from a faulted state by resetting fault counts and transitioning the activity to the running status. /// + /// + /// This method is asymmetric with Fault: it sets the current context's fault count to zero, which is idempotent, but + /// decrements the count on every ancestor, which is not. Calling it more than once per fault drives ancestor counts negative. + /// The status transition only applies to an activity that is still , so that a + /// handler that already terminalized the activity does not see its decision undone. + /// public void RecoverFromFault() { activityExecutionContext.AggregateFaultCount = 0; @@ -344,7 +364,8 @@ public static partial class ActivityExecutionContextExtensions foreach (var ancestor in ancestors) ancestor.AggregateFaultCount--; - activityExecutionContext.TransitionTo(ActivityStatus.Running); + if (activityExecutionContext.Status == ActivityStatus.Faulted) + activityExecutionContext.TransitionTo(ActivityStatus.Running); } /// diff --git a/src/modules/Elsa.Workflows.Core/Middleware/Activities/ExceptionHandlingMiddleware.cs b/src/modules/Elsa.Workflows.Core/Middleware/Activities/ExceptionHandlingMiddleware.cs index e2237498c..49f240a55 100644 --- a/src/modules/Elsa.Workflows.Core/Middleware/Activities/ExceptionHandlingMiddleware.cs +++ b/src/modules/Elsa.Workflows.Core/Middleware/Activities/ExceptionHandlingMiddleware.cs @@ -1,5 +1,6 @@ using Elsa.Extensions; using Elsa.Workflows.Pipelines.ActivityExecution; +using Elsa.Workflows.Signals; using Microsoft.Extensions.Logging; namespace Elsa.Workflows.Middleware.Activities; @@ -16,7 +17,8 @@ public static class ExceptionHandlingMiddlewareExtensions } /// -/// Catches any exceptions thrown by downstream components and transitions the workflow into the faulted state. +/// Catches any exceptions thrown by downstream components and transitions the workflow into the faulted state, +/// unless an enclosing container claims the fault by handling the . /// public class ExceptionHandlingMiddleware(ActivityMiddlewareDelegate next, IIncidentStrategyResolver incidentStrategyResolver, ILogger logger) : IActivityExecutionMiddleware @@ -32,6 +34,15 @@ public class ExceptionHandlingMiddleware(ActivityMiddlewareDelegate next, IIncid { logger.LogWarning(e, "An exception was caught from a downstream middleware component"); context.Fault(e); + + // Give the ancestor chain a chance to handle the fault. See FaultSignal for the contract: the handler owns + // terminalizing the faulted activity, and recovering the fault bookkeeping is ours alone to do, exactly once. + if (await context.TrySendSignalAsync(new FaultSignal(e, context))) + { + context.RecoverFromFault(); + return; + } + await HandleIncidentAsync(context); } } diff --git a/src/modules/Elsa.Workflows.Core/Signals/FaultSignal.cs b/src/modules/Elsa.Workflows.Core/Signals/FaultSignal.cs new file mode 100644 index 000000000..50c839117 --- /dev/null +++ b/src/modules/Elsa.Workflows.Core/Signals/FaultSignal.cs @@ -0,0 +1,105 @@ +namespace Elsa.Workflows.Signals; + +/// +/// Sent up the ancestor chain when an activity faulted, giving an enclosing container the chance to handle the failure +/// itself instead of letting it fall through to the workflow-global . +/// +/// +/// +/// A container opts in exactly the way it opts into cancellation, by registering a handler for this signal: +/// +/// +/// public MyContainer() +/// { +/// OnSignalReceived<FaultSignal>(OnChildFaultedAsync); +/// } +/// +/// private async ValueTask OnChildFaultedAsync(FaultSignal signal, SignalContext context) +/// { +/// if (!IsMine(signal.FaultedContext)) +/// return; // Not my child: let it keep bubbling. +/// +/// context.StopPropagation(); // I am handling this. +/// await signal.FaultedContext.CancelActivityAsync(); +/// await ScheduleFallbackAsync(context.ReceiverActivityExecutionContext); +/// } +/// +/// +/// Bubbling is the default. A handler that does not recognize simply returns without +/// stopping propagation, and the signal continues to the next ancestor, so nested containers compose without any of +/// them knowing about the others. If no ancestor stops propagation, the incident strategy runs exactly as it does +/// when this signal does not exist at all. +/// +/// +/// The faulting activity is itself a receiver. SendSignalAsync delivers to the sender before walking its +/// ancestors, and this signal reuses that dispatch rather than introducing a variant of it. An activity that throws and +/// also handles therefore sees its own fault first, and may claim it, which is what a +/// self-retrying or self-compensating activity wants. This grants no ability to hide a failure that an activity did not +/// already have, since one that simply catches its own exception never faults at all. A handler that wants +/// ancestors-only semantics should check , or compare +/// against its own receiver context, the same way it already checks that the faulting context is one of its children. +/// +/// +/// The contract. Responsibilities are split, and the split is deliberate: +/// +/// +/// +/// +/// The middleware calls Fault(), sends this signal, and — only when propagation was stopped — calls +/// RecoverFromFault() exactly once. Nothing else. +/// +/// +/// +/// +/// The handler decides the fault is its own, calls , and then +/// terminalizes the faulted activity: cancel it, complete it, or reschedule it. There is no single right answer +/// for the middleware to pick here — an interrupting error boundary wants the activity cancelled, a retry handler +/// wants it rescheduled, a fallback handler may want it completed with a substitute result — so a handler that +/// claims the failure also owns deciding what becomes of the failed work. +/// +/// +/// +/// +/// The handler must not call RecoverFromFault(). +/// +/// +/// +/// +/// 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 +/// every ancestor, driving their counts to -1. Those counts are persisted and surface through +/// ActivityExecutionRecord.AggregateFaultCount and ActivityExecutionStats, so the failure mode is +/// silently wrong fault numbers in the UI rather than an exception anyone would notice. +/// +/// +/// On the faulted activity's status. RecoverFromFault() transitions the activity out of +/// and back to , so a handler that stops +/// propagation and terminalizes nothing leaves the activity having already thrown. +/// That is the handler's bug. A completing container happens to sweep such an activity via +/// , which cancels its non-completed children, so the +/// mistake degrades rather than hangs — but that is a backstop, not the mechanism, and a container that suspends +/// instead of completing will persist the activity as . +/// +/// +/// The exception that caused the fault. +/// The of the activity that faulted. +public record FaultSignal(Exception Exception, ActivityExecutionContext FaultedContext); diff --git a/test/integration/Elsa.Workflows.IntegrationTests/Scenarios/FaultSignals/Activities/FaultHandlingContainer.cs b/test/integration/Elsa.Workflows.IntegrationTests/Scenarios/FaultSignals/Activities/FaultHandlingContainer.cs new file mode 100644 index 000000000..f05e6e34d --- /dev/null +++ b/test/integration/Elsa.Workflows.IntegrationTests/Scenarios/FaultSignals/Activities/FaultHandlingContainer.cs @@ -0,0 +1,36 @@ +using Elsa.Testing.Shared.Activities; +using Elsa.Workflows.Signals; + +namespace Elsa.Workflows.IntegrationTests.Scenarios.FaultSignals.Activities; + +/// +/// A sequential container that delegates handling to a caller-supplied strategy, so that a +/// single activity type covers every case under test: declining a fault, claiming it, and claiming it incorrectly. +/// +public class FaultHandlingContainer : TestContainer +{ + private readonly Func? _onChildFaulted; + + /// + /// Invoked for every fault bubbling through this container. Passing null declines every fault, which lets it + /// keep bubbling to the next ancestor. + /// + public FaultHandlingContainer(Func? onChildFaulted = null) + { + _onChildFaulted = onChildFaulted; + OnSignalReceived(OnChildFaultedAsync); + } + + /// + /// The number of faults this container was notified of. + /// + public int FaultsSeen { get; private set; } + + private async ValueTask OnChildFaultedAsync(FaultSignal signal, SignalContext context) + { + FaultsSeen++; + + if (_onChildFaulted != null) + await _onChildFaulted(signal, context); + } +} diff --git a/test/integration/Elsa.Workflows.IntegrationTests/Scenarios/FaultSignals/Activities/SelfHandlingFaultingActivity.cs b/test/integration/Elsa.Workflows.IntegrationTests/Scenarios/FaultSignals/Activities/SelfHandlingFaultingActivity.cs new file mode 100644 index 000000000..0ae5cd096 --- /dev/null +++ b/test/integration/Elsa.Workflows.IntegrationTests/Scenarios/FaultSignals/Activities/SelfHandlingFaultingActivity.cs @@ -0,0 +1,17 @@ +using Elsa.Workflows.Signals; + +namespace Elsa.Workflows.IntegrationTests.Scenarios.FaultSignals.Activities; + +/// +/// An activity that throws and also claims its own , used to pin the channel's self-receipt +/// behavior: a signal is delivered to its sender before the walk up the ancestor chain begins. +/// +public class SelfHandlingFaultingActivity : CodeActivity +{ + public SelfHandlingFaultingActivity() + { + OnSignalReceived((_, context) => context.StopPropagation()); + } + + protected override void Execute(ActivityExecutionContext context) => throw new InvalidOperationException("Whoops!"); +} diff --git a/test/integration/Elsa.Workflows.IntegrationTests/Scenarios/FaultSignals/FaultSignalTests.cs b/test/integration/Elsa.Workflows.IntegrationTests/Scenarios/FaultSignals/FaultSignalTests.cs new file mode 100644 index 000000000..962e72a62 --- /dev/null +++ b/test/integration/Elsa.Workflows.IntegrationTests/Scenarios/FaultSignals/FaultSignalTests.cs @@ -0,0 +1,276 @@ +using Elsa.Extensions; +using Elsa.Testing.Shared; +using Elsa.Testing.Shared.Activities; +using Elsa.Workflows.Activities; +using Elsa.Workflows.IncidentStrategies; +using Elsa.Workflows.IntegrationTests.Scenarios.FaultSignals.Activities; +using Elsa.Workflows.Models; +using Elsa.Workflows.Signals; +using Xunit.Abstractions; + +namespace Elsa.Workflows.IntegrationTests.Scenarios.FaultSignals; + +/// +/// Integration tests for : a container's opportunity to claim a child's fault before it +/// becomes a workflow-global incident. +/// +public class FaultSignalTests(ITestOutputHelper testOutputHelper) +{ + private readonly WorkflowTestFixture _fixture = new(testOutputHelper); + private readonly Fault _faultingActivity = Fault.Create("Whoops!", "Test", "Test"); + + [Fact(DisplayName = "A container that handles the signal suppresses the incident strategy")] + public async Task HandledFault_DoesNotFaultTheWorkflow() + { + // Arrange + var container = ContainerAround(_faultingActivity, ClaimAndCancelChildAsync); + + // Act + var result = await RunAsync(container); + + // Assert + Assert.Equal(1, container.FaultsSeen); + Assert.Equal(WorkflowStatus.Finished, result.WorkflowState.Status); + Assert.Equal(WorkflowSubStatus.Finished, result.WorkflowState.SubStatus); + + // The incident stays on record even though the fault was handled: caught failures remain visible to operators. + Assert.Single(result.WorkflowState.Incidents); + } + + [Theory(DisplayName = "A fault nobody handles is left to the incident strategy, exactly as before")] + [InlineData(typeof(FaultStrategy), WorkflowSubStatus.Faulted)] + [InlineData(typeof(ContinueWithIncidentsStrategy), WorkflowSubStatus.Suspended)] + public async Task UnhandledFault_LeavesIncidentStrategyInCharge(Type incidentStrategyType, WorkflowSubStatus expectedSubStatus) + { + // Arrange: a plain container, with no FaultSignal handler anywhere in the chain. + var container = new TestContainer + { + Activities = + { + _faultingActivity + } + }; + + // Act + var result = await RunAsync(container, incidentStrategyType); + + // Assert + Assert.Equal(expectedSubStatus, result.WorkflowState.SubStatus); + Assert.Single(result.WorkflowState.Incidents); + Assert.Equal(ActivityStatus.Faulted, result.GetActivityStatus(_faultingActivity)); + AssertFaultCounts(result, expected: 1); + } + + [Fact(DisplayName = "A fault the inner container declines keeps bubbling to the outer one")] + public async Task DeclinedFault_ReachesTheNextAncestor() + { + // Arrange + var inner = ContainerAround(_faultingActivity); + var outer = ContainerAround(inner, ClaimAndCancelChildAsync); + + // Act + var result = await RunAsync(outer); + + // Assert + Assert.Equal(1, inner.FaultsSeen); + Assert.Equal(1, outer.FaultsSeen); + Assert.Equal(WorkflowSubStatus.Finished, result.WorkflowState.SubStatus); + } + + [Fact(DisplayName = "The faulting activity receives its own fault before any ancestor does")] + public async Task FaultingActivity_ReceivesItsOwnSignalFirst() + { + // Pinning the channel's self-plus-ancestors dispatch, which this signal reuses rather than varying. It lets a + // self-retrying or self-compensating activity claim its own failure, and grants no ability to hide a failure + // that an activity did not already have: one that simply catches its own exception never faults at all. + + // Arrange + var faultingActivity = new SelfHandlingFaultingActivity(); + var container = ContainerAround(faultingActivity); + + // Act + var result = await RunAsync(container, typeof(FaultStrategy)); + + // Assert: the activity claimed its own fault, so the walk never reached the container. + Assert.Equal(0, container.FaultsSeen); + Assert.NotEqual(WorkflowSubStatus.Faulted, result.WorkflowState.SubStatus); + + // The incident is still on record, and the fault bookkeeping was still recovered exactly once. + Assert.Single(result.WorkflowState.Incidents); + + var faultedContext = result.GetActivityContext(faultingActivity); + Assert.NotNull(faultedContext); + Assert.Equal(0, faultedContext.AggregateFaultCount); + Assert.All(faultedContext.GetAncestors(), x => Assert.Equal(0, x.AggregateFaultCount)); + } + + [Fact(DisplayName = "A handled fault restores the fault count on the faulting context and every ancestor")] + public async Task HandledFault_RestoresFaultCounts() + { + // Arrange + var container = ContainerAround(_faultingActivity, ClaimAndCancelChildAsync); + + // Act + var result = await RunAsync(container); + + // Assert + AssertFaultCounts(result, expected: 0); + } + + [Fact(DisplayName = "A handler that also recovers from the fault drives ancestor fault counts negative")] + public async Task HandlerThatAlsoRecoversFromFault_CorruptsAncestorFaultCounts() + { + // Asserting the documented consequence of violating the contract rather than leaving it accidental: recovery + // *sets* the faulting context's count to zero, which is idempotent, but *decrements* every ancestor, which is + // not. The middleware recovers too, so the ancestors end up one below where they started. + + // Arrange + var container = ContainerAround(_faultingActivity, async (signal, context) => + { + context.StopPropagation(); + signal.FaultedContext.RecoverFromFault(); + await context.ReceiverActivityExecutionContext.CompleteActivityAsync(); + }); + + // Act + var result = await RunAsync(container); + + // Assert + var faultedContext = result.GetActivityContext(_faultingActivity); + Assert.NotNull(faultedContext); + + var ancestors = faultedContext.GetAncestors().ToList(); + Assert.NotEmpty(ancestors); + Assert.Equal(0, faultedContext.AggregateFaultCount); + 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() + { + // Arrange + var container = ContainerAround(_faultingActivity, ClaimAndCancelChildAsync); + + // Act + var result = await RunAsync(container); + + // Assert + Assert.Equal(ActivityStatus.Canceled, result.GetActivityStatus(_faultingActivity)); + } + + [Fact(DisplayName = "A handler that terminalizes nothing leaves the child Running and the workflow suspended")] + public async Task HandlerThatTerminalizesNothing_DegradesRatherThanHangs() + { + // The handler's bug, documented: recovery transitions the faulted child back to Running, and a handler that + // schedules no follow-up work moves it no further. The run returns rather than hanging, but the workflow + // suspends with nothing left to resume it, which is why terminalizing is the handler's responsibility. + + // Arrange + var container = ContainerAround(_faultingActivity, (_, context) => + { + context.StopPropagation(); + return default; + }); + + // Act + var result = await RunAsync(container); + + // Assert + Assert.Equal(ActivityStatus.Running, result.GetActivityStatus(_faultingActivity)); + Assert.Equal(WorkflowStatus.Running, result.WorkflowState.Status); + Assert.Equal(WorkflowSubStatus.Suspended, result.WorkflowState.SubStatus); + } + + [Fact(DisplayName = "A completing container sweeps a child the handler failed to terminalize")] + public async Task HandlerThatTerminalizesNothing_IsBackstoppedByTheCompletingContainer() + { + // The backstop, not the mechanism: CompleteActivityAsync cancels non-completed children on the way out. + + // Arrange + var container = ContainerAround(_faultingActivity, async (_, context) => + { + context.StopPropagation(); + await context.ReceiverActivityExecutionContext.CompleteActivityAsync(); + }); + + // Act + var result = await RunAsync(container); + + // Assert + Assert.Equal(ActivityStatus.Completed, result.GetActivityStatus(container)); + Assert.Equal(ActivityStatus.Canceled, result.GetActivityStatus(_faultingActivity)); + } + + /// + /// The canonical handler: claim the fault, terminalize the faulted child, and wind the container up. + /// + private static async ValueTask ClaimAndCancelChildAsync(FaultSignal signal, SignalContext context) + { + context.StopPropagation(); + await signal.FaultedContext.CancelActivityAsync(); + await context.ReceiverActivityExecutionContext.CompleteActivityAsync(); + } + + private static FaultHandlingContainer ContainerAround(IActivity child, Func? onChildFaulted = null) + { + return new(onChildFaulted) + { + Activities = + { + child + } + }; + } + + private Task RunAsync(IActivity root, Type? incidentStrategyType = null) + { + return _fixture.RunWorkflowAsync(new TestWorkflow(builder => + { + builder.WorkflowOptions.IncidentStrategyType = incidentStrategyType; + builder.Root = root; + })); + } + + private void AssertFaultCounts(RunWorkflowResult result, int expected) + { + var faultedContext = result.GetActivityContext(_faultingActivity); + Assert.NotNull(faultedContext); + + var ancestors = faultedContext.GetAncestors().ToList(); + Assert.NotEmpty(ancestors); + Assert.Equal(expected, faultedContext.AggregateFaultCount); + Assert.All(ancestors, x => Assert.Equal(expected, x.AggregateFaultCount)); + } +} diff --git a/test/unit/Elsa.Workflows.Core.UnitTests/Extensions/ActivityExecutionContextExtensions/FaultHandlingTests.cs b/test/unit/Elsa.Workflows.Core.UnitTests/Extensions/ActivityExecutionContextExtensions/FaultHandlingTests.cs index 8db759f1e..d8e15fe5f 100644 --- a/test/unit/Elsa.Workflows.Core.UnitTests/Extensions/ActivityExecutionContextExtensions/FaultHandlingTests.cs +++ b/test/unit/Elsa.Workflows.Core.UnitTests/Extensions/ActivityExecutionContextExtensions/FaultHandlingTests.cs @@ -36,4 +36,74 @@ public class FaultHandlingTests Assert.Equal(0, context.AggregateFaultCount); Assert.Equal(ActivityStatus.Running, context.Status); } + + [Fact] + public async Task Fault_IncrementsFaultCountOnEveryAncestor() + { + // Arrange + var chain = await CreateContextChainAsync(); + + // Act + chain[^1].Fault(new InvalidOperationException("Test error")); + + // Assert + Assert.Equal(new[] { 1, 1, 1 }, chain.Select(x => x.AggregateFaultCount)); + } + + [Fact] + public async Task RecoverFromFault_ResetsFaultCountOnEveryAncestor() + { + // Arrange + var chain = await CreateContextChainAsync(); + var faultedContext = chain[^1]; + faultedContext.Fault(new InvalidOperationException("Test error")); + + // Act + faultedContext.RecoverFromFault(); + + // Assert + Assert.Equal(new[] { 0, 0, 0 }, chain.Select(x => x.AggregateFaultCount)); + Assert.Equal(ActivityStatus.Running, faultedContext.Status); + } + + [Fact] + public async Task RecoverFromFault_CalledTwice_DrivesAncestorFaultCountsNegative() + { + // This is why a FaultSignal handler must not call RecoverFromFault: recovery *sets* the faulting context's + // count to zero, which is idempotent, but *decrements* every ancestor, which is not. The failure mode is + // silently wrong fault numbers rather than an exception, so it is asserted here rather than left accidental. + + // Arrange + var chain = await CreateContextChainAsync(); + var faultedContext = chain[^1]; + faultedContext.Fault(new InvalidOperationException("Test error")); + + // Act + faultedContext.RecoverFromFault(); + faultedContext.RecoverFromFault(); + + // Assert + Assert.Equal(0, faultedContext.AggregateFaultCount); + Assert.Equal(new[] { -1, -1 }, chain.Take(chain.Count - 1).Select(x => x.AggregateFaultCount)); + } + + [Fact] + public async Task RecoverFromFault_LeavesAlreadyTerminalizedStatusAlone() + { + // A FaultSignal handler terminalizes the faulted activity, and the middleware recovers the fault bookkeeping + // afterwards. Recovery must restore the counts without resurrecting the status the handler chose. + + // Arrange + var chain = await CreateContextChainAsync(); + var faultedContext = chain[^1]; + faultedContext.Fault(new InvalidOperationException("Test error")); + faultedContext.TransitionTo(ActivityStatus.Canceled); + + // Act + faultedContext.RecoverFromFault(); + + // Assert + Assert.Equal(ActivityStatus.Canceled, faultedContext.Status); + Assert.Equal(new[] { 0, 0, 0 }, chain.Select(x => x.AggregateFaultCount)); + } } diff --git a/test/unit/Elsa.Workflows.Core.UnitTests/Extensions/ActivityExecutionContextExtensions/TestHelpers.cs b/test/unit/Elsa.Workflows.Core.UnitTests/Extensions/ActivityExecutionContextExtensions/TestHelpers.cs index 81d04a34e..51be2fab9 100644 --- a/test/unit/Elsa.Workflows.Core.UnitTests/Extensions/ActivityExecutionContextExtensions/TestHelpers.cs +++ b/test/unit/Elsa.Workflows.Core.UnitTests/Extensions/ActivityExecutionContextExtensions/TestHelpers.cs @@ -19,4 +19,28 @@ public static class TestHelpers var fixture = new ActivityTestFixture(activity); return fixture.BuildAsync(); } + + /// + /// Creates a chain of nested activity execution contexts for testing ancestor-sensitive behavior. + /// + /// The number of contexts to create. Must be at least two. + /// The contexts, ordered from the outermost ancestor to the innermost descendant. + public static async Task> CreateContextChainAsync(int depth = 3) + { + // Every level is a Sequence so that a single registered activity type covers the whole chain. + var activities = Enumerable.Range(0, depth).Select(_ => new Sequence()).ToList(); + + for (var i = 0; i < depth - 1; i++) + activities[i].Activities.Add(activities[i + 1]); + + var contexts = new List + { + await CreateContextAsync(activities[0]) + }; + + for (var i = 1; i < depth; i++) + contexts.Add(await contexts[0].WorkflowExecutionContext.CreateActivityExecutionContextAsync(activities[i], new() { Owner = contexts[i - 1] })); + + return contexts; + } }