Skip to content
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

Avoid dispatching SendMessageToTransportsEvent on redeliver #30676

Merged
merged 1 commit into from Mar 26, 2019

Conversation

Projects
None yet
4 participants
@weaverryan
Copy link
Member

commented Mar 25, 2019

Q A
Branch? master
Bug fix? yes - I think so
New feature? no
BC breaks? no (feature only on master)
Deprecations? no
Tests pass? yes
Fixed tickets none
License MIT
Doc PR a lot, soon :)

This purpose of this event is to be a hook when a message is sent to a transport.
If that message is redelivered later, that's not the purpose of this hook (there
are Worker events for that) and could cause problems if the user unknowingly
tries to modify the Envelope in some way, not thinking about how this might
be a redelivery message.

@weaverryan weaverryan referenced this pull request Mar 25, 2019

Closed

[Messenger] Making it Shine #30540

30 of 36 tasks complete
@fabpot

fabpot approved these changes Mar 25, 2019

@nicolas-grekas nicolas-grekas added this to the next milestone Mar 25, 2019

Avoid dispatching SendMessageToTransportsEvent on redeliver
This purpose of this event is to be a hook when a message is sent to a transport.
If that message is redelivered later, that's not the purpose of this hook (there
are Worker events for that) and could cause problems if the user unknowingly
tries to modify the Envelope in some way, not thinking about how this might
be a redelivery message.

@weaverryan weaverryan force-pushed the weaverryan:event-not-on-retry branch from dfd8584 to 3ac6bf9 Mar 26, 2019

@weaverryan

This comment has been minimized.

Copy link
Member Author

commented Mar 26, 2019

Ready!

@fabpot

This comment has been minimized.

Copy link
Member

commented Mar 26, 2019

Thank you @weaverryan.

@fabpot fabpot merged commit 3ac6bf9 into symfony:master Mar 26, 2019

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
fabbot.io Your code looks good.
Details

fabpot added a commit that referenced this pull request Mar 26, 2019

feature #30676 Avoid dispatching SendMessageToTransportsEvent on rede…
…liver (weaverryan)

This PR was merged into the 4.3-dev branch.

Discussion
----------

Avoid dispatching SendMessageToTransportsEvent on redeliver

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | yes - I think so
| New feature?  | no
| BC breaks?    | no (feature only on master)
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | none
| License       | MIT
| Doc PR        | a lot, soon :)

This purpose of this event is to be a hook when a message is sent to a transport.
If that message is redelivered later, that's not the purpose of this hook (there
are Worker events for that) and could cause problems if the user unknowingly
tries to modify the Envelope in some way, not thinking about how this might
be a redelivery message.

Commits
-------

3ac6bf9 Avoid dispatching SendMessageToTransportsEvent on redeliver

@weaverryan weaverryan deleted the weaverryan:event-not-on-retry branch Mar 26, 2019

@nicolas-grekas nicolas-grekas modified the milestones: next, 4.3 Apr 30, 2019

@fabpot fabpot referenced this pull request May 9, 2019

Merged

Release v4.3.0-BETA1 #31435

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.