From f38d4b07fafcf9b198597ef07ec971512dcf7eaa Mon Sep 17 00:00:00 2001
From: Nicolas PHILIPPE <nikophil@gmail.com>
Date: Thu, 5 Dec 2024 18:36:22 +0100
Subject: [PATCH] [Messenger] ensure exception on rollback does not hide
 previous exception

---
 .../DoctrineTransactionMiddleware.php         | 12 ++++++--
 .../DoctrineTransactionMiddlewareTest.php     | 30 +++++++++++++++----
 2 files changed, 34 insertions(+), 8 deletions(-)

diff --git a/src/Symfony/Bridge/Doctrine/Messenger/DoctrineTransactionMiddleware.php b/src/Symfony/Bridge/Doctrine/Messenger/DoctrineTransactionMiddleware.php
index e4831557f01d..8e10891b0ba7 100644
--- a/src/Symfony/Bridge/Doctrine/Messenger/DoctrineTransactionMiddleware.php
+++ b/src/Symfony/Bridge/Doctrine/Messenger/DoctrineTransactionMiddleware.php
@@ -27,15 +27,17 @@ class DoctrineTransactionMiddleware extends AbstractDoctrineMiddleware
     protected function handleForManager(EntityManagerInterface $entityManager, Envelope $envelope, StackInterface $stack): Envelope
     {
         $entityManager->getConnection()->beginTransaction();
+
+        $success = false;
         try {
             $envelope = $stack->next()->handle($envelope, $stack);
             $entityManager->flush();
             $entityManager->getConnection()->commit();
 
+            $success = true;
+
             return $envelope;
         } catch (\Throwable $exception) {
-            $entityManager->getConnection()->rollBack();
-
             if ($exception instanceof HandlerFailedException) {
                 // Remove all HandledStamp from the envelope so the retry will execute all handlers again.
                 // When a handler fails, the queries of allegedly successful previous handlers just got rolled back.
@@ -43,6 +45,12 @@ protected function handleForManager(EntityManagerInterface $entityManager, Envel
             }
 
             throw $exception;
+        } finally {
+            $connection = $entityManager->getConnection();
+
+            if (!$success && $connection->isTransactionActive()) {
+                $connection->rollBack();
+            }
         }
     }
 }
diff --git a/src/Symfony/Bridge/Doctrine/Tests/Messenger/DoctrineTransactionMiddlewareTest.php b/src/Symfony/Bridge/Doctrine/Tests/Messenger/DoctrineTransactionMiddlewareTest.php
index 977f32e30fa6..05e5dae1b34a 100644
--- a/src/Symfony/Bridge/Doctrine/Tests/Messenger/DoctrineTransactionMiddlewareTest.php
+++ b/src/Symfony/Bridge/Doctrine/Tests/Messenger/DoctrineTransactionMiddlewareTest.php
@@ -56,12 +56,9 @@ public function testMiddlewareWrapsInTransactionAndFlushes()
 
     public function testTransactionIsRolledBackOnException()
     {
-        $this->connection->expects($this->once())
-            ->method('beginTransaction')
-        ;
-        $this->connection->expects($this->once())
-            ->method('rollBack')
-        ;
+        $this->connection->expects($this->once())->method('beginTransaction');
+        $this->connection->expects($this->once())->method('isTransactionActive')->willReturn(true);
+        $this->connection->expects($this->once())->method('rollBack');
 
         $this->expectException(\RuntimeException::class);
         $this->expectExceptionMessage('Thrown from next middleware.');
@@ -69,6 +66,27 @@ public function testTransactionIsRolledBackOnException()
         $this->middleware->handle(new Envelope(new \stdClass()), $this->getThrowingStackMock());
     }
 
+    public function testExceptionInRollBackDoesNotHidePreviousException()
+    {
+        $this->connection->expects($this->once())->method('beginTransaction');
+        $this->connection->expects($this->once())->method('isTransactionActive')->willReturn(true);
+        $this->connection->expects($this->once())->method('rollBack')->willThrowException(new \RuntimeException('Thrown from rollBack.'));
+
+        try {
+            $this->middleware->handle(new Envelope(new \stdClass()), $this->getThrowingStackMock());
+        } catch (\Throwable $exception) {
+        }
+
+        self::assertNotNull($exception);
+        self::assertInstanceOf(\RuntimeException::class, $exception);
+        self::assertSame('Thrown from rollBack.', $exception->getMessage());
+
+        $previous = $exception->getPrevious();
+        self::assertNotNull($previous);
+        self::assertInstanceOf(\RuntimeException::class, $previous);
+        self::assertSame('Thrown from next middleware.', $previous->getMessage());
+    }
+
     public function testInvalidEntityManagerThrowsException()
     {
         $managerRegistry = $this->createMock(ManagerRegistry::class);