Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a guard to prevent multiple task handlers from acknowledging or rejecting the same AMQP message if it has already been handled. By checking the amqpHandled state within the ack and reject methods, the implementation avoids duplicate side effects such as multiple tasks being sent to the Dead Letter Queue. Additionally, the edge case tests were refactored to use the public dequeue method and now include assertions to verify that messages are only sent to the queue once. I have no feedback to provide.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 15c86b9f91
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const context: TaskContext = { | ||
| ack: async () => { | ||
| if (acknowledged || rejected || !this._active) { | ||
| if (acknowledged || rejected || !this._active || isAmqpHandled()) { |
There was a problem hiding this comment.
Preserve attempt-count cleanup after AMQP is handled
processTask increments _attemptCounts before handler execution, but with this new isAmqpHandled() early return in ctx.ack, any later handler on the same message will skip cleanup once an earlier handler has already acked/nacked. In multi-handler queues, that leaves a new _attemptCounts entry behind for an already-completed task, causing unbounded map growth over time (and stale retry metadata for that task id). The guard should avoid duplicate AMQP side effects without bypassing local bookkeeping cleanup.
Useful? React with 👍 / 👎.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #186 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 10 10
Lines 1264 1265 +1
Branches 230 230
=========================================
+ Hits 1264 1265 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Please check if the PR fulfills these requirements
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
rabbitmq - chore: test cleanup