bug symfony#48120 [Messenger] Do not throw 'no handlers' exception when skipping handlers due to duplicate handling (wouterj)

chalasr · chalasr · commit 1fd4b630bcf0 · 2022-11-08T13:44:34.000+01:00
This PR was merged into the 4.4 branch. Discussion ---------- [Messenger] Do not throw 'no handlers' exception when skipping handlers due to duplicate handling | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | n/a I'm not sure if this change is expected or not, but I found this [while investigating another bug](https://github.com/wouterj/sf-reproducer/tree/messenger-star-routing in the Messenger component. When you handle the same message twice, you'll get an `NoHandlerForMessageException` because all handlers are skipped. What would be the expected behavior? Maybe we have to throw a different exception when the same envelope is handled twice? Commits ------- 0e4455b [Messenger] Do not throw 'no handlers' exception when skipping due to duplicate handling
diff --git a/src/Symfony/Component/Messenger/Middleware/HandleMessageMiddleware.php b/src/Symfony/Component/Messenger/Middleware/HandleMessageMiddleware.php
@@ -53,8 +53,10 @@ public function handle(Envelope $envelope, StackInterface $stack): Envelope
         ];
 
         $exceptions = [];
+        $alreadyHandled = false;
         foreach ($this->handlersLocator->getHandlers($envelope) as $handlerDescriptor) {
             if ($this->messageHasAlreadyBeenHandled($envelope, $handlerDescriptor)) {
+                $alreadyHandled = true;
                 continue;
             }
 
@@ -68,7 +70,7 @@ public function handle(Envelope $envelope, StackInterface $stack): Envelope
             }
         }
 
-        if (null === $handler) {
+        if (null === $handler && !$alreadyHandled) {
             if (!$this->allowNoHandlers) {
                 throw new NoHandlerForMessageException(sprintf('No handler for message "%s".', $context['class']));
             }
diff --git a/src/Symfony/Component/Messenger/Tests/Middleware/HandleMessageMiddlewareTest.php b/src/Symfony/Component/Messenger/Tests/Middleware/HandleMessageMiddlewareTest.php
@@ -123,6 +123,24 @@ public function testThrowsNoHandlerException()
         $middleware->handle(new Envelope(new DummyMessage('Hey')), new StackMiddleware());
     }
 
+    public function testMessageAlreadyHandled()
+    {
+        $handler = $this->createPartialMock(HandleMessageMiddlewareTestCallable::class, ['__invoke']);
+
+        $middleware = new HandleMessageMiddleware(new HandlersLocator([
+            DummyMessage::class => [$handler],
+        ]));
+
+        $envelope = new Envelope(new DummyMessage('Hey'));
+
+        $envelope = $middleware->handle($envelope, $this->getStackMock());
+        $handledStamp = $envelope->all(HandledStamp::class);
+
+        $envelope = $middleware->handle($envelope, $this->getStackMock());
+
+        $this->assertSame($envelope->all(HandledStamp::class), $handledStamp);
+    }
+
     public function testAllowNoHandlers()
     {
         $middleware = new HandleMessageMiddleware(new HandlersLocator([]), true);

Original file line number	Diff line number	Diff line change
`@@ -53,8 +53,10 @@ public function handle(Envelope $envelope, StackInterface $stack): Envelope`
`53`	`53`	`];`
`54`	`54`
`55`	`55`	`$exceptions = [];`
	`56`	`+ $alreadyHandled = false;`
`56`	`57`	`foreach ($this->handlersLocator->getHandlers($envelope) as $handlerDescriptor) {`
`57`	`58`	`if ($this->messageHasAlreadyBeenHandled($envelope, $handlerDescriptor)) {`
	`59`	`+ $alreadyHandled = true;`
`58`	`60`	`continue;`
`59`	`61`	`}`
`60`	`62`
`@@ -68,7 +70,7 @@ public function handle(Envelope $envelope, StackInterface $stack): Envelope`
`68`	`70`	`}`
`69`	`71`	`}`
`70`	`72`
`71`		`- if (null === $handler) {`
	`73`	`+ if (null === $handler && !$alreadyHandled) {`
`72`	`74`	`if (!$this->allowNoHandlers) {`
`73`	`75`	`throw new NoHandlerForMessageException(sprintf('No handler for message "%s".', $context['class']));`
`74`	`76`	`}`