Move the logic of wrapping driver exceptions to the connection

UPGRADE.md

@@ -4,6 +4,7 @@
 
 1. The `convertException()` method has been removed from the `Driver` interface. The logic of exception conversion has been moved to the `ExceptionConverter` interface. The drivers now must implement the `getExceptionConverter()` method.
 2. The `driverException()` and `driverExceptionDuringQuery()` factory methods have been removed from the `DBALException` class.
+3. Non-driver exceptions (e.g. exceptions of type `Error`) are no longer wrapped in a `DBALException`.
 
 ## BC BREAK: More driver-level methods are allowed to throw a Driver\Exception.
 

phpstan.neon.dist

@@ -8,10 +8,6 @@ parameters:
     reportUnmatchedIgnoredErrors: false
     checkMissingIterableValueType: false
     checkGenericClassInNonGenericObjectType: false
-    earlyTerminatingMethodCalls:
-        Doctrine\DBAL\Connection:
-            - handleDriverException
-            - handleExceptionDuringQuery
     ignoreErrors:
         # removing it would be BC break
         - '~^Constructor of class Doctrine\\DBAL\\Schema\\Table has an unused parameter \$idGeneratorType\.\z~'

src/Connection.php

@@ -9,6 +9,7 @@ use Doctrine\DBAL\Cache\ArrayResult;
 use Doctrine\DBAL\Cache\CacheException;
 use Doctrine\DBAL\Cache\CachingResult;
 use Doctrine\DBAL\Cache\QueryCacheProfile;
+use Doctrine\DBAL\Driver\API\ExceptionConverter;
 use Doctrine\DBAL\Driver\Connection as DriverConnection;
 use Doctrine\DBAL\Driver\Exception as DriverException;
 use Doctrine\DBAL\Driver\Result as DriverResult;
@@ -26,12 +27,18 @@ use Throwable;
 use Traversable;
 
 use function array_key_exists;
+use function array_map;
 use function assert;
+use function bin2hex;
 use function count;
 use function implode;
 use function is_int;
+use function is_resource;
 use function is_string;
+use function json_encode;
 use function key;
+use function preg_replace;
+use function sprintf;
 
 /**
  * A wrapper around a Doctrine\DBAL\Driver\Connection that adds features like
@@ -114,6 +121,9 @@ class Connection implements DriverConnection
      */
     private $platform;
 
+    /** @var ExceptionConverter|null */
+    private $exceptionConverter;
+
     /**
      * The schema manager.
      *
@@ -284,7 +294,7 @@ class Connection implements DriverConnection
         try {
             $this->_conn = $this->_driver->connect($this->params);
         } catch (DriverException $e) {
-            throw DBALException::driverException($this->_driver, $e);
+            throw $this->convertException($e);
         }
 
         $this->transactionNestingLevel = 0;
@@ -468,7 +478,7 @@ class Connection implements DriverConnection
         try {
             return $this->executeQuery($query, $params, $types)->fetchAssociative();
         } catch (DriverException $e) {
-            $this->handleExceptionDuringQuery($e, $query, $params, $types);
+            throw $this->convertExceptionDuringQuery($e, $query, $params, $types);
         }
     }
 
@@ -489,7 +499,7 @@ class Connection implements DriverConnection
         try {
             return $this->executeQuery($query, $params, $types)->fetchNumeric();
         } catch (DriverException $e) {
-            $this->handleExceptionDuringQuery($e, $query, $params, $types);
+            throw $this->convertExceptionDuringQuery($e, $query, $params, $types);
         }
     }
 
@@ -510,7 +520,7 @@ class Connection implements DriverConnection
         try {
             return $this->executeQuery($query, $params, $types)->fetchOne();
         } catch (DriverException $e) {
-            $this->handleExceptionDuringQuery($e, $query, $params, $types);
+            throw $this->convertExceptionDuringQuery($e, $query, $params, $types);
         }
     }
 
@@ -772,7 +782,7 @@ class Connection implements DriverConnection
         try {
             return $this->executeQuery($query, $params, $types)->fetchAllNumeric();
         } catch (DriverException $e) {
-            $this->handleExceptionDuringQuery($e, $query, $params, $types);
+            throw $this->convertExceptionDuringQuery($e, $query, $params, $types);
         }
     }
 
@@ -792,7 +802,7 @@ class Connection implements DriverConnection
         try {
             return $this->executeQuery($query, $params, $types)->fetchAllAssociative();
         } catch (DriverException $e) {
-            $this->handleExceptionDuringQuery($e, $query, $params, $types);
+            throw $this->convertExceptionDuringQuery($e, $query, $params, $types);
         }
     }
 
@@ -812,7 +822,7 @@ class Connection implements DriverConnection
         try {
             return $this->executeQuery($query, $params, $types)->fetchFirstColumn();
         } catch (DriverException $e) {
-            $this->handleExceptionDuringQuery($e, $query, $params, $types);
+            throw $this->convertExceptionDuringQuery($e, $query, $params, $types);
         }
     }
 
@@ -836,7 +846,7 @@ class Connection implements DriverConnection
                 yield $row;
             }
         } catch (DriverException $e) {
-            $this->handleExceptionDuringQuery($e, $query, $params, $types);
+            throw $this->convertExceptionDuringQuery($e, $query, $params, $types);
         }
     }
 
@@ -860,7 +870,7 @@ class Connection implements DriverConnection
                 yield $row;
             }
         } catch (DriverException $e) {
-            $this->handleExceptionDuringQuery($e, $query, $params, $types);
+            throw $this->convertExceptionDuringQuery($e, $query, $params, $types);
         }
     }
 
@@ -884,7 +894,7 @@ class Connection implements DriverConnection
                 yield $value;
             }
         } catch (DriverException $e) {
-            $this->handleExceptionDuringQuery($e, $query, $params, $types);
+            throw $this->convertExceptionDuringQuery($e, $query, $params, $types);
         }
     }
 
@@ -949,7 +959,7 @@ class Connection implements DriverConnection
 
             return new Result($result, $this);
         } catch (DriverException $e) {
-            $this->handleExceptionDuringQuery($e, $query, $params, $types);
+            throw $this->convertExceptionDuringQuery($e, $query, $params, $types);
         } finally {
             if ($logger !== null) {
                 $logger->stopQuery();
@@ -1021,7 +1031,7 @@ class Connection implements DriverConnection
         try {
             return $connection->query($sql);
         } catch (DriverException $e) {
-            $this->handleExceptionDuringQuery($e, $sql);
+            throw $this->convertExceptionDuringQuery($e, $sql);
         } finally {
             if ($logger !== null) {
                 $logger->stopQuery();
@@ -1069,7 +1079,7 @@ class Connection implements DriverConnection
 
             return $connection->exec($query);
         } catch (DriverException $e) {
-            $this->handleExceptionDuringQuery($e, $query, $params, $types);
+            throw $this->convertExceptionDuringQuery($e, $query, $params, $types);
         } finally {
             if ($logger !== null) {
                 $logger->stopQuery();
@@ -1092,7 +1102,7 @@ class Connection implements DriverConnection
         try {
             return $connection->exec($statement);
         } catch (DriverException $e) {
-            $this->handleExceptionDuringQuery($e, $statement);
+            throw $this->convertExceptionDuringQuery($e, $statement);
         } finally {
             if ($logger !== null) {
                 $logger->stopQuery();
@@ -1580,15 +1590,12 @@ class Connection implements DriverConnection
     /**
      * Resolves the parameters to a format which can be displayed.
      *
-     * @internal This is a purely internal method. If you rely on this method, you are advised to
-     *           copy/paste the code as this method may change, or be removed without prior notice.
-     *
      * @param mixed[]                $params
      * @param array<int|string|null> $types
      *
      * @return mixed[]
      */
-    public function resolveParams(array $params, array $types)
+    private function resolveParams(array $params, array $types): array
     {
         $resolvedParams = [];
 
@@ -1640,53 +1647,73 @@ class Connection implements DriverConnection
      *
      * @param array<mixed>           $params
      * @param array<int|string|null> $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,
+    final public function convertExceptionDuringQuery(
+        DriverException $e,
+        string $sql,
+        array $params = [],
+        array $types = []
+    ): DBALException {
+        $message = "An exception occurred while executing '" . $sql . "'";
+
+        if (count($params) > 0) {
+            $message .= ' with params ' . $this->formatParameters(
                 $this->resolveParams($params, $types)
-            )
-        );
+            );
+        }
+
+        $message .= ":\n\n" . $e->getMessage();
+
+        return $this->handleDriverException($e, $message);
     }
 
     /**
      * @internal
-     *
-     * @throws DBALException
-     *
-     * @psalm-return never-return
      */
-    public function handleDriverException(Throwable $e): void
+    final public function convertException(DriverException $e): DBALException
     {
-        $this->throw(
-            DBALException::driverException(
-                $this->_driver,
-                $e
-            )
+        return $this->handleDriverException(
+            $e,
+            'An exception occurred in driver: ' . $e->getMessage()
         );
     }
 
     /**
-     * @internal
-     *
-     * @throws DBALException
+     * Returns a human-readable representation of an array of parameters.
+     * This properly handles binary data by returning a hex representation.
      *
-     * @psalm-return never-return
+     * @param mixed[] $params
      */
-    private function throw(DBALException $e): void
+    private function formatParameters(array $params): string
     {
-        if ($e instanceof ConnectionLost) {
+        return '[' . implode(', ', array_map(static function ($param): string {
+            if (is_resource($param)) {
+                return (string) $param;
+            }
+
+            $json = @json_encode($param);
+
+            if (! is_string($json) || $json === 'null' && is_string($param)) {
+                // JSON encoding failed, this is not a UTF-8 string.
+                return sprintf('"%s"', preg_replace('/.{2}/', '\\x$0', bin2hex($param)));
+            }
+
+            return $json;
+        }, $params)) . ']';
+    }
+
+    private function handleDriverException(DriverException $driverException, string $message): DBALException
+    {
+        if ($this->exceptionConverter === null) {
+            $this->exceptionConverter = $this->_driver->getExceptionConverter();
+        }
+
+        $exception = $this->exceptionConverter->convert($message, $driverException);
+
+        if ($exception instanceof ConnectionLost) {
             $this->close();
         }
 
-        throw $e;
+        return $exception;
     }
 }

src/Connections/PrimaryReadReplicaConnection.php

@@ -232,7 +232,7 @@ class PrimaryReadReplicaConnection extends Connection
         try {
             return $this->_driver->connect($connectionParams);
         } catch (DriverException $e) {
-            throw DBALException::driverException($this->_driver, $e);
+            throw $this->convertException($e);
         }
     }
 

src/DBALException.php

@@ -2,24 +2,14 @@
 
 namespace Doctrine\DBAL;
 
-use Doctrine\DBAL\Driver\Exception as TheDriverException;
-use Doctrine\DBAL\Exception\DriverException;
 use Doctrine\DBAL\Platforms\AbstractPlatform;
 use Doctrine\DBAL\Types\Type;
 use Exception;
-use Throwable;
 
-use function array_map;
-use function bin2hex;
-use function count;
 use function get_class;
 use function gettype;
 use function implode;
 use function is_object;
-use function is_resource;
-use function is_string;
-use function json_encode;
-use function preg_replace;
 use function spl_object_hash;
 use function sprintf;
 
@@ -133,74 +123,6 @@ class DBALException extends Exception
             'Doctrine currently supports only the following drivers: ' . implode(', ', $knownDrivers));
     }
 
-    /**
-     * @param string  $sql
-     * @param mixed[] $params
-     *
-     * @return self
-     */
-    public static function driverExceptionDuringQuery(Driver $driver, Throwable $driverEx, $sql, array $params = [])
-    {
-        $msg = "An exception occurred while executing '" . $sql . "'";
-        if (count($params) > 0) {
-            $msg .= ' with params ' . self::formatParameters($params);
-        }
-
-        $msg .= ":\n\n" . $driverEx->getMessage();
-
-        return static::wrapException($driver, $driverEx, $msg);
-    }
-
-    /**
-     * @return self
-     */
-    public static function driverException(Driver $driver, Throwable $driverEx)
-    {
-        return static::wrapException($driver, $driverEx, 'An exception occurred in driver: ' . $driverEx->getMessage());
-    }
-
-    /**
-     * @return self
-     */
-    private static function wrapException(Driver $driver, Throwable $driverEx, string $msg)
-    {
-        if ($driverEx instanceof DriverException) {
-            return $driverEx;
-        }
-
-        if ($driverEx instanceof TheDriverException) {
-            return $driver->getExceptionConverter()->convert($msg, $driverEx);
-        }
-
-        return new self($msg, 0, $driverEx);
-    }
-
-    /**
-     * Returns a human-readable representation of an array of parameters.
-     * This properly handles binary data by returning a hex representation.
-     *
-     * @param mixed[] $params
-     *
-     * @return string
-     */
-    private static function formatParameters(array $params)
-    {
-        return '[' . implode(', ', array_map(static function ($param): string {
-            if (is_resource($param)) {
-                return (string) $param;
-            }
-
-            $json = @json_encode($param);
-
-            if (! is_string($json) || $json === 'null' && is_string($param)) {
-                // JSON encoding failed, this is not a UTF-8 string.
-                return sprintf('"%s"', preg_replace('/.{2}/', '\\x$0', bin2hex($param)));
-            }
-
-            return $json;
-        }, $params)) . ']';
-    }
-
     /**
      * @param string $wrapperClass
      *

src/Result.php

@@ -33,7 +33,7 @@ final class Result implements ResultInterface
         try {
             return $this->result->fetchNumeric();
         } catch (DriverException $e) {
-            $this->connection->handleDriverException($e);
+            throw $this->connection->convertException($e);
         }
     }
 
@@ -47,7 +47,7 @@ final class Result implements ResultInterface
         try {
             return $this->result->fetchAssociative();
         } catch (DriverException $e) {
-            $this->connection->handleDriverException($e);
+            throw $this->connection->convertException($e);
         }
     }
 
@@ -61,7 +61,7 @@ final class Result implements ResultInterface
         try {
             return $this->result->fetchOne();
         } catch (DriverException $e) {
-            $this->connection->handleDriverException($e);
+            throw $this->connection->convertException($e);
         }
     }
 
@@ -75,7 +75,7 @@ final class Result implements ResultInterface
         try {
             return $this->result->fetchAllNumeric();
         } catch (DriverException $e) {
-            $this->connection->handleDriverException($e);
+            throw $this->connection->convertException($e);
         }
     }
 
@@ -89,7 +89,7 @@ final class Result implements ResultInterface
         try {
             return $this->result->fetchAllAssociative();
         } catch (DriverException $e) {
-            $this->connection->handleDriverException($e);
+            throw $this->connection->convertException($e);
         }
     }
 
@@ -103,7 +103,7 @@ final class Result implements ResultInterface
         try {
             return $this->result->fetchFirstColumn();
         } catch (DriverException $e) {
-            $this->connection->handleDriverException($e);
+            throw $this->connection->convertException($e);
         }
     }
 
@@ -119,7 +119,7 @@ final class Result implements ResultInterface
                 yield $row;
             }
         } catch (DriverException $e) {
-            $this->connection->handleDriverException($e);
+            throw $this->connection->convertException($e);
         }
     }
 
@@ -135,7 +135,7 @@ final class Result implements ResultInterface
                 yield $row;
             }
         } catch (DriverException $e) {
-            $this->connection->handleDriverException($e);
+            throw $this->connection->convertException($e);
         }
     }
 
@@ -151,7 +151,7 @@ final class Result implements ResultInterface
                 yield $value;
             }
         } catch (DriverException $e) {
-            $this->connection->handleDriverException($e);
+            throw $this->connection->convertException($e);
         }
     }
 

src/Statement.php

@@ -75,7 +75,7 @@ class Statement implements DriverStatement
         try {
             $stmt = $driverConnection->prepare($sql);
         } catch (Exception $ex) {
-            $conn->handleExceptionDuringQuery($ex, $sql);
+            throw $conn->convertExceptionDuringQuery($ex, $sql);
         }
 
         $this->sql      = $sql;
@@ -165,7 +165,7 @@ class Statement implements DriverStatement
                 $this->conn
             );
         } catch (Exception $ex) {
-            $this->conn->handleExceptionDuringQuery($ex, $this->sql, $this->params, $this->types);
+            throw $this->conn->convertExceptionDuringQuery($ex, $this->sql, $this->params, $this->types);
         } finally {
             if ($logger !== null) {
                 $logger->stopQuery();

tests/Connection/ExceptionHandlingTest.php

+<?php
+
+namespace Doctrine\Tests\DBAL\Connection;
+
+use Doctrine\DBAL\Connection;
+use Doctrine\DBAL\Driver;
+use Doctrine\DBAL\Driver\API\DefaultExceptionConverter;
+use Doctrine\DBAL\Driver\Exception as DriverException;
+use PHPUnit\Framework\TestCase;
+
+use function chr;
+use function fopen;
+
+final class ExceptionHandlingTest extends TestCase
+{
+    /** @var Connection */
+    private $connection;
+
+    protected function setUp(): void
+    {
+        $this->connection = new Connection([], $this->createConfiguredMock(Driver::class, [
+            'getExceptionConverter' => new DefaultExceptionConverter(),
+        ]));
+    }
+
+    public function testDriverExceptionDuringQueryAcceptsBinaryData(): void
+    {
+        $e = $this->connection->convertExceptionDuringQuery(
+            $this->createMock(DriverException::class),
+            '',
+            ['ABC', chr(128)]
+        );
+
+        self::assertStringContainsString('with params ["ABC", "\x80"]', $e->getMessage());
+    }
+
+    public function testDriverExceptionDuringQueryAcceptsResource(): void
+    {
+        $e = $this->connection->convertExceptionDuringQuery(
+            $this->createMock(DriverException::class),
+            'INSERT INTO file (`content`) VALUES (?)',
+            [
+                1 => fopen(__FILE__, 'r'),
+            ]
+        );
+
+        self::assertStringContainsString('Resource', $e->getMessage());
+    }
+}

tests/DBALExceptionTest.php

@@ -3,44 +3,13 @@
 namespace Doctrine\DBAL\Tests;
 
 use Doctrine\DBAL\DBALException;
-use Doctrine\DBAL\Driver;
-use Doctrine\DBAL\Driver\Exception as InnerDriverException;
-use Doctrine\DBAL\Exception\DriverException;
-use Exception;
 use PHPUnit\Framework\TestCase;
 use stdClass;
 
-use function chr;
-use function fopen;
 use function sprintf;
 
 class DBALExceptionTest extends TestCase
 {
-    public function testDriverExceptionDuringQueryAcceptsBinaryData(): void
-    {
-        $driver = $this->createMock(Driver::class);
-        $e      = DBALException::driverExceptionDuringQuery($driver, new Exception(), '', ['ABC', chr(128)]);
-        self::assertStringContainsString('with params ["ABC", "\x80"]', $e->getMessage());
-    }
-
-    public function testDriverExceptionDuringQueryAcceptsResource(): void
-    {
-        $driver = $this->createMock(Driver::class);
-        $e      = DBALException::driverExceptionDuringQuery($driver, new Exception(), 'INSERT INTO file (`content`) VALUES (?)', [1 => fopen(__FILE__, 'r')]);
-        self::assertStringContainsString('Resource', $e->getMessage());
-    }
-
-    public function testAvoidOverWrappingOnDriverException(): void
-    {
-        $driver = $this->createMock(Driver::class);
-
-        $inner = $this->createMock(InnerDriverException::class);
-
-        $ex = new DriverException('', $inner);
-        $e  = DBALException::driverExceptionDuringQuery($driver, $ex, '');
-        self::assertSame($ex, $e);
-    }
-
     public function testDriverRequiredWithUrl(): void
     {
         $url       = 'mysql://localhost';

tests/Functional/DataAccessTest.php

@@ -4,10 +4,6 @@ namespace Doctrine\DBAL\Tests\Functional;
 
 use DateTime;
 use Doctrine\DBAL\Connection;
-use Doctrine\DBAL\DBALException;
-use Doctrine\DBAL\Driver\IBMDB2\Driver as IBMDB2Driver;
-use Doctrine\DBAL\Driver\Mysqli\Driver as MySQLiDriver;
-use Doctrine\DBAL\Driver\SQLSrv\Driver as SQLSrvDriver;
 use Doctrine\DBAL\ParameterType;
 use Doctrine\DBAL\Platforms\SqlitePlatform;
 use Doctrine\DBAL\Platforms\TrimMode;

tests/StatementTest.php

@@ -127,10 +127,6 @@ class StatementTest extends TestCase
             ->method('getSQLLogger')
             ->will(self::returnValue($logger));
 
-        $this->conn->expects(self::any())
-            ->method('handleExceptionDuringQuery')
-            ->will(self::throwException(new DBALException()));
-
         $logger->expects(self::once())
             ->method('startQuery');