From 41536db8280eb4c7a6b591c9af4a54b630b77588 Mon Sep 17 00:00:00 2001 From: loks0n <22452787+loks0n@users.noreply.github.com> Date: Wed, 17 Jun 2026 23:28:44 +0100 Subject: [PATCH 1/3] fix(transaction): tolerate desynced cleanup rollback on startTransaction A pooled connection can report a transaction it no longer holds after a reconnect: Swoole's PDOProxy keeps its own inTransaction counter and does not reset it in reconnect(), so it still reports an open transaction while the underlying connection has none. startTransaction()'s cleanup rollBack() then throws "There is no active transaction", which was rewrapped as "Failed to start transaction" and aborted an otherwise-valid start. Because the pool reclaims (rather than destroys) the connection on error, the desynced connection stays in rotation and every subsequent transaction on it fails the same way. Make the pre-begin cleanup best-effort: swallow a PDOException from the rollback and continue to beginTransaction() on the (clean, reconnected) connection. The utopia PDO wrapper is unaffected as it reads the real connection state; this only hardens the path when wrapped by a proxy that tracks transactions independently. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/Database/Adapter/SQL.php | 17 +++++++++----- tests/unit/SQLTransactionTest.php | 37 +++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 5 deletions(-) create mode 100644 tests/unit/SQLTransactionTest.php diff --git a/src/Database/Adapter/SQL.php b/src/Database/Adapter/SQL.php index fb949dfa48..d6729d9f4a 100644 --- a/src/Database/Adapter/SQL.php +++ b/src/Database/Adapter/SQL.php @@ -68,11 +68,18 @@ public function startTransaction(): bool { try { if ($this->inTransaction === 0) { - if ($this->getPDO()->inTransaction()) { - $this->getPDO()->rollBack(); - } else { - // If no active transaction, this has no effect. - $this->getPDO()->prepare('ROLLBACK')->execute(); + try { + if ($this->getPDO()->inTransaction()) { + $this->getPDO()->rollBack(); + } else { + // If no active transaction, this has no effect. + $this->getPDO()->prepare('ROLLBACK')->execute(); + } + } catch (PDOException) { + // A pooled connection can report a transaction it no longer + // holds after a reconnect (e.g. Swoole PDOProxy keeps its own + // counter), making this cleanup rollback throw. It is best + // effort; swallow it and begin a fresh transaction below. } $this->getPDO()->beginTransaction(); diff --git a/tests/unit/SQLTransactionTest.php b/tests/unit/SQLTransactionTest.php new file mode 100644 index 0000000000..3069f6fad3 --- /dev/null +++ b/tests/unit/SQLTransactionTest.php @@ -0,0 +1,37 @@ +getMockBuilder(\PDO::class) + ->disableOriginalConstructor() + ->getMock(); + + $pdo->method('inTransaction')->willReturn(true); + $pdo->method('rollBack')->willThrowException( + new PDOException('There is no active transaction') + ); + $pdo->expects($this->once()) + ->method('beginTransaction') + ->willReturn(true); + + $adapter = new MySQL($pdo); + + $this->assertTrue($adapter->startTransaction()); + $this->assertTrue($adapter->inTransaction()); + } +} From d9ed5451cbec9a29242164d0dc510880c904ebac Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Thu, 18 Jun 2026 15:58:27 +1200 Subject: [PATCH 2/3] test: cover transaction begin failure --- tests/unit/SQLTransactionTest.php | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/unit/SQLTransactionTest.php b/tests/unit/SQLTransactionTest.php index 3069f6fad3..d370a9896e 100644 --- a/tests/unit/SQLTransactionTest.php +++ b/tests/unit/SQLTransactionTest.php @@ -5,6 +5,7 @@ use PDOException; use PHPUnit\Framework\TestCase; use Utopia\Database\Adapter\MySQL; +use Utopia\Database\Exception\Transaction as TransactionException; class SQLTransactionTest extends TestCase { @@ -34,4 +35,26 @@ public function testStartTransactionRecoversFromDesyncedRollback(): void $this->assertTrue($adapter->startTransaction()); $this->assertTrue($adapter->inTransaction()); } + + public function testStartTransactionDoesNotMaskBeginFailureAfterDesyncedRollback(): void + { + $pdo = $this->getMockBuilder(\PDO::class) + ->disableOriginalConstructor() + ->getMock(); + + $pdo->method('inTransaction')->willReturn(true); + $pdo->method('rollBack')->willThrowException( + new PDOException('There is no active transaction') + ); + $pdo->expects($this->once()) + ->method('beginTransaction') + ->willThrowException(new PDOException('Connection lost')); + + $adapter = new MySQL($pdo); + + $this->expectException(TransactionException::class); + $this->expectExceptionMessage('Failed to start transaction: Connection lost'); + + $adapter->startTransaction(); + } } From 0913c1866dfa8aabe13e47a71435617613280865 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Thu, 18 Jun 2026 17:03:56 +1200 Subject: [PATCH 3/3] fix(transaction): tolerate postgres cleanup rollback --- src/Database/Adapter/Postgres.php | 17 ++++++++++++----- tests/unit/SQLTransactionTest.php | 21 +++++++++++++++++++++ 2 files changed, 33 insertions(+), 5 deletions(-) diff --git a/src/Database/Adapter/Postgres.php b/src/Database/Adapter/Postgres.php index 2af11aea31..7f4e5ca0dd 100644 --- a/src/Database/Adapter/Postgres.php +++ b/src/Database/Adapter/Postgres.php @@ -38,11 +38,18 @@ public function startTransaction(): bool { try { if ($this->inTransaction === 0) { - if ($this->getPDO()->inTransaction()) { - $this->getPDO()->rollBack(); - } else { - // If no active transaction, this has no effect. - $this->getPDO()->prepare('ROLLBACK')->execute(); + try { + if ($this->getPDO()->inTransaction()) { + $this->getPDO()->rollBack(); + } else { + // If no active transaction, this has no effect. + $this->getPDO()->prepare('ROLLBACK')->execute(); + } + } catch (PDOException) { + // A pooled connection can report a transaction it no longer + // holds after a reconnect (e.g. Swoole PDOProxy keeps its own + // counter), making this cleanup rollback throw. It is best + // effort; swallow it and begin a fresh transaction below. } $result = $this->getPDO()->beginTransaction(); diff --git a/tests/unit/SQLTransactionTest.php b/tests/unit/SQLTransactionTest.php index d370a9896e..6fdfe9f033 100644 --- a/tests/unit/SQLTransactionTest.php +++ b/tests/unit/SQLTransactionTest.php @@ -5,6 +5,7 @@ use PDOException; use PHPUnit\Framework\TestCase; use Utopia\Database\Adapter\MySQL; +use Utopia\Database\Adapter\Postgres; use Utopia\Database\Exception\Transaction as TransactionException; class SQLTransactionTest extends TestCase @@ -57,4 +58,24 @@ public function testStartTransactionDoesNotMaskBeginFailureAfterDesyncedRollback $adapter->startTransaction(); } + + public function testPostgresStartTransactionRecoversFromDesyncedRollback(): void + { + $pdo = $this->getMockBuilder(\PDO::class) + ->disableOriginalConstructor() + ->getMock(); + + $pdo->method('inTransaction')->willReturn(true); + $pdo->method('rollBack')->willThrowException( + new PDOException('There is no active transaction') + ); + $pdo->expects($this->once()) + ->method('beginTransaction') + ->willReturn(true); + + $adapter = new Postgres($pdo); + + $this->assertTrue($adapter->startTransaction()); + $this->assertTrue($adapter->inTransaction()); + } }