-
-
Notifications
You must be signed in to change notification settings - Fork 9.8k
[Messenger] Allow to respect retry strategy with RecoverableMessageHandlingException #61936
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
base: 8.1
Are you sure you want to change the base?
Conversation
4fd9606 to
c8be16f
Compare
src/Symfony/Component/Messenger/Exception/RecoverableExceptionInterface.php
Outdated
Show resolved
Hide resolved
src/Symfony/Component/Messenger/Exception/RecoverableMessageHandlingException.php
Outdated
Show resolved
Hide resolved
nicolas-grekas
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.
the changelog and upgrade files should mention the new method
f18a81c to
9559f73
Compare
|
Thanks for the review @nicolas-grekas ; I updated the PR. Failure seems unrelated since they are redis-related |
9559f73 to
9beab3a
Compare
|
Friendly ping @nicolas-grekas for a small review :) This was tagged as |
9beab3a to
1dd27a9
Compare
1dd27a9 to
b2185b2
Compare
b2185b2 to
3274e58
Compare
|
I rebased, ready for a new review @nicolas-grekas :) |
Since
getRetryDelaywas added, theRecoverableMessageHandlingExceptionis useful to override the defaut delay of the retry strategy (for instance if we get rate limited by some thing and would like to retry after some delay). BUT this imply the message will be retried again and again if we keep getting such error.#49063 asked RecoverableMessageHandlingException to respect the retryStrategy and #50655 revert it because if
RecoverableMessageHandlingExceptionrespect the retry strategy it has no use. Now I think this feature would be useful, as an option: