[Messenger] dispatch event when a message is retried #36152

nikophil · 2020-03-20T17:10:45Z

Q	A
Branch?	master
Bug fix?	no
New feature?	yes
Deprecations?	no
License	MIT

Hello,

i'm working on a bundle which helps to monitor messenger queues (add some stats for queues/transports + ability to manage failed messages from the browser)
https://github.com/SymfonyCasts/messenger-monitor-bundle/

and we're missing some hooks in the messaging system:

a way to know when a message has been retried (fixed by dispatching a new WorkerMessageRetriedEvent in SendFailedMessageForRetryListener::onMessageFailed())
a way to update the enveloppe in worker message events (fixed by adding AbstractWorkerMessageEvent::setEnvelope())

if needed i can provide some precise use cases.

thanks.

src/Symfony/Component/Messenger/EventListener/SendFailedMessageForRetryListener.php

chalasr · 2020-03-20T22:32:02Z

a way to update the enveloppe in worker message events (fixed by adding AbstractWorkerMessageEvent::setEnvelope())

Can you please provide a use case for this?

nikophil · 2020-03-21T11:35:03Z

@chalasr we're aggregating some metadata for each message (received_at, waiting_time, etc...)
thus, in order to track messages, we affect an id with a stamp once message is sent to transport leveraging SendMessageToTransportsEvent::setEnvelope().

But if a message is sent by another system (eg: in a micro-services architecture), we cannot use SendMessageToTransportsEvent and need another hook to add this id.

chalasr · 2020-03-21T13:42:20Z

@nikophil Works for me, thanks.

Can you add a CHANGELOG entry for this?

Tobion

-1 on this feature as people can just use the WorkerMessageFailedEvent with a priority that comes after the SendFailedMessageForRetryListener

Tobion · 2020-03-21T15:02:28Z

src/Symfony/Component/Messenger/Event/AbstractWorkerMessageEvent.php

@@ -29,6 +29,11 @@ public function getEnvelope(): Envelope
        return $this->envelope;
    }

+    public function setEnvelope(Envelope $envelope)
+    {
+        $this->envelope = $envelope;


making the envelope mutable means, all the dispatching of events would need to be changed to refetch the potentially modified envelope, e.g. in

symfony/src/Symfony/Component/Messenger/Worker.php

Lines 105 to 113 in 7ab51f3

$event = new WorkerMessageReceivedEvent($envelope, $transportName);

$this->dispatchEvent($event);

if (!$event->shouldHandle()) {

return;

}

try {

$envelope = $this->bus->dispatch($envelope->with(new ReceivedStamp($transportName), new ConsumedByWorkerStamp()));

just adding a setter does not do anything by itself. -1 on doing this.

Yes, this part is missing. But do you see it as a huge problem to (effectively) add $envelope = $event->getEnvelope()? I made the events immutable originally because I feel that it's always better to be conservative until we have a use-case. But this PR now does have a use-case.

sorry i missed that...
in case this PR is accepted, i've fixed that and refetched envelope from mutable events.
i've added two new tests cases which check that events mutability is handled

Did we need to add this in the abstract class? This will affect (on master currently) 3 classes. Maybe we need hook points in all of them? But in this PR, we only need a hook point in one of them.

ok, i'll do that, it will be less error prone this way

weaverryan · 2020-05-03T17:07:21Z

I'm still supportive of this feature, but we would need @Tobion to reconsider his vote :)

fabpot · 2020-08-17T12:36:28Z

@Tobion Can you have another look at this PR?

src/Symfony/Component/Messenger/Tests/WorkerTest.php

src/Symfony/Component/Messenger/CHANGELOG.md

src/Symfony/Component/Messenger/Event/WorkerMessageReceivedEvent.php

src/Symfony/Component/Messenger/Event/WorkerMessageRetriedEvent.php

src/Symfony/Component/Messenger/EventListener/SendFailedMessageForRetryListener.php

fabpot · 2020-09-06T07:12:35Z

@nikophil Still willing to finish this PR?

src/Symfony/Bundle/FrameworkBundle/Resources/config/messenger.xml

nikophil · 2020-09-06T12:28:10Z

Hi, yes sorry, i'll do that shortly

nikophil · 2020-10-01T07:53:30Z

@fabpot @Tobion i've fixed the PR, sorry for the delay

src/Symfony/Component/Messenger/CHANGELOG.md

src/Symfony/Component/Messenger/Event/WorkerMessageReceivedEvent.php

src/Symfony/Component/Messenger/Worker.php

weaverryan

This is ready - it duplicates some logic from #32904, as both PR's needed the same extension point.

fabpot · 2020-10-02T05:48:12Z

Thank you @nikophil.

nikophil requested a review from sroze as a code owner March 20, 2020 17:10

carsonbot added Status: Needs Review Messenger Feature labels Mar 20, 2020

weaverryan reviewed Mar 20, 2020

View reviewed changes

src/Symfony/Component/Messenger/EventListener/SendFailedMessageForRetryListener.php Show resolved Hide resolved

chalasr added this to the next milestone Mar 20, 2020

Tobion suggested changes Mar 21, 2020

View reviewed changes

carsonbot added Status: Needs Work and removed Status: Needs Review labels Mar 21, 2020

nikophil mentioned this pull request Mar 22, 2020

Handle messages comming from "outside" SymfonyCasts/messenger-monitor-bundle#6

Open

Tobion suggested changes Aug 17, 2020

View reviewed changes

fabpot requested changes Sep 6, 2020

View reviewed changes

src/Symfony/Bundle/FrameworkBundle/Resources/config/messenger.xml Outdated Show resolved Hide resolved

weaverryan requested changes Oct 1, 2020

View reviewed changes

src/Symfony/Component/Messenger/CHANGELOG.md Outdated Show resolved Hide resolved

src/Symfony/Component/Messenger/Event/WorkerMessageReceivedEvent.php Outdated Show resolved Hide resolved

src/Symfony/Component/Messenger/Worker.php Show resolved Hide resolved

weaverryan mentioned this pull request Oct 1, 2020

[Messenger] Added ErrorDetailsStamp #32904

Merged

6 tasks

weaverryan approved these changes Oct 1, 2020

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Work labels Oct 1, 2020

fabpot approved these changes Oct 2, 2020

View reviewed changes

[Messenger] dispatch event when a message is retried

55bddcb

fabpot mentioned this pull request Oct 2, 2020

[Messenger] dispatch event when a message is retried symfony/symfony-docs#14318

Closed

fabpot merged commit 14942db into symfony:master Oct 2, 2020

nicolas-grekas modified the milestones: next, 5.2 Oct 5, 2020

fabpot mentioned this pull request Oct 5, 2020

Release v5.2.0-BETA1 #38417

Merged

HypeMC mentioned this pull request Jun 27, 2023

[Messenger] WorkerMessageReceivedEvent::setEnvelope was never added #50799

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Messenger] dispatch event when a message is retried #36152

[Messenger] dispatch event when a message is retried #36152

nikophil commented Mar 20, 2020

chalasr commented Mar 20, 2020

nikophil commented Mar 21, 2020 •

edited

Loading

chalasr commented Mar 21, 2020 •

edited

Loading

Tobion left a comment

Tobion Mar 21, 2020 •

edited

Loading

weaverryan Mar 21, 2020

nikophil Mar 22, 2020 •

edited

Loading

weaverryan Mar 27, 2020

nikophil Mar 27, 2020

nikophil Mar 28, 2020

weaverryan commented May 3, 2020

fabpot commented Aug 17, 2020

fabpot commented Sep 6, 2020

nikophil commented Sep 6, 2020

nikophil commented Oct 1, 2020

weaverryan left a comment

fabpot commented Oct 2, 2020

	$event = new WorkerMessageReceivedEvent($envelope, $transportName);
	$this->dispatchEvent($event);

	if (!$event->shouldHandle()) {
	return;
	}

	try {
	$envelope = $this->bus->dispatch($envelope->with(new ReceivedStamp($transportName), new ConsumedByWorkerStamp()));

[Messenger] dispatch event when a message is retried #36152

[Messenger] dispatch event when a message is retried #36152

Conversation

nikophil commented Mar 20, 2020

chalasr commented Mar 20, 2020

nikophil commented Mar 21, 2020 • edited Loading

chalasr commented Mar 21, 2020 • edited Loading

Tobion left a comment

Choose a reason for hiding this comment

Tobion Mar 21, 2020 • edited Loading

Choose a reason for hiding this comment

weaverryan Mar 21, 2020

Choose a reason for hiding this comment

nikophil Mar 22, 2020 • edited Loading

Choose a reason for hiding this comment

weaverryan Mar 27, 2020

Choose a reason for hiding this comment

nikophil Mar 27, 2020

Choose a reason for hiding this comment

nikophil Mar 28, 2020

Choose a reason for hiding this comment

weaverryan commented May 3, 2020

fabpot commented Aug 17, 2020

fabpot commented Sep 6, 2020

nikophil commented Sep 6, 2020

nikophil commented Oct 1, 2020

weaverryan left a comment

Choose a reason for hiding this comment

fabpot commented Oct 2, 2020

nikophil commented Mar 21, 2020 •

edited

Loading

chalasr commented Mar 21, 2020 •

edited

Loading

Tobion Mar 21, 2020 •

edited

Loading

nikophil Mar 22, 2020 •

edited

Loading