Add poison message handling to Azure Storage

[sophiatev]

This PR introduces poison message handling to the Azure Storage. This is a very primitive implementation which simply stores a message in poison storage if its dequeue count exceeds the (user-configurable) maximum, and deletes it from the queue.

[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 10 out of 10 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

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

• This test file is under Test/, but the active test projects referenced by DurableTask.sln are under test/ (lowercase) and the test/DurableTask.AzureStorage.Tests project will not compile/run sources from Test/DurableTask.AzureStorage.Tests. As-is, these new poison-message tests won't execute in CI; move the file into test/DurableTask.AzureStorage.Tests (or update the test .csproj to include it).

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

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (3)

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

• This test file is under Test/ (capital T), but the solution references the AzureStorage test project under test/DurableTask.AzureStorage.Tests/ (see DurableTask.sln), and SDK-style projects only compile sources under their project directory by default. As-is, these tests likely won’t be built or executed in CI, leaving the new poison-message behavior effectively untested.

namespace DurableTask.AzureStorage.Tests
{
    using System;
    using System.Collections.Generic;
    using System.Diagnostics;

src/DurableTask.AzureStorage/AzureStorageOrchestrationServiceSettings.cs:323

• Typo in XML doc: "messsage(s)" should be "message(s)" (and the sentence is missing "to" before "make progress").

        /// non-terminal state) if the messsage(s) necessary for them make progress are deemed "poisoned" and deleted.

src/DurableTask.AzureStorage/Messaging/TaskHubQueue.cs:464

• On failure to store a poison message, the MessageFailure log call drops the eventType/taskEventId context by passing empty/0, even though those values are available in this method.

                    string.Empty /* EventType */,
                    0 /* TaskEventId */,

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

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

• This new test file is under Test/, but the solution and active test projects are under the lowercase test/ folder (e.g., DurableTask.AzureStorage.Tests is test\\DurableTask.AzureStorage.Tests\\DurableTask.AzureStorage.Tests.csproj in DurableTask.sln). With no Test/**/*.csproj in the repo, these tests won’t be compiled or executed in CI.

namespace DurableTask.AzureStorage.Tests
{
    using System;
    using System.Collections.Generic;
    using System.Diagnostics;

src/DurableTask.AzureStorage/AzureStorageOrchestrationServiceSettings.cs:323

• Grammar issue in the XML documentation: "make to progress" should be "to make progress".

        /// non-terminal state) if the message(s) necessary for them make to progress are deemed "poisoned" and deleted.

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (4)

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

• This new test file is under the capitalized Test/ tree, but the active test projects in this repo are under lowercase test/ and referenced from DurableTask.sln (e.g., test\DurableTask.AzureStorage.Tests\...). As-is, these integration tests are very likely not compiled/executed in CI, which defeats the purpose of adding coverage for poison message handling.

    /// <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.AzureStorage/Messaging/TaskHubQueue.cs:63

• poisonMessageContainerName is composed without normalizing/validating PoisonMessageStorageContainerNameSuffix. If callers set this suffix with uppercase characters, whitespace, or null/empty, the resulting container name can be invalid (container names must be lowercase and must not end with '-'), causing poison handling to fail at runtime when the first poison message is encountered.

            this.backoffHelper = new BackoffPollingHelper(minPollingDelay, maxPollingDelay);
            this.poisonMessageContainerName = $"{this.settings.TaskHubName.ToLowerInvariant()}-{this.settings.PoisonMessageStorageContainerNameSuffix}";
        }

src/DurableTask.AzureStorage/AzureStorageOrchestrationServiceSettings.cs:324

• Doc comment typo/grammar: "if the message(s) necessary for them make to progress" reads like a missing word ("make it to progress"). This is public API documentation and should be corrected for clarity.

        /// This may leave orchestrations permanently as <see cref="OrchestrationStatus.Running"/> (or any other
        /// non-terminal state) if the message(s) necessary for them make to progress are deemed "poisoned" and deleted.
        /// </para>

src/DurableTask.AzureStorage/AzureStorageOrchestrationServiceSettings.cs:331

• Doc comment grammar: "considered "poisoned" moved to poison message storage" is missing punctuation/wording (e.g., a comma after "poisoned"). This is user-facing documentation for a new setting.

        /// <summary>
        /// Gets or sets the number of times a message is dequeued for processing before it is considered "poisoned"
        /// moved to poison message storage, and deleted from the source queue. The default value is 5,000, chosen with
        /// the intent that only very pathological cases will be handled automatically.

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (2)

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

• This new test is under Test/ (capital T), but the solution and test projects are under test/ (lowercase). As a result, this file is not compiled or executed by test/DurableTask.AzureStorage.Tests, so it won’t provide coverage for the new poison-message behavior.

namespace DurableTask.AzureStorage.Tests
{
    using System;
    using System.Collections.Generic;
    using System.Diagnostics;

src/DurableTask.AzureStorage/Messaging/TaskHubQueue.cs:458

• MessageFailure here logs azureStorageClient.BlobAccountName as the account, but the failure is in the context of processing a queue message. For hubs configured with a separate tracking store account, logging the queue account (storageAccountName) makes it easier to correlate with other queue-processing logs.

                this.settings.Logger.MessageFailure(
                    this.azureStorageClient.BlobAccountName,
                    this.settings.TaskHubName,

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

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

• This new test file is under Test/DurableTask.AzureStorage.Tests/, but the repository’s active test project is test/DurableTask.AzureStorage.Tests (lowercase) referenced from DurableTask.sln. As a result, this test won’t be compiled or executed in CI unless it’s moved into the test/ tree (or the test project is updated to include it).

namespace DurableTask.AzureStorage.Tests

src/DurableTask.AzureStorage/AzureStorageOrchestrationServiceSettings.cs:336

• MaxDequeueCount is user-configurable but accepts negative values, which will cause essentially all dequeued messages to be treated as poison (since DequeueCount is always > 0). Consider validating that it’s non-negative to avoid surprising behavior.

        public long MaxDequeueCount { get; set; } = 5000;

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

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

• This new test file is under Test/ (capital T), but the solution and active test projects live under test/ (lowercase) (e.g., DurableTask.sln references test\DurableTask.AzureStorage.Tests\DurableTask.AzureStorage.Tests.csproj). There are no Test/**/*.csproj projects, so these tests won’t be compiled or executed in CI.

namespace DurableTask.AzureStorage.Tests
{
    using System;