Run risky code in finally block

doctrine · Oct 11, 2024 · 2f5c77f · 2f5c77f
1 parent 61446f0
commit 2f5c77f
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 4 deletions.
diff --git a/src/Connection.php b/src/Connection.php
@@ -1278,15 +1278,20 @@ public function lastInsertId($name = null)
     public function transactional(Closure $func)
     {
         $this->beginTransaction();
+
+        $successful = false;
+
         try {
             $res = $func($this);
             $this->commit();
 
-            return $res;
-        } catch (Throwable $e) {
-            $this->rollBack();
+            $successful = true;
 
-            throw $e;
+            return $res;
+        } finally {
+            if (! $successful) {
+                $this->rollBack();
+            }
         }
     }
 

diff --git a/tests/ConnectionTest.php b/tests/ConnectionTest.php
@@ -1004,6 +1004,27 @@ public function testLegacySchemaManagerFactory(): void
         $connection = DriverManager::getConnection(['driver' => 'sqlite3', 'memory' => true]);
         self::assertInstanceOf(SqliteSchemaManager::class, $connection->createSchemaManager());
     }
+
+    public function testItPreservesTheOriginalExceptionOnRollbackFailure(): void
+    {
+        $connection = new class (['memory' => true], new Driver\SQLite3\Driver()) extends Connection {
+            public function rollBack(): void
+            {
+                throw new Exception('Rollback exception');
+            }
+        };
+
+        try {
+            $connection->transactional(static function (): void {
+                throw new Exception('Original exception');
+            });
+            self::fail('Exception expected');
+        } catch (Exception $e) {
+            self::assertSame('Rollback exception', $e->getMessage());
+            self::assertNotNull($e->getPrevious());
+            self::assertSame('Original exception', $e->getPrevious()->getMessage());
+        }
+    }
 }
 
 interface ConnectDispatchEventListener