Skip to content

Prevent duplicate DLQ messages when afterDlq throws after successful publish#76

Open
Dimitris-Ilias wants to merge 1 commit intoWorkable:masterfrom
Dimitris-Ilias:prevent_dlq_duplication_when_afterdlq_throws
Open

Prevent duplicate DLQ messages when afterDlq throws after successful publish#76
Dimitris-Ilias wants to merge 1 commit intoWorkable:masterfrom
Dimitris-Ilias:prevent_dlq_duplication_when_afterdlq_throws

Conversation

@Dimitris-Ilias
Copy link
Copy Markdown
Contributor

@Dimitris-Ilias Dimitris-Ilias commented May 8, 2026

Summary

As found on #75, on BaseQueueHandler.addToDLQ when an afterDlq hook throws after a successful DLQ publish a duplicate message is published on the DLQ.
This PR tackles this issue

Before this PR

BaseQueueHandler.addToDLQ wraps the decode → publish → afterDlq sequence in a single try/catch. The catch-path republishes the message as a raw-buffer fallback so that messages still reach the DLQ when something goes wrong.

  • That fallback is correct when decode(msg) throws or when the first rabbit.publish is rejected by the broker — the DLQ entry is missing and the catch-path provides it.
  • When afterDlq throws after a successful first publish, the catch-path runs anyway and publishes a second copy of the same message. The DLQ ends up with two entries for one input.
  • Downstream consumers and DLQ replay tooling see duplicate work for every afterDlq failure under load.

After this PR

  • addToDLQ tracks whether the first publish resolved by setting a local dlqPublished flag. The catch-path republishes only when !dlqPublished, so a successful publish + failing afterDlq produces exactly one DLQ entry.
  • All three pre-existing failure modes still work correctly:
    • decode throws → fallback republish runs → 1 entry.
    • First rabbit.publish rejects → fallback republish runs → 1 entry.
    • afterDlq throws after success → fallback skipped → 1 entry (new behavior).
  • Happy-path behavior is unchanged.
  • Tests rewritten and extended to assert the new invariant via a real DLQ subscriber rather than just inspecting publish arguments.

@Dimitris-Ilias Dimitris-Ilias marked this pull request as draft May 8, 2026 11:17
@Dimitris-Ilias Dimitris-Ilias force-pushed the prevent_dlq_duplication_when_afterdlq_throws branch from ce06bda to 7c3e875 Compare May 8, 2026 13:35
@Dimitris-Ilias Dimitris-Ilias marked this pull request as ready for review May 8, 2026 13:36
@Dimitris-Ilias Dimitris-Ilias requested a review from Copilot May 8, 2026 13:36
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes a duplication bug in BaseQueueHandler.addToDLQ where a thrown afterDlq hook could cause the catch-path fallback publish to send a second (duplicate) DLQ message even after the initial DLQ publish succeeded.

Changes:

  • Track whether the primary DLQ publish succeeded (dlqPublished) and skip fallback republish when it did.
  • Update/extend tests to assert the “no duplicate DLQ entry” invariant and validate raw-buffer fallback behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
ts/base-queue-handler.ts Adds a dlqPublished flag to prevent fallback republish when the first DLQ publish already succeeded.
test/base-queue-handler.test.ts Reworks DLQ tests to assert single DLQ delivery and raw-buffer fallback on publish failure.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread ts/base-queue-handler.ts Outdated
Comment thread test/base-queue-handler.test.ts Outdated
Comment thread test/base-queue-handler.test.ts Outdated
Comment thread test/base-queue-handler.test.ts Outdated
@Dimitris-Ilias Dimitris-Ilias force-pushed the prevent_dlq_duplication_when_afterdlq_throws branch 4 times, most recently from 9ac8f59 to ee526ab Compare May 8, 2026 15:16
@Dimitris-Ilias Dimitris-Ilias force-pushed the prevent_dlq_duplication_when_afterdlq_throws branch from ee526ab to 2f466b4 Compare May 8, 2026 15:23
});

it('should add raw buffer to dlq when afterDlq throws to prevent double-encoding', async function () {
it('should not duplicate dlq message when afterDlq throws after successful publish', async function () {
Copy link
Copy Markdown
Contributor Author

@Dimitris-Ilias Dimitris-Ilias May 8, 2026

Choose a reason for hiding this comment

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

ℹ️ This is the new test but diff is a little out of whack because these two tests have very similar setup

@Dimitris-Ilias Dimitris-Ilias requested review from a team May 8, 2026 15:26
@Dimitris-Ilias Dimitris-Ilias force-pushed the prevent_dlq_duplication_when_afterdlq_throws branch from aeafbfd to 2f466b4 Compare May 8, 2026 15:42
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.

2 participants