Skip to content

Isolate inbound payload hydration failures to their deliveries - #829

Open
wangbill (YunchuWang) wants to merge 1 commit into
mainfrom
yunchuwang-payload-hydration-isolation
Open

wangbill (YunchuWang) wants to merge 1 commit into
mainfrom
yunchuwang-payload-hydration-isolation

Conversation

@YunchuWang

Copy link
Copy Markdown
Member

Summary

What changed?

  • Carry an inbound work-item hydration failure, including its delivery/completion token, through an internal error contract in the shared gRPC assembly.
  • Read work items with MoveNext directly and recover at the delivery boundary, rather than trying to resume a faulted ReadAllAsync iterator. Failed/partially hydrated items are never dispatched.
  • Reuse the worker's existing per-kind abandonment and retry logic. Abandon only the failed token; tolerate ownership-loss NotFound locally and propagate other abandonment failures to connection-level recovery.
  • Add four stream-consumer regression cases, 34 real HTTP/2 loopback worker/interceptor cases, and an Unreleased changelog entry.

Why is this change needed?

  • A missing blob, transient download error, or authorization failure for one incoming delivery currently terminates the shared GetWorkItems stream, disrupting unrelated healthy work. The fix keeps that same stream open while leaving failed deliveries retryable.

Issues / work items


Project checklist

  • Release notes are not required for the next release
    • Otherwise: Notes added to release_notes.md
  • Backport is not required
    • Otherwise: Backport tracked by issue/PR #issue_or_pr
  • All required tests have been added/updated (unit tests, E2E tests)
  • Breaking change?
    • If yes:
      • Impact: N/A.
      • Migration guidance: N/A.

AI-assisted code disclosure (required)

Was an AI tool used? (select one)

  • No
  • Yes, AI helped write parts of this PR (e.g., GitHub Copilot)
  • Yes, an AI agent generated most of this PR

If AI was used:

  • Tool(s): GitHub Copilot (GPT-6.1 Sol).
  • AI-assisted areas/files: The internal gRPC error contract, payload interceptor/worker receive and abandon paths, regression tests, changelog, and PR description.
  • What you changed after AI output: The agent reproduced the pre-fix failure, iterated on the receive-loop boundary and shared internal contract, and reviewed/validated the final patch. No human-authored edits are claimed; human review is pending.

AI verification (required if AI was used):

  • I understand the code and can explain it
  • I verified referenced APIs/types exist and are correct
  • I reviewed edge cases/failure paths (timeouts, retries, cancellation, exceptions)
  • I reviewed concurrency/async behavior
  • I checked for unintended breaking or behavior changes

Testing

Automated tests

  • Result: Passed for the final patch. The build emits existing compiler/analyzer warnings; the final multi-target invocation reported 96 warnings and 0 errors.

Dependency restoration (both commands passed using the checked-in feed configuration):

dotnet restore test\Grpc.IntegrationTests\Grpc.IntegrationTests.csproj --configfile nuget.config --verbosity minimal --nologo
dotnet restore test\Worker\Grpc.Tests\Worker.Grpc.Tests.csproj --configfile nuget.config --verbosity minimal --nologo
dotnet build src\Extensions\AzureBlobPayloads\AzureBlobPayloads.csproj --configuration Release --no-restore --verbosity minimal --nologo

Result: Passed for netstandard2.0, net6.0, net8.0, and net10.0, including the changed worker and gRPC dependencies.

dotnet test test\Worker\Grpc.Tests\Worker.Grpc.Tests.csproj --configuration Release --no-restore --filter 'FullyQualifiedName~WorkItemStreamConsumerTests|FullyQualifiedName~RunBackgroundTaskLoggingTests|FullyQualifiedName~PayloadInterceptorTests|FullyQualifiedName~GrpcDurableTaskWorkerTests|FullyQualifiedName~ExecuteWithRetryTests' --verbosity minimal --nologo

Result: Passed, 77 tests; 0 failed, 0 skipped.

dotnet test test\Grpc.IntegrationTests\Grpc.IntegrationTests.csproj --configuration Release --no-restore --filter 'FullyQualifiedName~PayloadHydrationFailureTests|FullyQualifiedName~AzureBlobPayloadsSideCarInterceptorTests|FullyQualifiedName~LargePayloadTests' --verbosity minimal --nologo

Result: Passed, 129 tests; 0 failed, 0 skipped. This includes all 34 new loopback cases plus existing payload/history/unary/outbound behavior coverage.

git --no-pager diff --check
git --no-pager diff --cached --check

Result: Both passed before committing.

Manual validation (only if runtime/behavior changed)

  • Environment (OS, .NET version, components): Windows; .NET SDK 10.0.403 / runtime 10.0.14; actual worker and registered Azure Blob payload interceptor over loopback Kestrel HTTP/2 with a mocked PayloadStore.
  • Steps + observed results: These runtime exercises are automated, not a separate manual/live-Azure run.
    1. Before applying the fix, the missing-blob activity regression failed: the healthy follow-up delivery never completed and the bounded wait expired. With the fix, the scenario passes.
    2. Across activity, orchestration, entity V1, and entity V2 deliveries, inject missing-blob, authorization, and transient I/O failures, with successful or NotFound abandonment. Assert exactly one GetWorkItems stream and one Hello, abandonment of only the failed token, and successful completion of both already-running and subsequent healthy activities.
    3. Verify redelivery succeeds after payload recovery; real stream failures and non-NotFound abandon failures reconnect; shutdown during hydration or abandonment does not reconnect; history/unary hydration failures retain their original exception behavior. Consumer tests also verify heartbeat/retry-counter observation and that partially hydrated/current stale items are never dispatched.
  • Evidence (optional): Local baseline/final test logs and TRX results are retained in the coding session. No public log upload or live Azure fault-injection run was performed.

Notes for reviewers

  • No breaking change - public signatures, protobuf field numbers, and persisted JSON/history formats are unchanged. The new delivery error contract is internal to the SDK; no work-item cloning is added to the hot path.
  • Non-NotFound abandonment failures propagate after the existing gRPC retry policy. Failed payload deliveries remain retryable; terminal poison-payload settlement and changes to other SDKs are outside scope.
  • The full solution suite was not run; the focused suites and all supported package target frameworks above were run. CI and human review remain pending. The author-understanding checkbox is intentionally left for human review.
  • An Unreleased entry was added to CHANGELOG.md; no separate release_notes.md or backport was requested.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 30, 2026 19:15

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No active review issues block the change to isolate payload hydration failures to individual deliveries.

No open issues 🎉

What changed in this PR

This PR addresses #825 by isolating inbound payload hydration failures to individual deliveries, preserving the Durable Task worker’s shared stream and unrelated in-flight work.

Changes:

  • Introduces an internal exception carrying the failed delivery and its completion token.
  • Reads work items directly and reuses abandonment logic, tolerating ownership loss while preserving connection-level recovery for other failures.
  • Adds consumer and HTTP/2 loopback regression coverage plus an Unreleased changelog entry.
File Description
test/​Worker/​Grpc.Tests/​WorkItemStreamConsumerTests.cs Tests delivery recovery, callback ordering, and failure propagation.
test/​Grpc.IntegrationTests/​PayloadHydrationFailureTests.cs Adds loopback coverage for isolation, redelivery, reconnection, and shutdown.
src/​Worker/​Grpc/​WorkItemStreamConsumer.cs Uses direct stream reads with delivery-local recovery.
src/​Worker/​Grpc/​Worker.Grpc.csproj Grants integration tests access to worker internals.
src/​Worker/​Grpc/​Logs.cs Adds ownership-loss logging.
src/​Worker/​Grpc/​GrpcDurableTaskWorker.Processor.cs Handles hydration failures using shared abandonment logic.
src/​Grpc/​WorkItemPayloadException.cs Defines the internal failed-delivery exception.
src/​Grpc/​Grpc.csproj Exposes the internal contract to consuming assemblies and tests.
src/​Extensions/​AzureBlobPayloads/​Interceptors/​PayloadInterceptor.cs Wraps work-item hydration failures with delivery context.
CHANGELOG.md Documents delivery-local hydration failure handling.

Review effort: Balanced

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Payload hydration failure for one work item tears down the shared GetWorkItems stream

2 participants