From d085b536f0c9b0d1ae2c4f077ddd4465e38d86be Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Tue, 11 Aug 2026 03:04:49 +0200 Subject: [PATCH] docs(core): document FaultSignal self-receipt and pin it with a test Review raised that TrySendSignalAsync delivers to the faulting activity before walking ancestors, so an activity that throws and also handles FaultSignal can claim its own fault and suppress the incident strategy. That is real, but it is the channel's existing dispatch, which #7911 chose deliberately over a variant of it, and SignalContext.IsSelf exists so handlers can discriminate. It also grants no capability: an activity that catches its own exception never faults at all, ending Finished/Finished with zero incidents, which is a cleaner suppression than self-handling (incident still recorded, activity left Running, workflow suspended). So dispatch is unchanged. What was missing is that none of this was written down: the contract describes the handler as an enclosing container and never mentioned self-receipt. Document it on FaultSignal, including how a handler that wants ancestors-only semantics opts out, and add a test so the behavior is pinned rather than incidental. Refs #7911 Co-Authored-By: Claude Opus 5 --- .../Signals/FaultSignal.cs | 9 +++++++ .../SelfHandlingFaultingActivity.cs | 17 ++++++++++++ .../FaultSignals/FaultSignalTests.cs | 27 +++++++++++++++++++ 3 files changed, 53 insertions(+) create mode 100644 test/integration/Elsa.Workflows.IntegrationTests/Scenarios/FaultSignals/Activities/SelfHandlingFaultingActivity.cs diff --git a/src/modules/Elsa.Workflows.Core/Signals/FaultSignal.cs b/src/modules/Elsa.Workflows.Core/Signals/FaultSignal.cs index f29072c0b..ff6305340 100644 --- a/src/modules/Elsa.Workflows.Core/Signals/FaultSignal.cs +++ b/src/modules/Elsa.Workflows.Core/Signals/FaultSignal.cs @@ -31,6 +31,15 @@ namespace Elsa.Workflows.Signals; /// 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: /// /// 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 index 560cb9e8e..6552a7dff 100644 --- a/test/integration/Elsa.Workflows.IntegrationTests/Scenarios/FaultSignals/FaultSignalTests.cs +++ b/test/integration/Elsa.Workflows.IntegrationTests/Scenarios/FaultSignals/FaultSignalTests.cs @@ -77,6 +77,33 @@ public class FaultSignalTests(ITestOutputHelper testOutputHelper) 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() {