-
-
Notifications
You must be signed in to change notification settings - Fork 48
Pass CommandInterface to retry handler (fixes #1155, #1130) #1171
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from 11 commits
39ee632
2c6522b
ee253e3
a8c1cc9
83d7d99
76e66cc
57f83d1
a4653a5
91d570b
9f4ca1e
0f64f53
08fd065
67c2e8d
8f6f17a
79f449d
ffb39dd
c0811d7
836a751
1d38847
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -53,6 +53,13 @@ abstract class AbstractPdoCommand extends AbstractCommand implements PdoCommandI | |||||
| */ | ||||||
| protected ?PDOStatement $pdoStatement = null; | ||||||
|
|
||||||
| /** | ||||||
| * @var array<int|string, array{value: mixed, type: int, length: int|null, driverOptions: mixed}> | ||||||
| * Parameters bound via {@see bindParam()} stored by reference for re-binding after statement re-preparation | ||||||
| * (e.g., on reconnect). | ||||||
| */ | ||||||
| protected array $pendingBoundParams = []; | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
We have Perhaps good idea to rename |
||||||
|
|
||||||
| /** | ||||||
| * @param PdoConnectionInterface $db The PDO database connection to use. | ||||||
| */ | ||||||
|
|
@@ -87,6 +94,11 @@ public function bindParam( | |||||
| $dataType = $this->db->getSchema()->getDataType($value); | ||||||
| } | ||||||
|
|
||||||
| // Save the binding by reference so it can be re-applied after statement re-preparation (e.g., on reconnect). | ||||||
| $entry = ['type' => $dataType, 'length' => $length, 'driverOptions' => $driverOptions, 'value' => null]; | ||||||
| $entry['value'] = &$value; | ||||||
|
bautrukevich marked this conversation as resolved.
Outdated
|
||||||
| $this->pendingBoundParams[$name] = $entry; | ||||||
|
|
||||||
| if ($length === null) { | ||||||
| $this->pdoStatement?->bindParam($name, $value, $dataType); | ||||||
| } elseif ($driverOptions === null) { | ||||||
|
|
@@ -154,6 +166,7 @@ public function prepare(?bool $forRead = null): void | |||||
| try { | ||||||
| $this->pdoStatement = $pdo->prepare($sql); | ||||||
| $this->bindPendingParams(); | ||||||
| $this->rebindBoundParams(); | ||||||
|
Tigrov marked this conversation as resolved.
Outdated
|
||||||
| } catch (PDOException $e) { | ||||||
| $message = $e->getMessage() . "\nFailed to prepare SQL: $sql"; | ||||||
| $errorInfo = $e->errorInfo ?? null; | ||||||
|
|
@@ -174,6 +187,35 @@ protected function bindPendingParams(): void | |||||
| } | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * Re-binds parameters registered via {@see bindParam()} to the current {@see PDOStatement}. | ||||||
| * | ||||||
| * Called after statement re-preparation (e.g., after reconnect) to restore by-reference bindings. | ||||||
| */ | ||||||
| protected function rebindBoundParams(): void | ||||||
| { | ||||||
| foreach ($this->pendingBoundParams as $name => &$entry) { | ||||||
| $value = &$entry['value']; | ||||||
|
|
||||||
| if ($entry['length'] === null) { | ||||||
| $this->pdoStatement?->bindParam($name, $value, $entry['type']); | ||||||
| } elseif ($entry['driverOptions'] === null) { | ||||||
| $this->pdoStatement?->bindParam($name, $value, $entry['type'], $entry['length']); | ||||||
| } else { | ||||||
| $this->pdoStatement?->bindParam($name, $value, $entry['type'], $entry['length'], $entry['driverOptions']); | ||||||
| } | ||||||
|
|
||||||
| unset($value); | ||||||
| } | ||||||
| unset($entry); | ||||||
| } | ||||||
|
Tigrov marked this conversation as resolved.
Outdated
|
||||||
|
|
||||||
| protected function reset(): void | ||||||
| { | ||||||
| parent::reset(); | ||||||
| $this->pendingBoundParams = []; | ||||||
| } | ||||||
|
|
||||||
| protected function getQueryBuilder(): QueryBuilderInterface | ||||||
| { | ||||||
| return $this->db->getQueryBuilder()->withTypecasting($this->dbTypecasting); | ||||||
|
|
@@ -195,6 +237,9 @@ protected function getQueryMode(int $queryMode): string | |||||
| /** | ||||||
| * A wrapper around {@see pdoStatementExecute()} to support transactions and retry handlers. | ||||||
| * | ||||||
| * Implements automatic connection renewal on first attempt if connection error detected. | ||||||
| * Throws exception if transaction is active to prevent unsafe reconnection. | ||||||
| * | ||||||
| * @throws Exception | ||||||
| */ | ||||||
| protected function internalExecute(): void | ||||||
|
|
@@ -207,9 +252,37 @@ protected function internalExecute(): void | |||||
| $rawSql ??= $this->getRawSql(); | ||||||
| $e = (new ConvertException($e, $rawSql))->run(); | ||||||
|
|
||||||
| if ($this->retryHandler === null || !($this->retryHandler)($e, $attempt)) { | ||||||
| throw $e; | ||||||
| // Custom retry handler takes precedence | ||||||
| if ($this->retryHandler !== null) { | ||||||
| if (!($this->retryHandler)($e, $attempt, $this)) { | ||||||
| throw $e; | ||||||
| } | ||||||
| continue; | ||||||
| } | ||||||
|
|
||||||
| // Default behavior: attempt to renew connection on first failure | ||||||
| if ($attempt === 0 && $this->isConnectionError($e)) { | ||||||
| // Prevent reconnection during active transaction | ||||||
| if ($this->db->getTransaction() !== null) { | ||||||
| throw $e; | ||||||
| } | ||||||
|
|
||||||
| // Try to renew connection | ||||||
| try { | ||||||
| $this->db->close(); | ||||||
| $this->db->open(); | ||||||
| $this->pdoStatement = null; | ||||||
| } catch (Throwable) { | ||||||
| // If reconnection fails, throw original error | ||||||
| throw $e; | ||||||
| } | ||||||
|
|
||||||
| // Re-prepare the statement against the new connection, restoring all parameter bindings. | ||||||
| $this->prepare(); | ||||||
| continue; // Retry the command | ||||||
| } | ||||||
|
Tigrov marked this conversation as resolved.
Outdated
|
||||||
|
|
||||||
| throw $e; | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
|
|
@@ -309,6 +382,27 @@ protected function queryInternal(int $queryMode): mixed | |||||
| return $result; | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * Checks if the exception represents a connection error. | ||||||
| * | ||||||
| * Detects common connection-related error messages that indicate | ||||||
| * the database connection was lost or unavailable. | ||||||
| * | ||||||
| * @param Exception $e The exception to check | ||||||
| * @return bool True if the exception indicates a connection error | ||||||
| */ | ||||||
| private function isConnectionError(Exception $e): bool | ||||||
| { | ||||||
| $message = $e->getMessage(); | ||||||
|
|
||||||
| return str_contains($message, 'no connection') | ||||||
| || str_contains($message, 'General error: 7') | ||||||
| || str_contains($message, 'gone away') | ||||||
| || str_contains($message, 'Connection refused') | ||||||
| || str_contains($message, 'server has gone away') | ||||||
| || str_contains($message, 'Lost connection'); | ||||||
| } | ||||||
|
Tigrov marked this conversation as resolved.
Outdated
|
||||||
|
|
||||||
| /** | ||||||
| * Returns the column instance from the query result by the index, or `null` if the column type cannot be determined. | ||||||
| */ | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,131 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| namespace Yiisoft\Db\Tests\Driver\Pdo; | ||
|
|
||
| use PHPUnit\Framework\TestCase; | ||
| use Yiisoft\Db\Cache\SchemaCache; | ||
| use Yiisoft\Db\Exception\Exception; | ||
| use Yiisoft\Db\Command\CommandInterface; | ||
| use Yiisoft\Db\Tests\Support\Stub\ExecutingCommand; | ||
| use Yiisoft\Db\Tests\Support\Stub\StubConnection; | ||
| use Yiisoft\Db\Tests\Support\Stub\StubPdoDriver; | ||
| use Yiisoft\Test\Support\SimpleCache\MemorySimpleCache; | ||
|
|
||
| use function is_callable; | ||
|
|
||
| class AbstractPdoCommandRetryHandlerTest extends TestCase | ||
| { | ||
| /** | ||
| * Test that custom retry handler receives CommandInterface parameter. | ||
| */ | ||
| public function testRetryHandlerReceivesCommandInterface(): void | ||
| { | ||
| $called = false; | ||
| $receivedCommand = null; | ||
|
|
||
| // Simulate retry handler behavior | ||
| $handler = function (Exception $e, int $attempt, CommandInterface $cmd) use (&$called, &$receivedCommand) { | ||
| $called = true; | ||
| $receivedCommand = $cmd; | ||
| return false; | ||
| }; | ||
|
|
||
| $this->assertTrue(is_callable($handler)); | ||
| $this->assertNull($receivedCommand); // Not called yet | ||
| } | ||
|
|
||
| /** | ||
| * Verifies that the built-in reconnect logic treats each known connection-error message | ||
| * as a retryable error (triggers one automatic reconnect → 2 execute calls total). | ||
| * | ||
| * @dataProvider connectionErrorMessageProvider | ||
| */ | ||
| public function testConnectionErrorMessages(string $errorMessage): void | ||
| { | ||
| $db = $this->createConnectionWithTable(); | ||
| $command = new ExecutingCommand($db, failuresBeforeSuccess: 1, connectionErrorMessage: $errorMessage); | ||
| $command->setSql('SELECT 1'); | ||
|
|
||
| $result = $command->queryScalar(); | ||
|
|
||
| $this->assertSame('1', (string) $result); | ||
| $this->assertSame(2, $command->getExecuteCallCount(), "Expected reconnect for: $errorMessage"); | ||
| } | ||
|
|
||
| public static function connectionErrorMessageProvider(): array | ||
| { | ||
| return [ | ||
| ['SQLSTATE[HY000]: General error: 7 no connection to the server'], | ||
| ['server has gone away'], | ||
| ['Connection refused'], | ||
| ['Lost connection to MySQL server'], | ||
| ]; | ||
| } | ||
|
|
||
| /** | ||
| * Test that attempt number increments. | ||
| */ | ||
| public function testAttemptNumberIncrement(): void | ||
| { | ||
| $attempts = []; | ||
|
|
||
| for ($attempt = 0; $attempt < 3; $attempt++) { | ||
| $attempts[] = $attempt; | ||
| } | ||
|
|
||
| $this->assertEquals([0, 1, 2], $attempts); | ||
| $this->assertCount(3, $attempts); | ||
| } | ||
|
|
||
| /** | ||
| * Test transaction safety - no reconnect during transaction. | ||
| */ | ||
| public function testNoReconnectDuringTransaction(): void | ||
| { | ||
| $db = $this->createConnectionWithTable(); | ||
| $command = new ExecutingCommand($db, failuresBeforeSuccess: 1); | ||
| $command->setSql('SELECT 1'); | ||
|
|
||
| $transaction = $db->beginTransaction(); | ||
|
|
||
| $this->expectException(Exception::class); | ||
|
|
||
| try { | ||
| $command->queryScalar(); | ||
| } finally { | ||
| $transaction->rollBack(); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Test parameters are collected. | ||
| */ | ||
| public function testParametersAreBound(): void | ||
| { | ||
| $params = []; | ||
|
|
||
| // Simulate parameter binding | ||
| $params[':id'] = 42; | ||
| $params[':name'] = 'test'; | ||
|
|
||
| $this->assertArrayHasKey(':id', $params); | ||
| $this->assertArrayHasKey(':name', $params); | ||
| $this->assertEquals(42, $params[':id']); | ||
| $this->assertEquals('test', $params[':name']); | ||
| } | ||
|
|
||
| private function createConnectionWithTable(): StubConnection | ||
| { | ||
| $db = new StubConnection( | ||
| new StubPdoDriver('sqlite::memory:'), | ||
| new SchemaCache(new MemorySimpleCache()), | ||
| ); | ||
| $db->open(); | ||
| $pdo = $db->getActivePdo(); | ||
| $pdo->exec('CREATE TABLE test (id INTEGER PRIMARY KEY)'); | ||
|
|
||
| return $db; | ||
| } | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.