Skip to content

PROD-77638 - Prevent double-stringify of DLQ messages on catch path#75

Merged
Dimitris-Ilias merged 3 commits intoWorkable:masterfrom
Dimitris-Ilias:prevent_double_stringify_on_dlq_catch_path
May 8, 2026
Merged

PROD-77638 - Prevent double-stringify of DLQ messages on catch path#75
Dimitris-Ilias merged 3 commits intoWorkable:masterfrom
Dimitris-Ilias:prevent_double_stringify_on_dlq_catch_path

Conversation

@Dimitris-Ilias
Copy link
Copy Markdown
Contributor

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

Summary

The BaseQueueHandler.addToDLQ catch-path corrupts message payloads when the primary DLQ publish or afterDlq hook fails, producing double-encoded messages on the DLQ that downstream consumers cannot decode back to their original shape.

Before this PR

When a message fails repeatedly and the primary addToDLQ flow throws (e.g. afterDlq raises during transient broker instability or downstream errors), execution falls into the catch block:

} catch (err) {
  this.logger.error(err);
  await this.rabbit.publish(this.dlqName, msg.content.toString(), msg.properties);
  ack(err.message, null);
}
  • msg.content is the original raw Buffer of already-encoded JSON; calling .toString() yields a JSON string. rabbit.publish then routes it through encode() which calls JSON.stringify again, producing a double-encoded payload. For an original event of {"foo":1}, the DLQ ends up with "{\"foo\":1}"
  • Any consumer that decodes this twin gets a plain string instead of the expected object — handlers and DLQ replay tooling break

After this PR

  • Catch-path passes msg.content (the raw Buffer) directly to rabbit.publish instead of msg.content.toString(). The DLQ entry is now byte-identical to the original message.
  • encode() short-circuits on Buffer.isBuffer(message) and returns the Buffer unchanged. This honors the type contract and is what allows the catch-path to keep using the high-level rabbit.publish API without dropping to channel.sendToQueue.
  • Catch-path error log now includes the correlationId and target DLQ name, matching the [correlationId]-prefixed convention used elsewhere in the file.
  • Test rewritten to subscribe to the DLQ and assert that delivered messages decode round-trip to the original event object, catching regressions at the actual broker boundary instead of the publish argument.

Follow-ups

The catch-path may still publish two messages to the DLQ when afterDlq throws after a successful first publish (one from the try-block, one from the catch). This PR fixes the malformation but not the duplication. The new test already exercises that path and can be extended in a follow-up PR to assert deduplicated behavior.

@Dimitris-Ilias Dimitris-Ilias changed the title Prevent double-stringify of DLQ messages on catch path PROD-77638 - Prevent double-stringify of DLQ messages on catch path May 7, 2026
@Dimitris-Ilias Dimitris-Ilias requested review from a team and nikosd23 May 8, 2026 09:00
Comment thread ts/encode-decode.ts
Comment thread ts/base-queue-handler.ts
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 DLQ payload corruption when the addToDLQ catch-path republishes a message after a DLQ publish or afterDlq hook failure, preventing double-JSON-stringification and ensuring DLQ messages remain byte-identical to the original.

Changes:

  • encode() now short-circuits when given a Buffer, returning it unchanged.
  • BaseQueueHandler.addToDLQ catch-path republishes msg.content (raw Buffer) instead of msg.content.toString(), and improves error logging context.
  • Test rewritten to validate DLQ messages decode back to the original event shape at the broker boundary.

Reviewed changes

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

File Description
ts/encode-decode.ts Treat Buffer payloads as already-encoded and return them unchanged to avoid double-encoding.
ts/base-queue-handler.ts Republishes raw message bytes to DLQ on catch-path to preserve payload fidelity; adds correlationId/DLQ context to error logs.
test/base-queue-handler.test.ts Updates DLQ test to subscribe to the DLQ and validate round-trip decoding for messages delivered by RabbitMQ.

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

Comment thread ts/encode-decode.ts
Comment thread test/base-queue-handler.test.ts
Comment thread test/base-queue-handler.test.ts
Comment thread test/base-queue-handler.test.ts Outdated
nikosd23
nikosd23 previously approved these changes May 8, 2026
@Dimitris-Ilias Dimitris-Ilias merged commit e0dee3f into Workable:master May 8, 2026
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.

4 participants