[Messenger] Fix exception message of failed message is dropped on retry#33600
Merged
fabpot merged 1 commit intosymfony:4.3from Sep 17, 2019
tienvx:fix-exception-message-dropped
Merged
[Messenger] Fix exception message of failed message is dropped on retry#33600fabpot merged 1 commit intosymfony:4.3from tienvx:fix-exception-message-dropped
fabpot merged 1 commit intosymfony:4.3from
tienvx:fix-exception-message-dropped
Conversation
Contributor
Author
|
NOTE: The test case will failed on Symfony 4.4, because component |
fabpot
requested changes
Sep 17, 2019
src/Symfony/Component/Messenger/Tests/Command/FailedMessagesRetryCommandTest.php
Outdated
Show resolved
Hide resolved
src/Symfony/Component/Messenger/Tests/Command/FailedMessagesRetryCommandTest.php
Outdated
Show resolved
Hide resolved
src/Symfony/Component/Messenger/Tests/Command/FailedMessagesRetryCommandTest.php
Outdated
Show resolved
Hide resolved
tienvx
commented
Sep 17, 2019
src/Symfony/Component/Messenger/Tests/Command/FailedMessagesRetryCommandTest.php
Show resolved
Hide resolved
fabpot
approved these changes
Sep 17, 2019
Member
|
Thank you @tienvx. |
fabpot
added a commit
that referenced
this pull request
Sep 17, 2019
…pped on retry (tienvx) This PR was merged into the 4.3 branch. Discussion ---------- [Messenger] Fix exception message of failed message is dropped on retry | Q | A | ------------- | --- | Branch? | 4.3 <!-- see below --> | Bug fix? | yes | New feature? | no <!-- please update src/**/CHANGELOG.md files --> | Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files --> | Tickets | Fix #32719 | License | MIT | Doc PR | NA <!-- required for new features --> <!-- Replace this notice by a short README for your feature/bugfix. This will help people understand your PR and can be used as a start for the documentation. Additionally (see https://symfony.com/roadmap): - Always add tests and ensure they pass. - Never break backward compatibility (see https://symfony.com/bc). - Bug fixes must be submitted against the lowest maintained branch where they apply (lowest branches are regularly merged to upper ones so they get the fixes too.) - Features and deprecations must be submitted against branch 4.4. - Legacy code removals go to the master branch. --> Commits ------- 8f9f44e [Messenger] Fix exception message of failed message is dropped on retry
Contributor
|
This fixes #32311 and invalidates #32341 where @weaverryan wanted to go with a different solution. |
Member
|
This most likely also invalidates #32904, unless the separation is still something we want to do in the future. |
Contributor
Author
Member
|
I wasn't aware of the other issues/PRs; tell me how you want to proceed @weaverryan |
Merged
Contributor
|
We need to revert this in #34082 as it's breaking the retry on amqp when the message gets too big. |
Tobion
added a commit
that referenced
this pull request
Oct 23, 2019
…e is dropped (Tobion) This PR was merged into the 4.3 branch. Discussion ---------- Revert "[Messenger] Fix exception message of failed message is dropped | Q | A | ------------- | --- | Branch? | 4.3 | Bug fix? | yes | New feature? | no <!-- please update src/**/CHANGELOG.md files --> | Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files --> | Tickets | | License | MIT | Doc PR | This reverts #33600 because it makes the message grow for each retry until AMQP cannot handle it anymore. On each retry, the full exception trace is added to the message. So in our case on the 5th retry, the message is too big for the AMQP library to encode it. AMQP extension then throws the exception > Library error: table too large for buffer (ref. alanxz/rabbitmq-c#224 and php-amqp/php-amqp#131) when trying to publish the message. To solve this, I suggest to revert #33600 (this PR) and merge #32341 instead which does not re-add the exception on each failure. Btw, the above problem causes other problematic side-effects of Symfony messenger. As the new retry message fails to be published with an exception, the old (currently processed message) also does not get removed (acknowledged) from the delay queue. So rabbitmq redelivers the message and the same thing happens forever. This can block the consumers and have a huge toll on your service. That's just another case for #32055 (comment). I'll try to fix this in another PR. Commits ------- 3dbe924 Revert "[Messenger] Fix exception message of failed message is dropped on retry"
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.