Round-trip Envelope.DeduplicationId, and let a ping reach a FIFO destination (GH-3793) - #3801
Merged
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 was referenced Aug 4, 2026
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
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.
Fixes #3793.
The bug
EnvelopeSerializerwroteGroupIdandPartitionKeybut neverDeduplicationId. Any envelope that went through durable storage — node recovery after a restart, or reassignment while a sending agent is latched — came back withDeduplicationId == null.Publishing that recovered envelope to an SNS/SQS FIFO destination that doesn't have
ContentBasedDeduplicationturned on is then rejected deterministically:So it retries forever, or dead-letters if a
SendingFailurepolicy is configured. The exact outage the durable outbox exists to survive was the one case that failed — and because messages on the in-memory fast path publish fine, it looks like a consequence of the outage itself.The reporter's diagnosis and proposed fix were exactly right. The set-side was covered by
DeliveryOptionsTests; the storage round-trip never was.The fix
deduplication-idis now written and read symmetrically withgroup-id. It also joinsReservedHeaderKeys—reserved_header_key_filtering.every_key_promoted_by_the_reader_is_in_the_reserved_setalready guards that invariant and would have failed otherwise (GH-3408).Backward compatible both ways: previously-stored envelopes simply lack the key, and the key-value format lets older readers skip an unknown one.
Two related AWS defects
Both reproduced against LocalStack, which returns the same validation errors as real AWS here:
MessageGroupIdMessageDeduplicationIdThe circuit-resume ping could never reach a FIFO destination.
Envelope.ForPingcarries aGroupIdbut noDeduplicationId, so the probe hit the same validation — a latched sender could never unlatch on a FIFO queue/topic no matter how healthy the broker was. Both AWS transports now fall back to the envelope id, for pings only. That's 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
MessageDeduplicationIdunconditionally, and a standard topic rejects that parameter outright. It's now gated on the topic type the wayAmazonSqsQueuealready gated it.Ordinary envelopes with no
DeduplicationIdare untouched, so FIFO destinations relying onContentBasedDeduplicationkeep behaving exactly as they do today.Verification
Both new integration tests were confirmed red before the fix — the ping one with the issue's verbatim error message.
CoreTests— 2231 passedWolverine.AmazonSns.Tests— 126 passedWolverine.AmazonSqs.Tests— 264 passeddotnet build wolverine.slnx -c Release— cleanNew coverage:
serialization_and_deserialization.deduplication_id,sqs_fifo_deduplication_id_3793,sns_fifo_deduplication_id_3793, andfifo_topic_publishing_3793(end-to-end against a real FIFO topic withContentBasedDeduplication=false).A 5.0 backport is coming in a separate PR, per the reporter's request.
🤖 Generated with Claude Code
https://claude.ai/code/session_0116vfBcKwcjWn8msM4ZjkuA