diff --git a/src/Database/Adapter/Postgres.php b/src/Database/Adapter/Postgres.php index 211761089..dbe41420c 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/src/Database/Adapter/SQL.php b/src/Database/Adapter/SQL.php index e56678f8e..cf6822bc2 100644 --- a/src/Database/Adapter/SQL.php +++ b/src/Database/Adapter/SQL.php @@ -80,11 +80,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 000000000..6fdfe9f03 --- /dev/null +++ b/tests/unit/SQLTransactionTest.php @@ -0,0 +1,81 @@ +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()); + } + + 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(); + } + + 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()); + } +}