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

Author: nikophil

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();
+            }
         }
     }
 }

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);