-
Notifications
You must be signed in to change notification settings - Fork 15
Add Error Details To DLQ Message Headers #91
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds error details to Dead Letter Queue (DLQ) message headers to improve error handling and debugging capabilities. When messages fail processing and are moved to the DLQ, error information is now encoded and included in the message headers.
Key changes:
- Introduces a new utility module with base64 encoding/decoding functions and error parsing
- Updates all adapter implementations to include error information when moving messages to DLQ
- Adds comprehensive test coverage for error header validation across all adapters
Reviewed Changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils.js | New utility module providing base64 encoding/decoding and error-to-headers parsing functionality |
| src/constants.js | Defines new header constants for error information fields (message, stack, code, etc.) |
| src/adapters/redis.js | Implements error info storage in Redis hash with TTL and merges with DLQ messages |
| src/adapters/nats.js | Updates NATS adapter to include error data in DLQ message headers |
| src/adapters/kafka.js | Updates Kafka adapter to include error data in DLQ message headers |
| src/adapters/amqp.js | Updates AMQP adapter to include error data in DLQ message headers |
| src/index.js | Updates JSDoc to document new deadLettering configuration options |
| test/integration/index.spec.js | Adds comprehensive test coverage for error header validation across all adapters |
| README.md | Documents the new error information feature and configuration options |
| package.json | Adds @types/jest dependency for better TypeScript support in tests |
src/adapters/redis.js
Outdated
| // It will be (eventually) picked by xclaim failed_messages() or xreadgroup() | ||
| const parsedError = this.error2ErrorInfoParser(result.reason); | ||
|
|
||
| if (parsedError && Object.keys(parsedError).length === 0) { |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
src/utils.js
Outdated
| if (!err) return null; | ||
|
|
||
| return { | ||
| // Encode to base64 to because of special characters For example, NATS JetStream does not support \n or \r in headers |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
README.md
Outdated
| if (!err) return null; | ||
|
|
||
| return { | ||
| // Encode to base64 to because of special characters. For example, NATS JetStream does not support \n or \r in headers |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
icebob
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks
What?
Fixes #86
Problem
At the moment we're just pushing the payload to DLQ without any error info, which makes it difficult to properly react and take an action to a different error type.
Solution
Right before pushing the message to DLQ the error is processed via newly introduced
error2ErrorInfoParser()method and returns a plain object with base64 values. This object is then placed in the headers of the message.Note that for Redis an additional hash is required to store the error info. This is necessary because in the
chan.failed_messages = async () => {}, function responsible for pushing failed messages to DLQ,we don't have any reference to the actual error.To keep the aforementioned hash "clean", whenever we add a new entry we set a TTL for that key. Defaults for 24h but can be configured via
deadLettering.errorInfoTTLparam.Redis does not have atm
hsetex()method: redis/ioredis#1998. Additionally, the hsetex was only introduced in Redis v8.0: https://redis.io/docs/latest/commands/hsetex/Testing
To test my solution I've updated DLQ test to ensure that error info is, in fact, included in the message.
Note that, each adapter has a different way of storing
headers. Tests take this into account but the devs should also keep this in mind when getting the error data from headers.Note
All error data is encoded in base64 because of special characters. For example, NATS JetStream does not support
\nor\rin headers. This means that in DLQ it's necessary to decode the info.Docs
The docs were updated to include this feature