From d726110b8ab6628244438478cad7894ae87373ba Mon Sep 17 00:00:00 2001 From: Claudear <262350598+claudear@users.noreply.github.com> Date: Mon, 15 Jun 2026 09:39:07 +0000 Subject: [PATCH] fix: catch stale-rollback PDOException in startTransaction() When a persistent PDO connection's transaction state is out of sync with the server (e.g. implicit commit, server-side timeout, connection reset), the defensive rollBack() in startTransaction() would throw "There is no active transaction" and prevent any new transaction from being created. This wraps the cleanup rollback in a try-catch so the failure is silently ignored and a fresh transaction can proceed. Also ensures withTransaction() resets the inTransaction counter immediately when rollbackTransaction() fails during a retry-eligible attempt, preventing the next startTransaction() from incorrectly taking the savepoint branch. Co-Authored-By: Claude Opus 4.6 --- src/Database/Adapter.php | 3 +- src/Database/Adapter/Postgres.php | 15 +++-- src/Database/Adapter/SQL.php | 15 +++-- tests/unit/TransactionTest.php | 92 +++++++++++++++++++++++++++++++ 4 files changed, 114 insertions(+), 11 deletions(-) create mode 100644 tests/unit/TransactionTest.php diff --git a/src/Database/Adapter.php b/src/Database/Adapter.php index ef0598341..6250ef399 100644 --- a/src/Database/Adapter.php +++ b/src/Database/Adapter.php @@ -436,12 +436,13 @@ public function withTransaction(callable $callback): mixed try { $this->rollbackTransaction(); } catch (\Throwable $rollback) { + $this->inTransaction = 0; + if ($attempts < $retries) { \usleep($sleep * ($attempts + 1)); continue; } - $this->inTransaction = 0; throw $rollback; } diff --git a/src/Database/Adapter/Postgres.php b/src/Database/Adapter/Postgres.php index d81cdec0b..76d598537 100644 --- a/src/Database/Adapter/Postgres.php +++ b/src/Database/Adapter/Postgres.php @@ -38,11 +38,16 @@ 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 { + $this->getPDO()->prepare('ROLLBACK')->execute(); + } + } catch (PDOException) { + // Defensive cleanup — if there is nothing to roll back + // (stale persistent connection, implicit commit, etc.) + // we can safely ignore the failure and proceed. } $result = $this->getPDO()->beginTransaction(); diff --git a/src/Database/Adapter/SQL.php b/src/Database/Adapter/SQL.php index e56678f8e..bc470f97d 100644 --- a/src/Database/Adapter/SQL.php +++ b/src/Database/Adapter/SQL.php @@ -80,11 +80,16 @@ 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 { + $this->getPDO()->prepare('ROLLBACK')->execute(); + } + } catch (PDOException) { + // Defensive cleanup — if there is nothing to roll back + // (stale persistent connection, implicit commit, etc.) + // we can safely ignore the failure and proceed. } $this->getPDO()->beginTransaction(); diff --git a/tests/unit/TransactionTest.php b/tests/unit/TransactionTest.php new file mode 100644 index 000000000..19a07dbdf --- /dev/null +++ b/tests/unit/TransactionTest.php @@ -0,0 +1,92 @@ +createMock(PDO::class); + + // PDO thinks a transaction is active, but rollBack() disagrees + $pdoMock->method('inTransaction') + ->willReturn(true); + + $pdoMock->method('rollBack') + ->willThrowException(new PDOException('There is no active transaction')); + + $pdoMock->expects($this->once()) + ->method('beginTransaction') + ->willReturn(true); + + $adapter = new MariaDB($pdoMock); + + $result = $adapter->startTransaction(); + + $this->assertTrue($result); + } + + /** + * Regression: when rollbackTransaction() fails during a withTransaction() + * retry, the inTransaction counter must be reset to 0 before the next + * attempt so startTransaction() takes the fresh-transaction path. + */ + public function testWithTransactionResetsCounterOnRollbackFailureDuringRetry(): void + { + $pdoMock = $this->createMock(PDO::class); + $stmtMock = $this->createMock(PDOStatement::class); + $stmtMock->method('execute')->willReturn(true); + + $callCount = 0; + + // First call: beginTransaction succeeds, then callback fails, + // then rollBack also fails (simulating connection issue). + // Second call: fresh beginTransaction succeeds, callback succeeds. + $pdoMock->method('inTransaction') + ->willReturnCallback(function () use (&$callCount) { + // After the first failed rollback, PDO no longer thinks + // it's in a transaction. + return false; + }); + + $pdoMock->method('prepare') + ->willReturn($stmtMock); + + $pdoMock->method('beginTransaction') + ->willReturn(true); + + $pdoMock->method('commit') + ->willReturn(true); + + // rollBack fails the first time (simulating stale transaction) + $pdoMock->method('rollBack') + ->willThrowException(new PDOException('There is no active transaction')); + + $adapter = new MariaDB($pdoMock); + + $result = $adapter->withTransaction(function () use (&$callCount) { + $callCount++; + if ($callCount === 1) { + throw new \RuntimeException('Transient error'); + } + return 'success'; + }); + + $this->assertSame('success', $result); + $this->assertSame(2, $callCount); + } +}