Skip to content

fix: prevent looping on poisoned messages (if worker crashes) - #18

Merged
GuillaumeDecMeetsMore merged 9 commits into
mainfrom
guillaume/fix/prevent-looping-on-poisoned-messages
Aug 7, 2026
Merged

GuillaumeDecMeetsMore merged 9 commits into
mainfrom
guillaume/fix/prevent-looping-on-poisoned-messages

Conversation

@GuillaumeDecMeetsMore

@GuillaumeDecMeetsMore GuillaumeDecMeetsMore commented Aug 5, 2026 •

Copy link
Copy Markdown
Collaborator
  • Fix poisoned messages not being checked before starting giving messages to subscribers

Tested locally on our use case

@GuillaumeDecMeetsMore GuillaumeDecMeetsMore self-assigned this Aug 5, 2026
@GuillaumeDecMeetsMore
GuillaumeDecMeetsMore marked this pull request as ready for review August 5, 2026 08:28
Comment thread packages/matador/src/retry/policy.ts Outdated
Comment thread packages/matador/src/retry/policy.ts Outdated

@mm-zacharydavison mm-zacharydavison left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

minor naming

Comment thread packages/matador/src/retry/policy.ts Outdated
* Decision for a poisoned message
*/
export type PoisonedMessageDecision =
| { action: 'not-poisoned' }

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

a bit of a semantic nit, but i feel like action should be a clear indication of what happens, which dead-letter and discard are, but not-poisoned is a state

either we define the decision for all 3 as "action to be taken", or if action to be taken is delegated outside of matador, then we define state for all 3

this is a bit pedantic on my part though

(also, do we use deadletter or dead-letter in the codebase?)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, I wasn't/I am not really sure what to use there. Initially, I was tempted to use continue or pass-through as the action, but both of them don't really work in both cases:

  • In the precheck flow, "continue" and "pass-through" make sense
  • In the shouldRetry flow (after getting an error), it makes less sense. We don't really continue there, but that wording seem to suggest that "we don't care about the error and do nothing", which we don't have as a flow 😕

For deadletter vs dead-letter, I've used the same wording as the RetryDecision just above. But it does look like we use dead-letter as well in other places 🤔

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

maybe just not using the term action is enough

{ type: `ok` | `deadletter` | `discard` }

lord knows why im making this nitpick when code probably wont exist in 12 months 😅

* refactor precheck naming

* reduce code duplication for failure handling
@GuillaumeDecMeetsMore
GuillaumeDecMeetsMore merged commit b6ac4c6 into main Aug 7, 2026
4 checks passed
@GuillaumeDecMeetsMore
GuillaumeDecMeetsMore deleted the guillaume/fix/prevent-looping-on-poisoned-messages branch August 7, 2026 07:23
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.

3 participants