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 8 comments.

[cgillum]

Adding a few quick comments for now.

[cgillum]

Shouldn't we be updating this in the message dequeue path - i.e. TaskQueue.cs or ControlQueue.cs?

[sophiatev]

We can, but this felt the more natural place for me since we are setting it at the point where it will actually be used (this value is only read by the dispatchers, and so it made sense to me to set it when we are generating the work item to be sent to the dispatchers). And this way all the changes are localized in AzureStorageOrchestrationService

That being said I don't feel strongly about this so I can move it if that's better

[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");
            }