Round-trip Envelope.DeduplicationId, and let a ping reach a FIFO destination (5.x backport, GH-3793) - #3802
Merged
Conversation
…ination (GH-3793) EnvelopeSerializer wrote GroupId and PartitionKey but never DeduplicationId, so every envelope that went through durable storage -- node recovery after a restart, or reassignment while a sending agent is latched -- came back with DeduplicationId == null. Publishing that recovered envelope to an SNS/SQS FIFO destination without ContentBasedDeduplication is then rejected deterministically ("The topic should either have ContentBasedDeduplication enabled or MessageDeduplicationId provided explicitly"), so it retries forever or dead-letters. The exact outage the durable outbox exists to survive was the one case that failed. The reserved-key guard test already covered the invariant that anything the reader promotes into a typed property must be filtered out of the loose Headers write, so the new key joins ReservedHeaderKeys too. Two related AWS defects, both reproduced against LocalStack: - Wolverine's own circuit-resume ping carries no DeduplicationId, so it was rejected by the same validation -- a latched sender could never probe its way back on a FIFO destination no matter how healthy the broker was. Both AWS transports now fall back to the envelope id for pings only. That is also the semantic we want: every ping body is the same four bytes, so content-based deduplication would happily collapse consecutive probes into one. - SNS mapped MessageDeduplicationId unconditionally, and a *standard* topic rejects that parameter outright. It is now gated on the topic type the way AmazonSqsQueue already gated it. Ordinary envelopes with no DeduplicationId are untouched, so FIFO destinations relying on ContentBasedDeduplication keep working exactly as before. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0116vfBcKwcjWn8msM4ZjkuA
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
5.x backport of #3801, for #3793. The reporter is on .NET 8 LTS and explicitly asked for this — 6.x requires .NET 9.
What it fixes
EnvelopeSerializerwroteGroupIdandPartitionKeybut neverDeduplicationId, so any envelope recovered out of durable storage came back withDeduplicationId == nulland was then rejected deterministically by an SNS/SQS FIFO destination withoutContentBasedDeduplication:Also carried over: the circuit-resume ping could never reach a FIFO destination (it has no
DeduplicationId, so a latched sender could never unlatch), and SNS mappedMessageDeduplicationIdon standard topics, which AWS rejects outright.See #3801 for the full write-up.
Differences from the 6.x PR
EnvelopeSerializerhas noReservedHeaderKeys(EnvelopeSerializer round-trips Headers entries into typed properties — a reserved-key collision escalates through the durable inbox #3408 was never backported), so the new key is just written and read — nothing to add to a filter that doesn't exist here.AmazonSqsQueuehas noEnableFairQueueMessageGroups(Support MessageGroupId on standard SQS queues for fair queues #2886 is 6.x-only); only the deduplication helper was taken.Task-returningIAsyncLifetime.Otherwise identical.
Verification (net8.0, against LocalStack)
CoreTests.Serialization— 47 passedWolverine.AmazonSns.Tests— 108 passedWolverine.AmazonSqs.Tests— 188 passed, 1 failedThat one SQS failure is
end_to_end_with_conventional_routing_with_prefix.send_from_one_node_to_another_all_with_conventional_routing(BrokerInitializationException: Unable to initialize the Broker sqs in time). It reproduces identically on cleanorigin/5.0with none of these changes applied, so it predates this work and is unrelated — nothing here touches broker initialization.🤖 Generated with Claude Code
https://claude.ai/code/session_0116vfBcKwcjWn8msM4ZjkuA