From 695e4c06b4becce150ee299b1603dffe70f960f1 Mon Sep 17 00:00:00 2001 From: Nick Howell Date: Mon, 19 Sep 2016 17:21:38 -0400 Subject: [PATCH 1/2] beginTransaction(): Reconnect if connection is lost If we're beginning a new transaction, we should check if the exception was caused by a lost connection. If so, then try again (like we do when when running a query/statement). We won't do a reconnect if it is a nested transaction though, because if the connection has been lost then the transaction has already been rolled back. (Queries/statements already won't try to reconnect if they're inside a transaction, either.) --- src/Illuminate/Database/Connection.php | 26 ++++++++++++++--------- tests/Database/DatabaseConnectionTest.php | 6 +++--- 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/src/Illuminate/Database/Connection.php b/src/Illuminate/Database/Connection.php index a319789fe80d..24da8bb7fd5f 100755 --- a/src/Illuminate/Database/Connection.php +++ b/src/Illuminate/Database/Connection.php @@ -498,26 +498,30 @@ public function transaction(Closure $callback) * Start a new database transaction. * * @return void - * @throws Exception + * + * @throws \Exception */ public function beginTransaction() { - ++$this->transactions; - - if ($this->transactions == 1) { + if ($this->transactions == 0) { try { - $this->pdo->beginTransaction(); + $this->getPdo()->beginTransaction(); } catch (Exception $e) { - --$this->transactions; - - throw $e; + if ($this->causedByLostConnection($e)) { + $this->reconnect(); + $this->getPdo()->beginTransaction(); + } else { + throw $e; + } } - } elseif ($this->transactions > 1 && $this->queryGrammar->supportsSavepoints()) { + } elseif ($this->transactions >= 1 && $this->queryGrammar->supportsSavepoints()) { $this->pdo->exec( - $this->queryGrammar->compileSavepoint('trans'.$this->transactions) + $this->queryGrammar->compileSavepoint('trans'.($this->transactions + 1)) ); } + ++$this->transactions; + $this->fireConnectionEvent('beganTransaction'); } @@ -866,6 +870,8 @@ public function getReadPdo() * * @param \PDO|null $pdo * @return $this + * + * @throws \RuntimeException */ public function setPdo($pdo) { diff --git a/tests/Database/DatabaseConnectionTest.php b/tests/Database/DatabaseConnectionTest.php index 97d71ef9f8b8..6f0e641c742c 100755 --- a/tests/Database/DatabaseConnectionTest.php +++ b/tests/Database/DatabaseConnectionTest.php @@ -111,14 +111,14 @@ public function testAffectingStatementProperlyCallsPDO() $this->assertTrue(is_numeric($log[0]['time'])); } - public function testTransactionsDecrementedOnTransactionException() + public function testTransactionLevelNotIncrementedOnTransactionException() { $pdo = $this->getMock('DatabaseConnectionTestMockPDO'); - $pdo->expects($this->once())->method('beginTransaction')->will($this->throwException(new ErrorException('MySQL server has gone away'))); + $pdo->expects($this->once())->method('beginTransaction')->will($this->throwException(new Exception)); $connection = $this->getMockConnection([], $pdo); try { $connection->beginTransaction(); - } catch (ErrorException $e) { + } catch (Exception $e) { $this->assertEquals(0, $connection->transactionLevel()); } } From 33c6f9a19e0e19d8d1610e26c00d8acad58a5d71 Mon Sep 17 00:00:00 2001 From: Nick Howell Date: Thu, 22 Sep 2016 10:35:26 -0400 Subject: [PATCH 2/2] Add tests for beginTransaction() retrying --- src/Illuminate/Database/Connection.php | 4 +-- tests/Database/DatabaseConnectionTest.php | 30 +++++++++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/src/Illuminate/Database/Connection.php b/src/Illuminate/Database/Connection.php index 24da8bb7fd5f..7e49014e1e0a 100755 --- a/src/Illuminate/Database/Connection.php +++ b/src/Illuminate/Database/Connection.php @@ -505,11 +505,11 @@ public function beginTransaction() { if ($this->transactions == 0) { try { - $this->getPdo()->beginTransaction(); + $this->pdo->beginTransaction(); } catch (Exception $e) { if ($this->causedByLostConnection($e)) { $this->reconnect(); - $this->getPdo()->beginTransaction(); + $this->pdo->beginTransaction(); } else { throw $e; } diff --git a/tests/Database/DatabaseConnectionTest.php b/tests/Database/DatabaseConnectionTest.php index 6f0e641c742c..50646347a02d 100755 --- a/tests/Database/DatabaseConnectionTest.php +++ b/tests/Database/DatabaseConnectionTest.php @@ -123,6 +123,36 @@ public function testTransactionLevelNotIncrementedOnTransactionException() } } + public function testBeginTransactionMethodRetriesOnFailure() + { + $pdo = $this->getMock('DatabaseConnectionTestMockPDO'); + $pdo->expects($this->exactly(2))->method('beginTransaction'); + $pdo->expects($this->at(0))->method('beginTransaction')->will($this->throwException(new ErrorException('server has gone away'))); + $connection = $this->getMockConnection(['reconnect'], $pdo); + $connection->expects($this->once())->method('reconnect'); + $connection->beginTransaction(); + $this->assertEquals(1, $connection->transactionLevel()); + } + + public function testBeginTransactionMethodNeverRetriesIfWithinTransaction() + { + $pdo = $this->getMock('DatabaseConnectionTestMockPDO'); + $pdo->expects($this->once())->method('beginTransaction'); + $pdo->expects($this->once())->method('exec')->will($this->throwException(new Exception)); + $connection = $this->getMockConnection([], $pdo); + $queryGrammar = $this->getMock('Illuminate\Database\Query\Grammars\Grammar'); + $queryGrammar->expects($this->once())->method('supportsSavepoints')->will($this->returnValue(true)); + $connection->setQueryGrammar($queryGrammar); + $connection->expects($this->never())->method('reconnect'); + $connection->beginTransaction(); + $this->assertEquals(1, $connection->transactionLevel()); + try { + $connection->beginTransaction(); + } catch (Exception $e) { + $this->assertEquals(1, $connection->transactionLevel()); + } + } + /** * @expectedException RuntimeException */