bug symfony#59103 [Messenger] ensure exception on rollback does not hide previous exception (nikophil)

This PR was merged into the 6.4 branch.

Discussion
----------

[Messenger] ensure exception on rollback does not hide previous exception

| Q             | A
| ------------- | ---
| Branch?       | 6.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| License       | MIT

Hello,

this is kinda related to doctrine/orm#11230 and doctrine/orm#11646

I propose to use the same logic in `DoctrineTransactionMiddleware` than in doctrine/orm#11646 towards rollback: currently if an error occurs in the rollback (for example, the infamous `SAVEPOINT DOCTRINE_2 does not exist`), the previous error is hidden, because the error should not occur in the `catch`.

ping `@ro0NL`

_EDIt: failure in CI is unrelated to this PR_

Commits
-------

f38d4b0 [Messenger] ensure exception on rollback does not hide previous exception

Author: fabpot

src/Symfony/Bridge/Doctrine/Messenger/DoctrineTransactionMiddleware.php

@@ -27,22 +27,30 @@ 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.
                 throw new HandlerFailedException($exception->getEnvelope()->withoutAll(HandledStamp::class), $exception->getWrappedExceptions());
             }
 
             throw $exception;
+        } finally {
+            $connection = $entityManager->getConnection();
+
+            if (!$success && $connection->isTransactionActive()) {
+                $connection->rollBack();
+            }
         }
     }
 }

src/Symfony/Bridge/Doctrine/Tests/Messenger/DoctrineTransactionMiddlewareTest.php

@@ -56,19 +56,37 @@ 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.');
 
         $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);