Skip to content
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

Move the logic of driver exception conversion into a separate interface #4136

Merged
merged 5 commits into from
Jul 2, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions UPGRADE.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Upgrade to 3.0

## BC BREAK: Changes in driver-level exception handling

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.

The following driver-level methods are allowed to throw a Driver\Exception:
Expand Down
4 changes: 0 additions & 4 deletions phpstan.neon.dist
Original file line number Diff line number Diff line change
Expand Up @@ -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~'
Expand Down
161 changes: 95 additions & 66 deletions src/Connection.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
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;
Expand All @@ -26,12 +27,18 @@
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
Expand Down Expand Up @@ -114,6 +121,9 @@ class Connection implements DriverConnection
*/
private $platform;

/** @var ExceptionConverter|null */
private $exceptionConverter;

/**
* The schema manager.
*
Expand Down Expand Up @@ -284,7 +294,7 @@ public function connect()
try {
$this->_conn = $this->_driver->connect($this->params);
} catch (DriverException $e) {
throw DBALException::driverException($this->_driver, $e);
throw $this->convertException($e);
}

$this->transactionNestingLevel = 0;
Expand Down Expand Up @@ -467,8 +477,8 @@ public function fetchAssociative(string $query, array $params = [], array $types
{
try {
return $this->executeQuery($query, $params, $types)->fetchAssociative();
} catch (Throwable $e) {
$this->handleExceptionDuringQuery($e, $query, $params, $types);
} catch (DriverException $e) {
throw $this->convertExceptionDuringQuery($e, $query, $params, $types);
}
}

Expand All @@ -488,8 +498,8 @@ public function fetchNumeric(string $query, array $params = [], array $types = [
{
try {
return $this->executeQuery($query, $params, $types)->fetchNumeric();
} catch (Throwable $e) {
$this->handleExceptionDuringQuery($e, $query, $params, $types);
} catch (DriverException $e) {
throw $this->convertExceptionDuringQuery($e, $query, $params, $types);
}
}

Expand All @@ -509,8 +519,8 @@ public function fetchOne(string $query, array $params = [], array $types = [])
{
try {
return $this->executeQuery($query, $params, $types)->fetchOne();
} catch (Throwable $e) {
$this->handleExceptionDuringQuery($e, $query, $params, $types);
} catch (DriverException $e) {
throw $this->convertExceptionDuringQuery($e, $query, $params, $types);
}
}

Expand Down Expand Up @@ -771,8 +781,8 @@ public function fetchAllNumeric(string $query, array $params = [], array $types
{
try {
return $this->executeQuery($query, $params, $types)->fetchAllNumeric();
} catch (Throwable $e) {
$this->handleExceptionDuringQuery($e, $query, $params, $types);
} catch (DriverException $e) {
throw $this->convertExceptionDuringQuery($e, $query, $params, $types);
}
}

Expand All @@ -791,8 +801,8 @@ public function fetchAllAssociative(string $query, array $params = [], array $ty
{
try {
return $this->executeQuery($query, $params, $types)->fetchAllAssociative();
} catch (Throwable $e) {
$this->handleExceptionDuringQuery($e, $query, $params, $types);
} catch (DriverException $e) {
throw $this->convertExceptionDuringQuery($e, $query, $params, $types);
}
}

Expand All @@ -811,8 +821,8 @@ public function fetchFirstColumn(string $query, array $params = [], array $types
{
try {
return $this->executeQuery($query, $params, $types)->fetchFirstColumn();
} catch (Throwable $e) {
$this->handleExceptionDuringQuery($e, $query, $params, $types);
} catch (DriverException $e) {
throw $this->convertExceptionDuringQuery($e, $query, $params, $types);
}
}

Expand All @@ -835,8 +845,8 @@ public function iterateNumeric(string $query, array $params = [], array $types =
while (($row = $result->fetchNumeric()) !== false) {
yield $row;
}
} catch (Throwable $e) {
$this->handleExceptionDuringQuery($e, $query, $params, $types);
} catch (DriverException $e) {
throw $this->convertExceptionDuringQuery($e, $query, $params, $types);
}
}

Expand All @@ -859,8 +869,8 @@ public function iterateAssociative(string $query, array $params = [], array $typ
while (($row = $result->fetchAssociative()) !== false) {
yield $row;
}
} catch (Throwable $e) {
$this->handleExceptionDuringQuery($e, $query, $params, $types);
} catch (DriverException $e) {
throw $this->convertExceptionDuringQuery($e, $query, $params, $types);
}
}

Expand All @@ -883,8 +893,8 @@ public function iterateColumn(string $query, array $params = [], array $types =
while (($value = $result->fetchOne()) !== false) {
yield $value;
}
} catch (Throwable $e) {
$this->handleExceptionDuringQuery($e, $query, $params, $types);
} catch (DriverException $e) {
throw $this->convertExceptionDuringQuery($e, $query, $params, $types);
}
}

Expand All @@ -899,11 +909,7 @@ public function iterateColumn(string $query, array $params = [], array $types =
*/
public function prepare(string $sql): DriverStatement
{
try {
return new Statement($sql, $this);
} catch (Throwable $e) {
$this->handleExceptionDuringQuery($e, $sql);
}
return new Statement($sql, $this);
}

/**
Expand Down Expand Up @@ -952,8 +958,8 @@ public function executeQuery(
}

return new Result($result, $this);
} catch (Throwable $e) {
$this->handleExceptionDuringQuery($e, $query, $params, $types);
} catch (DriverException $e) {
throw $this->convertExceptionDuringQuery($e, $query, $params, $types);
} finally {
if ($logger !== null) {
$logger->stopQuery();
Expand Down Expand Up @@ -1010,6 +1016,9 @@ public function executeCacheQuery($query, $params, $types, QueryCacheProfile $qc
return new Result($result, $this);
}

/**
* @throws DBALException
*/
public function query(string $sql): DriverResult
{
$connection = $this->getWrappedConnection();
Expand All @@ -1021,8 +1030,8 @@ public function query(string $sql): DriverResult

try {
return $connection->query($sql);
} catch (Throwable $e) {
$this->handleExceptionDuringQuery($e, $sql);
} catch (DriverException $e) {
throw $this->convertExceptionDuringQuery($e, $sql);
} finally {
if ($logger !== null) {
$logger->stopQuery();
Expand Down Expand Up @@ -1069,15 +1078,18 @@ public function executeUpdate(string $query, array $params = [], array $types =
}

return $connection->exec($query);
} catch (Throwable $e) {
$this->handleExceptionDuringQuery($e, $query, $params, $types);
} catch (DriverException $e) {
throw $this->convertExceptionDuringQuery($e, $query, $params, $types);
} finally {
if ($logger !== null) {
$logger->stopQuery();
}
}
}

/**
* @throws DBALException
*/
public function exec(string $statement): int
{
$connection = $this->getWrappedConnection();
Expand All @@ -1089,8 +1101,8 @@ public function exec(string $statement): int

try {
return $connection->exec($statement);
} catch (Throwable $e) {
$this->handleExceptionDuringQuery($e, $statement);
} catch (DriverException $e) {
throw $this->convertExceptionDuringQuery($e, $statement);
} finally {
if ($logger !== null) {
$logger->stopQuery();
Expand Down Expand Up @@ -1578,15 +1590,12 @@ private function getBindingInfo($value, $type)
/**
* 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 = [];

Expand Down Expand Up @@ -1638,53 +1647,73 @@ public function createQueryBuilder()
*
* @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
{
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 ($e instanceof ConnectionLost) {
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;
}
}
4 changes: 3 additions & 1 deletion src/Connections/PrimaryReadReplicaConnection.php
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,8 @@ public function ensureConnectedToReplica(): bool
* @param string $connectionName
*
* @return DriverConnection
*
* @throws DBALException
*/
protected function connectTo($connectionName)
{
Expand All @@ -230,7 +232,7 @@ protected function connectTo($connectionName)
try {
return $this->_driver->connect($connectionParams);
} catch (DriverException $e) {
throw DBALException::driverException($this->_driver, $e);
throw $this->convertException($e);
}
}

Expand Down
Loading