Merge pull request #4119 from morozov/deprecate-pingable

Deprecate PingableConnection in favor of handling lost connection

UPGRADE.md

 # Upgrade to 2.11
 
+## Statement constructors are marked internal
+
+The driver and wrapper statement objects can be only created by the corresponding connection objects.
+
+## The `PingableConnection` interface is deprecated
+
+The wrapper connection will automatically handle the lost connection if the driver supports reporting it.
+
 ## The `ExceptionConverterDriver` interface is deprecated
 
 All drivers will have to implement the exception conversion API.

lib/Doctrine/DBAL/Connection.php

@@ -13,6 +13,7 @@ use Doctrine\DBAL\Driver\Connection as DriverConnection;
 use Doctrine\DBAL\Driver\PingableConnection;
 use Doctrine\DBAL\Driver\ResultStatement;
 use Doctrine\DBAL\Driver\ServerInfoAwareConnection;
+use Doctrine\DBAL\Exception\ConnectionLost;
 use Doctrine\DBAL\Exception\InvalidArgumentException;
 use Doctrine\DBAL\Platforms\AbstractPlatform;
 use Doctrine\DBAL\Query\Expression\ExpressionBuilder;
@@ -614,7 +615,7 @@ class Connection implements DriverConnection
 
             return $stmt->fetch(FetchMode::ASSOCIATIVE);
         } catch (Throwable $e) {
-            throw DBALException::driverExceptionDuringQuery($this->_driver, $e, $query);
+            $this->handleExceptionDuringQuery($e, $query, $params, $types);
         }
     }
 
@@ -641,7 +642,7 @@ class Connection implements DriverConnection
 
             return $stmt->fetch(FetchMode::NUMERIC);
         } catch (Throwable $e) {
-            throw DBALException::driverExceptionDuringQuery($this->_driver, $e, $query);
+            $this->handleExceptionDuringQuery($e, $query, $params, $types);
         }
     }
 
@@ -668,7 +669,7 @@ class Connection implements DriverConnection
 
             return $stmt->fetch(FetchMode::COLUMN);
         } catch (Throwable $e) {
-            throw DBALException::driverExceptionDuringQuery($this->_driver, $e, $query);
+            $this->handleExceptionDuringQuery($e, $query, $params, $types);
         }
     }
 
@@ -952,7 +953,7 @@ class Connection implements DriverConnection
 
             return $stmt->fetchAll(FetchMode::NUMERIC);
         } catch (Throwable $e) {
-            throw DBALException::driverExceptionDuringQuery($this->_driver, $e, $query);
+            $this->handleExceptionDuringQuery($e, $query, $params, $types);
         }
     }
 
@@ -978,7 +979,7 @@ class Connection implements DriverConnection
 
             return $stmt->fetchAll(FetchMode::ASSOCIATIVE);
         } catch (Throwable $e) {
-            throw DBALException::driverExceptionDuringQuery($this->_driver, $e, $query);
+            $this->handleExceptionDuringQuery($e, $query, $params, $types);
         }
     }
 
@@ -1004,7 +1005,7 @@ class Connection implements DriverConnection
 
             return $stmt->fetchAll(FetchMode::COLUMN);
         } catch (Throwable $e) {
-            throw DBALException::driverExceptionDuringQuery($this->_driver, $e, $query);
+            $this->handleExceptionDuringQuery($e, $query, $params, $types);
         }
     }
 
@@ -1032,7 +1033,7 @@ class Connection implements DriverConnection
                 }
             }
         } catch (Throwable $e) {
-            throw DBALException::driverExceptionDuringQuery($this->_driver, $e, $query);
+            $this->handleExceptionDuringQuery($e, $query, $params, $types);
         }
     }
 
@@ -1060,7 +1061,7 @@ class Connection implements DriverConnection
                 }
             }
         } catch (Throwable $e) {
-            throw DBALException::driverExceptionDuringQuery($this->_driver, $e, $query);
+            $this->handleExceptionDuringQuery($e, $query, $params, $types);
         }
     }
 
@@ -1088,7 +1089,7 @@ class Connection implements DriverConnection
                 }
             }
         } catch (Throwable $e) {
-            throw DBALException::driverExceptionDuringQuery($this->_driver, $e, $query);
+            $this->handleExceptionDuringQuery($e, $query, $params, $types);
         }
     }
 
@@ -1105,8 +1106,8 @@ class Connection implements DriverConnection
     {
         try {
             $stmt = new Statement($statement, $this);
-        } catch (Throwable $ex) {
-            throw DBALException::driverExceptionDuringQuery($this->_driver, $ex, $statement);
+        } catch (Throwable $e) {
+            $this->handleExceptionDuringQuery($e, $statement);
         }
 
         $stmt->setFetchMode($this->defaultFetchMode);
@@ -1156,8 +1157,8 @@ class Connection implements DriverConnection
             } else {
                 $stmt = $connection->query($query);
             }
-        } catch (Throwable $ex) {
-            throw DBALException::driverExceptionDuringQuery($this->_driver, $ex, $query, $this->resolveParams($params, $types));
+        } catch (Throwable $e) {
+            $this->handleExceptionDuringQuery($e, $query, $params, $types);
         }
 
         $stmt->setFetchMode($this->defaultFetchMode);
@@ -1263,8 +1264,8 @@ class Connection implements DriverConnection
 
         try {
             $statement = $connection->query(...$args);
-        } catch (Throwable $ex) {
-            throw DBALException::driverExceptionDuringQuery($this->_driver, $ex, $args[0]);
+        } catch (Throwable $e) {
+            $this->handleExceptionDuringQuery($e, $args[0]);
         }
 
         $statement->setFetchMode($this->defaultFetchMode);
@@ -1316,8 +1317,8 @@ class Connection implements DriverConnection
             } else {
                 $result = $connection->exec($query);
             }
-        } catch (Throwable $ex) {
-            throw DBALException::driverExceptionDuringQuery($this->_driver, $ex, $query, $this->resolveParams($params, $types));
+        } catch (Throwable $e) {
+            $this->handleExceptionDuringQuery($e, $query, $params, $types);
         }
 
         if ($logger) {
@@ -1347,8 +1348,8 @@ class Connection implements DriverConnection
 
         try {
             $result = $connection->exec($statement);
-        } catch (Throwable $ex) {
-            throw DBALException::driverExceptionDuringQuery($this->_driver, $ex, $statement);
+        } catch (Throwable $e) {
+            $this->handleExceptionDuringQuery($e, $statement);
         }
 
         if ($logger) {
@@ -1924,6 +1925,8 @@ class Connection implements DriverConnection
      * It is responsibility of the developer to handle this case
      * and abort the request or reconnect manually:
      *
+     * @deprecated
+     *
      * @return bool
      *
      * @example
@@ -1954,4 +1957,59 @@ class Connection implements DriverConnection
             return false;
         }
     }
+
+    /**
+     * @internal
+     *
+     * @param array<int, mixed>|array<string, mixed>           $params
+     * @param array<int, int|string>|array<string, int|string> $types
+     *
+     * @throws DBALException
+     *
+     * @psalm-return never-return
+     */
+    public function handleExceptionDuringQuery(Throwable $e, string $sql, array $params = [], array $types = []): void
+    {
+        $this->throw(
+            DBALException::driverExceptionDuringQuery(
+                $this->_driver,
+                $e,
+                $sql,
+                $this->resolveParams($params, $types)
+            )
+        );
+    }
+
+    /**
+     * @internal
+     *
+     * @throws DBALException
+     *
+     * @psalm-return never-return
+     */
+    public function handleDriverException(Throwable $e): void
+    {
+        $this->throw(
+            DBALException::driverException(
+                $this->_driver,
+                $e
+            )
+        );
+    }
+
+    /**
+     * @internal
+     *
+     * @throws DBALException
+     *
+     * @psalm-return never-return
+     */
+    private function throw(DBALException $e): void
+    {
+        if ($e instanceof ConnectionLost) {
+            $this->close();
+        }
+
+        throw $e;
+    }
 }

lib/Doctrine/DBAL/Driver/AbstractMySQLDriver.php

@@ -7,6 +7,7 @@ use Doctrine\DBAL\DBALException;
 use Doctrine\DBAL\Driver;
 use Doctrine\DBAL\Driver\DriverException as DeprecatedDriverException;
 use Doctrine\DBAL\Exception\ConnectionException;
+use Doctrine\DBAL\Exception\ConnectionLost;
 use Doctrine\DBAL\Exception\DeadlockException;
 use Doctrine\DBAL\Exception\DriverException;
 use Doctrine\DBAL\Exception\ForeignKeyConstraintViolationException;
@@ -108,6 +109,9 @@ abstract class AbstractMySQLDriver implements Driver, ExceptionConverterDriver,
             case '2005':
                 return new ConnectionException($message, $exception);
 
+            case '2006':
+                return new ConnectionLost($message, $exception);
+
             case '1048':
             case '1121':
             case '1138':

lib/Doctrine/DBAL/Driver/IBMDB2/DB2Statement.php

@@ -92,6 +92,8 @@ class DB2Statement implements IteratorAggregate, StatementInterface, Result
     private $result = false;
 
     /**
+     * @internal The statement can be only instantiated by its driver connection.
+     *
      * @param resource $stmt
      */
     public function __construct($stmt)

lib/Doctrine/DBAL/Driver/Mysqli/MysqliConnection.php

@@ -281,6 +281,8 @@ class MysqliConnection implements ConnectionInterface, PingableConnection, Serve
     /**
      * Pings the server and re-connects when `mysqli.reconnect = 1`
      *
+     * @deprecated
+     *
      * @return bool
      */
     public function ping()

lib/Doctrine/DBAL/Driver/Mysqli/MysqliStatement.php

@@ -82,6 +82,8 @@ class MysqliStatement implements IteratorAggregate, StatementInterface, Result
     private $result = false;
 
     /**
+     * @internal The statement can be only instantiated by its driver connection.
+     *
      * @param string $prepareString
      *
      * @throws MysqliException

lib/Doctrine/DBAL/Driver/OCI8/OCI8Statement.php

@@ -105,6 +105,8 @@ class OCI8Statement implements IteratorAggregate, StatementInterface, Result
     /**
      * Creates a new OCI8Statement that uses the given connection handle and SQL statement.
      *
+     * @internal The statement can be only instantiated by its driver connection.
+     *
      * @param resource $dbh   The connection handle.
      * @param string   $query The SQL query.
      */

lib/Doctrine/DBAL/Driver/PDOStatement.php

@@ -46,6 +46,8 @@ class PDOStatement extends \PDOStatement implements StatementInterface, Result
 
     /**
      * Protected constructor.
+     *
+     * @internal The statement can be only instantiated by its driver connection.
      */
     protected function __construct()
     {

lib/Doctrine/DBAL/Driver/PingableConnection.php

@@ -4,6 +4,8 @@ namespace Doctrine\DBAL\Driver;
 
 /**
  * An interface for connections which support a "native" ping method.
+ *
+ * @deprecated
  */
 interface PingableConnection
 {

lib/Doctrine/DBAL/Driver/SQLAnywhere/SQLAnywhereStatement.php

@@ -71,6 +71,8 @@ class SQLAnywhereStatement implements IteratorAggregate, Statement, Result
     /**
      * Prepares given statement for given connection.
      *
+     * @internal The statement can be only instantiated by its driver connection.
+     *
      * @param resource $conn The connection resource to use.
      * @param string   $sql  The SQL statement to prepare.
      *

lib/Doctrine/DBAL/Driver/SQLSrv/SQLSrvStatement.php

@@ -136,6 +136,8 @@ class SQLSrvStatement implements IteratorAggregate, StatementInterface, Result
     public const LAST_INSERT_ID_SQL = ';SELECT SCOPE_IDENTITY() AS LastInsertId;';
 
     /**
+     * @internal The statement can be only instantiated by its driver connection.
+     *
      * @param resource $conn
      * @param string   $sql
      */

lib/Doctrine/DBAL/Exception/ConnectionLost.php

+<?php
+
+namespace Doctrine\DBAL\Exception;
+
+/**
+ * @psalm-immutable
+ */
+final class ConnectionLost extends ConnectionException
+{
+}

lib/Doctrine/DBAL/Statement.php

@@ -66,6 +66,8 @@ class Statement implements IteratorAggregate, DriverStatement, Result
     /**
      * Creates a new <tt>Statement</tt> for the given SQL and <tt>Connection</tt>.
      *
+     * @internal The statement can be only instantiated by {@link Connection}.
+     *
      * @param string     $sql  The SQL of the statement.
      * @param Connection $conn The connection on which the statement should be executed.
      */
@@ -161,12 +163,7 @@ class Statement implements IteratorAggregate, DriverStatement, Result
                 $logger->stopQuery();
             }
 
-            throw DBALException::driverExceptionDuringQuery(
-                $this->conn->getDriver(),
-                $ex,
-                $this->sql,
-                $this->conn->resolveParams($this->params, $this->types)
-            );
+            $this->conn->handleExceptionDuringQuery($ex, $this->sql, $this->params, $this->types);
         }
 
         if ($logger) {
@@ -297,7 +294,7 @@ class Statement implements IteratorAggregate, DriverStatement, Result
 
             return $this->stmt->fetch(FetchMode::NUMERIC);
         } catch (Exception $e) {
-            throw DBALException::driverException($this->conn->getDriver(), $e);
+            $this->conn->handleDriverException($e);
         }
     }
 
@@ -315,7 +312,7 @@ class Statement implements IteratorAggregate, DriverStatement, Result
 
             return $this->stmt->fetch(FetchMode::ASSOCIATIVE);
         } catch (Exception $e) {
-            throw DBALException::driverException($this->conn->getDriver(), $e);
+            $this->conn->handleDriverException($e);
         }
     }
 
@@ -333,7 +330,7 @@ class Statement implements IteratorAggregate, DriverStatement, Result
 
             return $this->stmt->fetch(FetchMode::COLUMN);
         } catch (Exception $e) {
-            throw DBALException::driverException($this->conn->getDriver(), $e);
+            $this->conn->handleDriverException($e);
         }
     }
 
@@ -351,7 +348,7 @@ class Statement implements IteratorAggregate, DriverStatement, Result
 
             return $this->stmt->fetchAll(FetchMode::NUMERIC);
         } catch (Exception $e) {
-            throw DBALException::driverException($this->conn->getDriver(), $e);
+            $this->conn->handleDriverException($e);
         }
     }
 
@@ -369,7 +366,7 @@ class Statement implements IteratorAggregate, DriverStatement, Result
 
             return $this->stmt->fetchAll(FetchMode::ASSOCIATIVE);
         } catch (Exception $e) {
-            throw DBALException::driverException($this->conn->getDriver(), $e);
+            $this->conn->handleDriverException($e);
         }
     }
 
@@ -387,7 +384,7 @@ class Statement implements IteratorAggregate, DriverStatement, Result
 
             return $this->stmt->fetchAll(FetchMode::COLUMN);
         } catch (Exception $e) {
-            throw DBALException::driverException($this->conn->getDriver(), $e);
+            $this->conn->handleDriverException($e);
         }
     }
 
@@ -411,7 +408,7 @@ class Statement implements IteratorAggregate, DriverStatement, Result
                 }
             }
         } catch (Exception $e) {
-            throw DBALException::driverException($this->conn->getDriver(), $e);
+            $this->conn->handleDriverException($e);
         }
     }
 
@@ -435,7 +432,7 @@ class Statement implements IteratorAggregate, DriverStatement, Result
                 }
             }
         } catch (Exception $e) {
-            throw DBALException::driverException($this->conn->getDriver(), $e);
+            $this->conn->handleDriverException($e);
         }
     }
 
@@ -459,7 +456,7 @@ class Statement implements IteratorAggregate, DriverStatement, Result
                 }
             }
         } catch (Exception $e) {
-            throw DBALException::driverException($this->conn->getDriver(), $e);
+            $this->conn->handleDriverException($e);
         }
     }
 

phpstan.neon.dist

@@ -7,6 +7,10 @@ parameters:
     reportUnmatchedIgnoredErrors: false
     checkMissingIterableValueType: false
     checkGenericClassInNonGenericObjectType: false
+    earlyTerminatingMethodCalls:
+        Doctrine\DBAL\Connection:
+            - handleDriverException
+            - handleExceptionDuringQuery
     ignoreErrors:
         # extension not available
         - '~^(Used )?(Function|Constant) sasql_\S+ not found\.\z~i'

tests/Doctrine/Tests/DBAL/Functional/Connection/ConnectionLostTest.php

+<?php
+
+namespace Doctrine\Tests\DBAL\Functional;
+
+use Doctrine\DBAL\Exception\ConnectionLost;
+use Doctrine\DBAL\Platforms\MySqlPlatform;
+use Doctrine\Tests\DbalFunctionalTestCase;
+
+use function sleep;
+
+class ConnectionLostTest extends DbalFunctionalTestCase
+{
+    protected function setUp(): void
+    {
+        parent::setUp();
+
+        if ($this->connection->getDatabasePlatform() instanceof MySqlPlatform) {
+            return;
+        }
+
+        $this->markTestSkipped('Currently only supported with MySQL');
+    }
+
+    protected function tearDown(): void
+    {
+        $this->resetSharedConn();
+
+        parent::tearDown();
+    }
+
+    public function testConnectionLost(): void
+    {
+        $this->connection->query('SET SESSION wait_timeout=1');
+
+        sleep(2);
+
+        $query = $this->connection->getDatabasePlatform()
+            ->getDummySelectSQL();
+
+        try {
+            // in addition to the error, PHP 7.3 will generate a warning that needs to be
+            // suppressed in order to not let PHPUnit handle it before the actual error
+            @$this->connection->executeQuery($query);
+        } catch (ConnectionLost $e) {
+            self::assertEquals(1, $this->connection->fetchOne($query));
+
+            return;
+        }
+
+        self::fail('The connection should have lost');
+    }
+}

tests/Doctrine/Tests/DBAL/StatementTest.php

@@ -131,10 +131,9 @@ class StatementTest extends DbalTestCase
             ->method('getSQLLogger')
             ->will($this->returnValue($logger));
 
-        // Needed to satisfy construction of DBALException
         $this->conn->expects($this->any())
-            ->method('resolveParams')
-            ->will($this->returnValue([]));
+            ->method('handleExceptionDuringQuery')
+            ->will($this->throwException(new DBALException()));
 
         $logger->expects($this->once())
             ->method('startQuery');