Add poison message handling to the dispatchers

[sophiatev]

This PR introduces poison message handling to the dispatchers. This is done by

1. Introducing a new interface IPoisonMessageHandler that the any orchestration service which has poison message handling is expected to implement
2. In the case of data corruption, the dispatchers will invoke this interface (if provided) to handle the message rather than throwing an exception
3. Otherwise, the dispatchers will invoke this interface to determine if a message is "poisoned", and if so take remediating action if possible (for example failing the orchestration or entity call).

The orchestration service is otherwise responsible for determining what to do with the poison message(s) and how to store them.

[sophiatev]

Unrelated to this PR but I bug I found when testing (JSON was not able to deserialize this event because it lacked a 0-arg constructor and the other constructors all had multiple parameters)

[sophiatev]

Also unrelated to this PR, but I realized while working on it that this code I wrote a while back had some incorrect assumptions so I took the opportunity to fix it

[Copilot]

Pull request overview

This PR adds an extensibility hook (IPoisonMessageHandler) and integrates poison/invalid message detection into the core dispatchers so that corrupted or “poisoned” inputs can be handled deterministically (e.g., fail orchestration/activity/entity work) instead of always throwing.

Changes:

• Introduces IPoisonMessageHandler and wires it into orchestration/activity/entity dispatchers for invalid work items and poison message detection.
• Adds structured logging support for poison-message detection (new event ID + event source + log event).
• Adds dispatch-count tracking on history events and propagates poison metadata through entity request processing.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/DurableTask.Core/Tracing/TraceHelper.cs	Adjusts entity invocation activity ending to better handle partial result sets.
src/DurableTask.Core/TaskOrchestrationDispatcher.cs	Adds poison detection/handling and updates reconciliation to return a drop reason.
src/DurableTask.Core/TaskEntityDispatcher.cs	Adds poison detection/handling for entity messages, plus poison-aware batching/result shaping.
src/DurableTask.Core/TaskActivityDispatcher.cs	Adds poison/invalid handling for activity scheduling messages (including failing poisoned tasks).
src/DurableTask.Core/Logging/StructuredEventSource.cs	Adds a new structured event for poison message detection.
src/DurableTask.Core/Logging/LogHelper.cs	Adds PoisonMessageDetected helper overloads emitting structured logs.
src/DurableTask.Core/Logging/LogEvents.cs	Adds a new structured log event type for poison messages.
src/DurableTask.Core/Logging/EventIds.cs	Reserves a new event ID for poison message detection.
src/DurableTask.Core/IPoisonMessageHandler.cs	New interface defining poison detection and handling hooks.
src/DurableTask.Core/History/HistoryEvent.cs	Adds DispatchCount to history events for poisoning heuristics/telemetry.
src/DurableTask.Core/History/ExecutionRewoundEvent.cs	Adds a parameterless ctor for JSON deserialization compatibility.
src/DurableTask.Core/Entities/OrchestrationEntityContext.cs	Adds AbandonAcquire() to reset lock acquisition state on failure.
src/DurableTask.Core/Entities/EventFormat/RequestMessage.cs	Adds poison metadata fields used during entity request processing.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.

[cgillum]

Some initial comments. I haven't gone through the dispatcher code yet (those are bigger diffs).

[Copilot]

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 9 comments.

[sophiatev]

I added all of these for future-proofing (and consistency, to cover all our poison message cases) in case another backend needs them but really the Azure Storage implementation only needs a handful of them for its own poison-message-handling.

I suppose if we ever need all of them, we could add them at that point and then make a new release that the backend could reference. Or we can keep them to be consistent

[Copilot]

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 9 comments.

[Copilot]

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 7 comments.

[Copilot]

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 8 comments.

[Copilot]

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (6)

src/DurableTask.Core/TaskOrchestrationDispatcher.cs:449

• this.poisonMessageHandler?.MaxDispatchCount is nullable, so evt.DispatchCount > this.poisonMessageHandler?.MaxDispatchCount becomes a lifted comparison that returns bool?, which cannot be used in Where(...). This will not compile when poisonMessageHandler is null.

Compute a non-null max value (e.g., int.MaxValue when no handler) and compare against that.

                        var poisonEvents = runtimeState.NewEvents.Where(evt => evt.DispatchCount > this.poisonMessageHandler?.MaxDispatchCount);

src/DurableTask.Core/TaskActivityDispatcher.cs:183

• scheduledEvent.DispatchCount > this.poisonMessageHandler?.MaxDispatchCount performs a lifted comparison (returns bool?) when poisonMessageHandler is null, which will not compile in an if condition.

Coalesce MaxDispatchCount to a non-null value (e.g., int.MaxValue when no handler) before comparing.

                if (poisonMessageReason == null && scheduledEvent.DispatchCount > this.poisonMessageHandler?.MaxDispatchCount)
                {
                    poisonMessageReason = $"Activity has received an event with dispatch count {taskMessage.Event.DispatchCount} which exceeds the maximum dispatch " +
                        $"count of {this.poisonMessageHandler.MaxDispatchCount}. The task will be failed.";
                }

src/DurableTask.Core/TaskEntityDispatcher.cs:474

• request.DispatchCount > this.poisonMessageHandler?.MaxDispatchCount is a lifted comparison that returns bool? when poisonMessageHandler is null, which cannot be assigned to bool.

Gate the comparison on a non-null handler (or coalesce to a non-null max).

            bool isPoisonMessage = request.DispatchCount > this.poisonMessageHandler?.MaxDispatchCount;

src/DurableTask.Core/TaskEntityDispatcher.cs:590

• eventRaisedEvent.DispatchCount > this.poisonMessageHandler?.MaxDispatchCount is a lifted comparison that returns bool? when poisonMessageHandler is null, which cannot be assigned to bool.

Gate the comparison on a non-null handler (or coalesce to a non-null max).

                        bool isPoisonMessage = eventRaisedEvent.DispatchCount > this.poisonMessageHandler?.MaxDispatchCount;

src/DurableTask.Core/TaskEntityDispatcher.cs:1116

• workToDoNow.Operations.Any(op => op.DispatchCount > this.poisonMessageHandler?.MaxDispatchCount) uses a lifted comparison in the Any predicate (returns bool? when the handler is null), which will not compile.

Only evaluate the Any(...) when the handler is non-null (or compare against a coalesced max).

            bool poisonMessagesExist = workToDoNow.Operations.Any(op => op.DispatchCount > this.poisonMessageHandler?.MaxDispatchCount);

Test/DurableTask.AzureStorage.Tests/PoisonMessageHandlingTests.cs:47

• This test file is added under Test/DurableTask.AzureStorage.Tests/, but the repo’s test projects are under test/ and the referenced AzureStorage test project is test/DurableTask.AzureStorage.Tests/DurableTask.AzureStorage.Tests.csproj. As a result, this file will not be compiled or executed by the existing test project/solution, so it won’t provide the intended coverage.

Move the file into test/DurableTask.AzureStorage.Tests/ (or add/update a Test/*.csproj + solution/project references) so the tests actually run in CI.

    [TestClass]
    public class PoisonMessageHandlingTests
    {

[Copilot]

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (9)

Test/DurableTask.AzureStorage.Tests/PoisonMessageHandlingTests.cs:47

• This file is under Test/DurableTask.AzureStorage.Tests, but the repo’s active test project is test/DurableTask.AzureStorage.Tests (referenced by DurableTask.sln). There is no .csproj under Test/, so these tests will not be compiled or executed in CI as-is.

Move this test to test/DurableTask.AzureStorage.Tests (or add an actual Test/DurableTask.AzureStorage.Tests/*.csproj and wire it into the solution/pipelines).

    [TestClass]
    public class PoisonMessageHandlingTests
    {

src/DurableTask.Core/TaskOrchestrationDispatcher.cs:449

• evt.DispatchCount > this.poisonMessageHandler?.MaxDispatchCount does not compile because this.poisonMessageHandler?.MaxDispatchCount is int?, making the comparison result bool? (which can’t be used by Where). Coalesce the max dispatch count to an int (e.g. int.MaxValue when the handler is null) before comparing.

                        var poisonEvents = runtimeState.NewEvents.Where(evt => evt.DispatchCount > this.poisonMessageHandler?.MaxDispatchCount);

src/DurableTask.Core/TaskEntityDispatcher.cs:464

• request.DispatchCount > this.poisonMessageHandler?.MaxDispatchCount won’t compile because the RHS becomes int? and the comparison produces bool?. Coalesce the handler max to an int (e.g. int.MaxValue when no handler is available).

            bool isPoisonMessage = request.DispatchCount > this.poisonMessageHandler?.MaxDispatchCount;

src/DurableTask.Core/TaskEntityDispatcher.cs:584

• eventRaisedEvent.DispatchCount > this.poisonMessageHandler?.MaxDispatchCount won’t compile because the RHS becomes int? and the comparison produces bool?. Coalesce the handler max to an int (e.g. int.MaxValue when no handler is available).

                        bool isPoisonMessage = eventRaisedEvent.DispatchCount > this.poisonMessageHandler?.MaxDispatchCount;

src/DurableTask.Core/TaskEntityDispatcher.cs:764

• if (request.DispatchCount > this.poisonMessageHandler?.MaxDispatchCount) won’t compile because the RHS becomes int? and the comparison produces bool?. Coalesce the handler max to an int (e.g. int.MaxValue when no handler is available).

                    if (request.DispatchCount > this.poisonMessageHandler?.MaxDispatchCount)

src/DurableTask.Core/TaskEntityDispatcher.cs:1106

• Any(op => op.DispatchCount > this.poisonMessageHandler?.MaxDispatchCount) won’t compile because the RHS becomes int? and the comparison produces bool?. Coalesce the handler max to an int (e.g. int.MaxValue when no handler is available).

            bool poisonMessagesExist = workToDoNow.Operations.Any(op => op.DispatchCount > this.poisonMessageHandler?.MaxDispatchCount);

src/DurableTask.Core/TaskEntityDispatcher.cs:993

• SendLockResponseMessage now requires a non-null FailureDetails, but the call site passes null for non-poison cases. Make the parameter nullable to match ResponseMessage.FailureDetails and the call sites.

        void SendLockResponseMessage(WorkItemEffects effects, OrchestrationInstance target, Guid requestId, FailureDetails failureDetails)

src/DurableTask.Core/TaskActivityDispatcher.cs:183

• The poison dispatch-count check compares against this.poisonMessageHandler?.MaxDispatchCount, which is int?. This makes the comparison a bool? and won’t compile, and it also makes the failure message rely on poisonMessageHandler being non-null. Capture a non-null maxDispatchCount first (using int.MaxValue when the handler is null) and use that for both the comparison and message.

                if (poisonMessageReason == null && scheduledEvent.DispatchCount > this.poisonMessageHandler?.MaxDispatchCount)
                {
                    poisonMessageReason = $"Activity has received an event with dispatch count {taskMessage.Event.DispatchCount} which exceeds the maximum dispatch " +
                        $"count of {this.poisonMessageHandler.MaxDispatchCount}. The task will be failed.";
                }

src/DurableTask.AzureStorage/AzureStorageOrchestrationService.cs:200

• instancePoisonMessageContainer/activityPoisonMessageContainer are readonly but are only assigned when IsPoisonMessageStorageEnabled is true. This fails definite-assignment rules and won’t compile when poison storage is disabled. Initialize the container references unconditionally (it should be a cheap reference object) and continue to guard actual usage behind IsPoisonMessageStorageEnabled.

            if (this.settings.IsPoisonMessageStorageEnabled)
            {
                string prefix = string.IsNullOrEmpty(this.settings.PoisonMessageStorageContainerNamePrefix)
                    ? "durable-task-poison"
                    : this.settings.PoisonMessageStorageContainerNamePrefix;

                this.instancePoisonMessageContainer = this.azureStorageClient.GetBlobContainerReference($"{prefix}-instance-messages");
                this.activityPoisonMessageContainer = this.azureStorageClient.GetBlobContainerReference($"{prefix}-activity-messages");
            }

[Copilot]

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (7)

Test/DurableTask.AzureStorage.Tests/PoisonMessageHandlingTests.cs:37

• These tests were added under Test/, but the repo solution and active test projects are under test/ (lowercase). Test/DurableTask.AzureStorage.Tests has no .csproj and isn’t referenced by DurableTask.sln, so this file likely won’t compile/run in CI and won’t validate the new poison-message behavior.

    /// <summary>
    /// Integration tests for poison message handling in <see cref="AzureStorageOrchestrationService"/>.
    /// These tests require the Azure Storage emulator (Azurite) to be running.
    /// </summary>

src/DurableTask.Core/TaskOrchestrationDispatcher.cs:449

• This Where predicate compares int to int? (poisonMessageHandler?.MaxDispatchCount), which produces a bool? and won’t compile as a LINQ predicate. Coalesce the max dispatch count to an int (e.g., int.MaxValue when no handler is available).

                        var poisonEvents = runtimeState.NewEvents.Where(evt => evt.DispatchCount > this.poisonMessageHandler?.MaxDispatchCount);

src/DurableTask.Core/TaskActivityDispatcher.cs:179

• This comparison uses this.poisonMessageHandler?.MaxDispatchCount (an int?), making the whole expression a bool?, which won’t compile in an if condition. Coalesce to an int value when there is no handler.

                if (poisonMessageReason == null && scheduledEvent.DispatchCount > this.poisonMessageHandler?.MaxDispatchCount)

src/DurableTask.Core/TaskEntityDispatcher.cs:464

• This comparison uses this.poisonMessageHandler?.MaxDispatchCount (an int?), so the result becomes bool? and won’t compile. Coalesce the max dispatch count to an int when no poison handler is available.

            bool isPoisonMessage = request.DispatchCount > this.poisonMessageHandler?.MaxDispatchCount;

src/DurableTask.Core/TaskEntityDispatcher.cs:584

• This comparison uses this.poisonMessageHandler?.MaxDispatchCount (an int?), which makes the result bool? and won’t compile. Coalesce the max dispatch count to an int (e.g., int.MaxValue when poison handling isn’t available).

                        bool isPoisonMessage = eventRaisedEvent.DispatchCount > this.poisonMessageHandler?.MaxDispatchCount;

src/DurableTask.Core/TaskEntityDispatcher.cs:764

• This if condition compares int to int? (poisonMessageHandler?.MaxDispatchCount), producing a bool? that won’t compile. Coalesce the max dispatch count to an int when no handler is available.

                    if (request.DispatchCount > this.poisonMessageHandler?.MaxDispatchCount)

src/DurableTask.Core/TaskEntityDispatcher.cs:1106

• The Any predicate compares int to int? (poisonMessageHandler?.MaxDispatchCount), which yields a bool? and won’t compile. Coalesce the max dispatch count to an int when no handler is available.

            bool poisonMessagesExist = workToDoNow.Operations.Any(op => op.DispatchCount > this.poisonMessageHandler?.MaxDispatchCount);

[cgillum]

Generally speaking all containers we create should be prefixed with the task hub name. Otherwise, we'll leak data across apps if they share the same storage account.

[cgillum]

I'm a bit surprised to see this inside of CompleteTaskOrchestrationWorkItemAsync and not AbandonTaskOrchestrationWorkItemAsync. This is the code path we generally go down when a work item completes successfully, but a message is a poison message, I wouldn't expect us to get here...unless we can somehow guarantee that we gracefully handle every possible poison message situation (which doesn't seem possible to guarantee)?

[cgillum]

How do we know that this is a request/response operation and not a one-way operation?