Skip to content

Commit 41536db

Browse files
loks0nclaude
andcommitted
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) <noreply@anthropic.com>
1 parent 7e84398 commit 41536db

2 files changed

Lines changed: 49 additions & 5 deletions

File tree

src/Database/Adapter/SQL.php

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -68,11 +68,18 @@ public function startTransaction(): bool
6868
{
6969
try {
7070
if ($this->inTransaction === 0) {
71-
if ($this->getPDO()->inTransaction()) {
72-
$this->getPDO()->rollBack();
73-
} else {
74-
// If no active transaction, this has no effect.
75-
$this->getPDO()->prepare('ROLLBACK')->execute();
71+
try {
72+
if ($this->getPDO()->inTransaction()) {
73+
$this->getPDO()->rollBack();
74+
} else {
75+
// If no active transaction, this has no effect.
76+
$this->getPDO()->prepare('ROLLBACK')->execute();
77+
}
78+
} catch (PDOException) {
79+
// A pooled connection can report a transaction it no longer
80+
// holds after a reconnect (e.g. Swoole PDOProxy keeps its own
81+
// counter), making this cleanup rollback throw. It is best
82+
// effort; swallow it and begin a fresh transaction below.
7683
}
7784

7885
$this->getPDO()->beginTransaction();

tests/unit/SQLTransactionTest.php

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
<?php
2+
3+
namespace Tests\Unit;
4+
5+
use PDOException;
6+
use PHPUnit\Framework\TestCase;
7+
use Utopia\Database\Adapter\MySQL;
8+
9+
class SQLTransactionTest extends TestCase
10+
{
11+
/**
12+
* A pooled connection (e.g. Swoole PDOProxy) can keep its own transaction
13+
* counter that survives a reconnect, so it reports an open transaction the
14+
* underlying connection no longer holds. The cleanup rollBack() then throws
15+
* "There is no active transaction". startTransaction() must swallow that and
16+
* still begin a fresh transaction instead of surfacing it as a failure.
17+
*/
18+
public function testStartTransactionRecoversFromDesyncedRollback(): void
19+
{
20+
$pdo = $this->getMockBuilder(\PDO::class)
21+
->disableOriginalConstructor()
22+
->getMock();
23+
24+
$pdo->method('inTransaction')->willReturn(true);
25+
$pdo->method('rollBack')->willThrowException(
26+
new PDOException('There is no active transaction')
27+
);
28+
$pdo->expects($this->once())
29+
->method('beginTransaction')
30+
->willReturn(true);
31+
32+
$adapter = new MySQL($pdo);
33+
34+
$this->assertTrue($adapter->startTransaction());
35+
$this->assertTrue($adapter->inTransaction());
36+
}
37+
}

0 commit comments

Comments
 (0)