Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Fixing a bug where messenger:consume could send message to wrong bus#30652

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed

Conversation

@weaverryan
Copy link
Member

@weaverryanweaverryan commentedMar 22, 2019•
edited
Loading

QA
Branch?master
Bug fix?yes
New feature?arguably, yes
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed tickets#30631
LicenseMIT
Doc PRNot needed

Thisfixes#30631, where you can runmessener:consume and accidentally sent received messages into the wrong bus.

The fix (done via middleware) is to attach a "bus name" to theEnvelope and use it when the message is received to find that bus.

@weaverryanweaverryan changed the titleFixing a bug where a transport could receive a message and dispatch i…Fixing a bug where messenger:consume could send message to wrong busMar 22, 2019
@chalasrchalasr added this to thenext milestoneMar 22, 2019
@weaverryanweaverryanforce-pushed thebus-name-on-envelope branch 4 times, most recently from41069af toa4f9bf3CompareMarch 22, 2019 19:17
@weaverryan
Copy link
MemberAuthor

This is ready to go!

Copy link
Contributor

@srozesroze left a comment

Choose a reason for hiding this comment

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

Great, indeed. I was aiming to write such a PR with me (but this small change) 👌

Copy link
Member

@chalasrchalasr left a comment

Choose a reason for hiding this comment

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

👍

@srozesrozeforce-pushed thebus-name-on-envelope branch froma94ce84 toef077cfCompareMarch 23, 2019 14:30
@fabpot
Copy link
Member

Thank you@weaverryan.

fabpot added a commit that referenced this pull requestMar 23, 2019
…e to wrong bus (weaverryan)This PR was merged into the 4.3-dev branch.Discussion----------Fixing a bug where messenger:consume could send message to wrong bus| Q             | A| ------------- | ---| Branch?       | master| Bug fix?      | yes| New feature?  | arguably, yes| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets |#30631| License       | MIT| Doc PR        | Not neededThisfixes#30631, where you can run `messener:consume` and accidentally sent received messages into the wrong bus.The fix (done via middleware) is to attach a "bus name" to the `Envelope` and use it when the message is received to find that bus.Commits-------ef077cf Fixing a bug where a transport could receive a message and dispatch it to a different bus
@sroze
Copy link
Contributor

Merged bye512b7e but GitHub couldn't recognise it because I pushed another commit in this branch in the meantime, closing.

@srozesroze closed thisMar 23, 2019
@weaverryanweaverryan deleted the bus-name-on-envelope branchMarch 23, 2019 18:22
@raress96
Copy link

Just a thought on this, I think it is a BC Break because if you have an application running SF 4.2 which has published a message that will then be consumed by an application running SF 4.3, you will get the error:
Envelope does not contain a BusNameStamp.

Hence I think it is a BC break. Moreover, if for example, some messages exist in a RabbitMQ queue and you updated your app to SF 4.3, then those messages can no longer be processed because they do not have this stamp. I am proposing that if the stamp is not present then the default bus to be used to handle them.

@sroze
Copy link
Contributor

I am proposing that if the stamp is not present then the default bus to be used to handle them.

@raresserban The component being experimental, we allow BC breaks. Though, I agree with you, it doesn't cost much effort to do so. Can you submit a pull request? Thank you!

weaverryan reacted with thumbs up emoji

@weaverryan
Copy link
MemberAuthor

Yes, I agree. I've been trying to "keep in mind" (at least) the migration path for messages that are inside a queue at the moment of upgrading.

See#31200

@nicolas-grekasnicolas-grekas modified the milestones:next,4.3Apr 30, 2019
@fabpotfabpot mentioned this pull requestMay 9, 2019
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@chalasrchalasrchalasr requested changes

@fabpotfabpotfabpot approved these changes

+1 more reviewer

@srozesrozesroze approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

4.3

Development

Successfully merging this pull request may close these issues.

[Messenger] messenger:consume - bus should be determined automatically

7 participants

@weaverryan@fabpot@sroze@raress96@chalasr@nicolas-grekas@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp