Driver::connect() should throw only driver-level exceptions

doctrine · Jun 13, 2020 · ae8c720 · ae8c720
1 parent 4b2fed5
commit ae8c720
Show file tree

Hide file tree

Showing 11 changed files with 86 additions and 110 deletions.
diff --git a/src/Connection.php b/src/Connection.php
@@ -10,6 +10,7 @@
 use Doctrine\DBAL\Cache\CachingResult;
 use Doctrine\DBAL\Cache\QueryCacheProfile;
 use Doctrine\DBAL\Driver\Connection as DriverConnection;
+use Doctrine\DBAL\Driver\DriverException;
 use Doctrine\DBAL\Driver\PingableConnection;
 use Doctrine\DBAL\Driver\Result as DriverResult;
 use Doctrine\DBAL\Driver\ServerInfoAwareConnection;
@@ -342,6 +343,8 @@ public function getExpressionBuilder()
      *
      * @return bool TRUE if the connection was successfully established, FALSE if
      *              the connection is already open.
+     *
+     * @throws DBALException
      */
     public function connect()
     {
@@ -353,7 +356,12 @@ public function connect()
         $user          = $this->params['user'] ?? null;
         $password      = $this->params['password'] ?? null;
 
-        $this->_conn       = $this->_driver->connect($this->params, $user, $password, $driverOptions);
+        try {
+            $this->_conn = $this->_driver->connect($this->params, $user, $password, $driverOptions);
+        } catch (DriverException $e) {
+            throw DBALException::driverException($this->_driver, $e);
+        }
+
         $this->isConnected = true;
 
         $this->transactionNestingLevel = 0;
@@ -420,7 +428,7 @@ private function getDatabasePlatformVersion()
         if ($this->_conn === null) {
             try {
                 $this->connect();
-            } catch (Throwable $originalException) {
+            } catch (DBALException $originalException) {
                 if (! isset($this->params['dbname'])) {
                     throw $originalException;
                 }
@@ -432,7 +440,7 @@ private function getDatabasePlatformVersion()
 
                 try {
                     $this->connect();
-                } catch (Throwable $fallbackException) {
+                } catch (DBALException $fallbackException) {
                     // Either the platform does not support database-less connections
                     // or something else went wrong.
                     // Reset connection parameters and rethrow the original exception.

diff --git a/src/Connections/PrimaryReadReplicaConnection.php b/src/Connections/PrimaryReadReplicaConnection.php
@@ -5,8 +5,10 @@
 use Doctrine\Common\EventManager;
 use Doctrine\DBAL\Configuration;
 use Doctrine\DBAL\Connection;
+use Doctrine\DBAL\DBALException;
 use Doctrine\DBAL\Driver;
 use Doctrine\DBAL\Driver\Connection as DriverConnection;
+use Doctrine\DBAL\Driver\DriverException;
 use Doctrine\DBAL\Driver\Result;
 use Doctrine\DBAL\Driver\Statement;
 use Doctrine\DBAL\Event\ConnectionEventArgs;
@@ -230,7 +232,11 @@ protected function connectTo($connectionName)
         $user     = $connectionParams['user'] ?? null;
         $password = $connectionParams['password'] ?? null;
 
-        return $this->_driver->connect($connectionParams, $user, $password, $driverOptions);
+        try {
+            return $this->_driver->connect($connectionParams, $user, $password, $driverOptions);
+        } catch (DriverException $e) {
+            throw DBALException::driverException($this->_driver, $e);
+        }
     }
 
     /**

diff --git a/src/Driver.php b/src/Driver.php
@@ -2,6 +2,7 @@
 
 namespace Doctrine\DBAL;
 
+use Doctrine\DBAL\Driver\DriverException;
 use Doctrine\DBAL\Platforms\AbstractPlatform;
 use Doctrine\DBAL\Schema\AbstractSchemaManager;
 
@@ -22,6 +23,8 @@ interface Driver
      * @param mixed[]     $driverOptions The driver options to use when connecting.
      *
      * @return \Doctrine\DBAL\Driver\Connection The database connection.
+     *
+     * @throws DriverException
      */
     public function connect(array $params, $username = null, $password = null, array $driverOptions = []);
 

diff --git a/src/Driver/Mysqli/Driver.php b/src/Driver/Mysqli/Driver.php
@@ -2,7 +2,6 @@
 
 namespace Doctrine\DBAL\Driver\Mysqli;
 
-use Doctrine\DBAL\DBALException;
 use Doctrine\DBAL\Driver\AbstractMySQLDriver;
 
 class Driver extends AbstractMySQLDriver
@@ -12,11 +11,7 @@ class Driver extends AbstractMySQLDriver
      */
     public function connect(array $params, $username = null, $password = null, array $driverOptions = [])
     {
-        try {
-            return new MysqliConnection($params, (string) $username, (string) $password, $driverOptions);
-        } catch (MysqliException $e) {
-            throw DBALException::driverException($this, $e);
-        }
+        return new MysqliConnection($params, (string) $username, (string) $password, $driverOptions);
     }
 
     /**

diff --git a/src/Driver/OCI8/Driver.php b/src/Driver/OCI8/Driver.php
@@ -2,7 +2,6 @@
 
 namespace Doctrine\DBAL\Driver\OCI8;
 
-use Doctrine\DBAL\DBALException;
 use Doctrine\DBAL\Driver\AbstractOracleDriver;
 
 use const OCI_NO_AUTO_COMMIT;
@@ -17,18 +16,14 @@ class Driver extends AbstractOracleDriver
      */
     public function connect(array $params, $username = null, $password = null, array $driverOptions = [])
     {
-        try {
-            return new OCI8Connection(
-                (string) $username,
-                (string) $password,
-                $this->_constructDsn($params),
-                $params['charset'] ?? '',
-                $params['sessionMode'] ?? OCI_NO_AUTO_COMMIT,
-                $params['persistent'] ?? false
-            );
-        } catch (OCI8Exception $e) {
-            throw DBALException::driverException($this, $e);
-        }
+        return new OCI8Connection(
+            (string) $username,
+            (string) $password,
+            $this->_constructDsn($params),
+            $params['charset'] ?? '',
+            $params['sessionMode'] ?? OCI_NO_AUTO_COMMIT,
+            $params['persistent'] ?? false
+        );
     }
 
     /**

diff --git a/src/Driver/PDOMySql/Driver.php b/src/Driver/PDOMySql/Driver.php
@@ -2,11 +2,9 @@
 
 namespace Doctrine\DBAL\Driver\PDOMySql;
 
-use Doctrine\DBAL\DBALException;
 use Doctrine\DBAL\Driver\AbstractMySQLDriver;
 use Doctrine\DBAL\Driver\PDOConnection;
 use PDO;
-use PDOException;
 
 /**
  * PDO MySql driver.
@@ -22,18 +20,12 @@ public function connect(array $params, $username = null, $password = null, array
             $driverOptions[PDO::ATTR_PERSISTENT] = true;
         }
 
-        try {
-            $conn = new PDOConnection(
-                $this->constructPdoDsn($params),
-                $username,
-                $password,
-                $driverOptions
-            );
-        } catch (PDOException $e) {
-            throw DBALException::driverException($this, $e);
-        }
-
-        return $conn;
+        return new PDOConnection(
+            $this->constructPdoDsn($params),
+            $username,
+            $password,
+            $driverOptions
+        );
     }
 
     /**

diff --git a/src/Driver/PDOOracle/Driver.php b/src/Driver/PDOOracle/Driver.php
@@ -2,11 +2,9 @@
 
 namespace Doctrine\DBAL\Driver\PDOOracle;
 
-use Doctrine\DBAL\DBALException;
 use Doctrine\DBAL\Driver\AbstractOracleDriver;
 use Doctrine\DBAL\Driver\PDOConnection;
 use PDO;
-use PDOException;
 
 /**
  * PDO Oracle driver.
@@ -27,16 +25,12 @@ public function connect(array $params, $username = null, $password = null, array
             $driverOptions[PDO::ATTR_PERSISTENT] = true;
         }
 
-        try {
-            return new PDOConnection(
-                $this->constructPdoDsn($params),
-                $username,
-                $password,
-                $driverOptions
-            );
-        } catch (PDOException $e) {
-            throw DBALException::driverException($this, $e);
-        }
+        return new PDOConnection(
+            $this->constructPdoDsn($params),
+            $username,
+            $password,
+            $driverOptions
+        );
     }
 
     /**

diff --git a/src/Driver/PDOPgSql/Driver.php b/src/Driver/PDOPgSql/Driver.php
@@ -2,11 +2,9 @@
 
 namespace Doctrine\DBAL\Driver\PDOPgSql;
 
-use Doctrine\DBAL\DBALException;
 use Doctrine\DBAL\Driver\AbstractPostgreSQLDriver;
 use Doctrine\DBAL\Driver\PDOConnection;
 use PDO;
-use PDOException;
 
 use function defined;
 
@@ -24,35 +22,31 @@ public function connect(array $params, $username = null, $password = null, array
             $driverOptions[PDO::ATTR_PERSISTENT] = true;
         }
 
-        try {
-            $connection = new PDOConnection(
-                $this->_constructPdoDsn($params),
-                $username,
-                $password,
-                $driverOptions
-            );
-
-            if (
-                defined('PDO::PGSQL_ATTR_DISABLE_PREPARES')
-                && (! isset($driverOptions[PDO::PGSQL_ATTR_DISABLE_PREPARES])
-                    || $driverOptions[PDO::PGSQL_ATTR_DISABLE_PREPARES] === true
-                )
-            ) {
-                $connection->getWrappedConnection()->setAttribute(PDO::PGSQL_ATTR_DISABLE_PREPARES, true);
-            }
-
-            /* defining client_encoding via SET NAMES to avoid inconsistent DSN support
-             * - the 'client_encoding' connection param only works with postgres >= 9.1
-             * - passing client_encoding via the 'options' param breaks pgbouncer support
-             */
-            if (isset($params['charset'])) {
-                $connection->exec('SET NAMES \'' . $params['charset'] . '\'');
-            }
-
-            return $connection;
-        } catch (PDOException $e) {
-            throw DBALException::driverException($this, $e);
+        $connection = new PDOConnection(
+            $this->_constructPdoDsn($params),
+            $username,
+            $password,
+            $driverOptions
+        );
+
+        if (
+            defined('PDO::PGSQL_ATTR_DISABLE_PREPARES')
+            && (! isset($driverOptions[PDO::PGSQL_ATTR_DISABLE_PREPARES])
+                || $driverOptions[PDO::PGSQL_ATTR_DISABLE_PREPARES] === true
+            )
+        ) {
+            $connection->getWrappedConnection()->setAttribute(PDO::PGSQL_ATTR_DISABLE_PREPARES, true);
         }
+
+        /* defining client_encoding via SET NAMES to avoid inconsistent DSN support
+         * - the 'client_encoding' connection param only works with postgres >= 9.1
+         * - passing client_encoding via the 'options' param breaks pgbouncer support
+         */
+        if (isset($params['charset'])) {
+            $connection->exec('SET NAMES \'' . $params['charset'] . '\'');
+        }
+
+        return $connection;
     }
 
     /**

diff --git a/src/Driver/PDOSqlite/Driver.php b/src/Driver/PDOSqlite/Driver.php
@@ -2,11 +2,9 @@
 
 namespace Doctrine\DBAL\Driver\PDOSqlite;
 
-use Doctrine\DBAL\DBALException;
 use Doctrine\DBAL\Driver\AbstractSQLiteDriver;
 use Doctrine\DBAL\Driver\PDOConnection;
 use Doctrine\DBAL\Platforms\SqlitePlatform;
-use PDOException;
 
 use function array_merge;
 
@@ -35,16 +33,12 @@ public function connect(array $params, $username = null, $password = null, array
             unset($driverOptions['userDefinedFunctions']);
         }
 
-        try {
-            $connection = new PDOConnection(
-                $this->_constructPdoDsn($params),
-                $username,
-                $password,
-                $driverOptions
-            );
-        } catch (PDOException $ex) {
-            throw DBALException::driverException($this, $ex);
-        }
+        $connection = new PDOConnection(
+            $this->_constructPdoDsn($params),
+            $username,
+            $password,
+            $driverOptions
+        );
 
         $pdo = $connection->getWrappedConnection();
 

diff --git a/src/Driver/SQLAnywhere/Driver.php b/src/Driver/SQLAnywhere/Driver.php
@@ -21,22 +21,18 @@ class Driver extends AbstractSQLAnywhereDriver
      */
     public function connect(array $params, $username = null, $password = null, array $driverOptions = [])
     {
-        try {
-            return new SQLAnywhereConnection(
-                $this->buildDsn(
-                    $params['host'] ?? null,
-                    $params['port'] ?? null,
-                    $params['server'] ?? null,
-                    $params['dbname'] ?? null,
-                    $username,
-                    $password,
-                    $driverOptions
-                ),
-                $params['persistent'] ?? false
-            );
-        } catch (SQLAnywhereException $e) {
-            throw DBALException::driverException($this, $e);
-        }
+        return new SQLAnywhereConnection(
+            $this->buildDsn(
+                $params['host'] ?? null,
+                $params['port'] ?? null,
+                $params['server'] ?? null,
+                $params['dbname'] ?? null,
+                $username,
+                $password,
+                $driverOptions
+            ),
+            $params['persistent'] ?? false
+        );
     }
 
     /**

diff --git a/tests/ConnectionTest.php b/tests/ConnectionTest.php
@@ -20,7 +20,6 @@
 use Doctrine\DBAL\ParameterType;
 use Doctrine\DBAL\Platforms\AbstractPlatform;
 use Doctrine\DBAL\VersionAwarePlatformDriver;
-use Exception;
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
 use stdClass;
@@ -788,8 +787,8 @@ public function testRethrowsOriginalExceptionOnDeterminingPlatformWhenConnecting
         $driverMock = $this->createMock(VersionAwarePlatformDriver::class);
 
         $connection        = new Connection(['dbname' => 'foo'], $driverMock);
-        $originalException = new Exception('Original exception');
-        $fallbackException = new Exception('Fallback exception');
+        $originalException = new DBALException('Original exception');
+        $fallbackException = new DBALException('Fallback exception');
 
         $driverMock->expects(self::at(0))
             ->method('connect')