Merge pull request doctrine#3536 from jwage/improve-exception-messages

Improve consistency of exception message formatting.
morozov · Aug 27, 2019 · c36f826 · c36f826
2 parents 9e5af55 + fb13d7d
commit c36f826
Show file tree

Hide file tree

Showing 88 changed files with 613 additions and 248 deletions.
diff --git a/lib/Doctrine/DBAL/Cache/ArrayStatement.php b/lib/Doctrine/DBAL/Cache/ArrayStatement.php
@@ -15,6 +15,7 @@
 use function array_values;
 use function count;
 use function reset;
+use function sprintf;
 
 class ArrayStatement implements IteratorAggregate, ResultStatement
 {
@@ -48,7 +49,7 @@ public function __construct(array $data)
      */
     public function closeCursor() : void
     {
-        unset($this->data);
+        $this->data = null;
     }
 
     /**
@@ -77,7 +78,7 @@ public function rowCount() : int
     public function setFetchMode($fetchMode, ...$args) : void
     {
         if (count($args) > 0) {
-            throw new InvalidArgumentException('Caching layer does not support 2nd/3rd argument to setFetchMode()');
+            throw new InvalidArgumentException('Caching layer does not support 2nd/3rd argument to setFetchMode().');
         }
 
         $this->defaultFetchMode = $fetchMode;
@@ -121,7 +122,9 @@ public function fetch($fetchMode = null, ...$args)
             return reset($row);
         }
 
-        throw new InvalidArgumentException('Invalid fetch-style given for fetching result.');
+        throw new InvalidArgumentException(
+            sprintf('Invalid fetch mode given for fetching result, %d given.', $fetchMode)
+        );
     }
 
     /**

diff --git a/lib/Doctrine/DBAL/DBALException.php b/lib/Doctrine/DBAL/DBALException.php
@@ -28,21 +28,28 @@ class DBALException extends Exception
      */
     public static function driverExceptionDuringQuery(Driver $driver, Throwable $driverEx, $sql, array $params = [])
     {
-        $msg = "An exception occurred while executing '" . $sql . "'";
-        if ($params) {
-            $msg .= ' with params ' . self::formatParameters($params);
-        }
-        $msg .= ":\n\n" . $driverEx->getMessage();
+        $messageFormat = <<<'MESSAGE'
+An exception occurred while executing "%s"%s:
+
+%s
+MESSAGE;
+
+        $message = sprintf(
+            $messageFormat,
+            $sql,
+            $params !== [] ? sprintf(' with params %s', self::formatParameters($params)) : '',
+            $driverEx->getMessage()
+        );
 
-        return static::wrapException($driver, $driverEx, $msg);
+        return static::wrapException($driver, $driverEx, $message);
     }
 
     /**
      * @return self
      */
     public static function driverException(Driver $driver, Throwable $driverEx)
     {
-        return static::wrapException($driver, $driverEx, 'An exception occurred in driver: ' . $driverEx->getMessage());
+        return static::wrapException($driver, $driverEx, sprintf('An exception occurred in driver with message: %s', $driverEx->getMessage()));
     }
 
     /**
@@ -64,11 +71,9 @@ private static function wrapException(Driver $driver, Throwable $driverEx, $msg)
      * 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
+     * @param array<mixed, mixed> $params
      */
-    private static function formatParameters(array $params)
+    private static function formatParameters(array $params) : string
     {
         return '[' . implode(', ', array_map(static function ($param) {
             if (is_resource($param)) {

diff --git a/lib/Doctrine/DBAL/Driver/Mysqli/Exception/ConnectionError.php b/lib/Doctrine/DBAL/Driver/Mysqli/Exception/ConnectionError.php
@@ -0,0 +1,16 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\DBAL\Driver\Mysqli\Exception;
+
+use Doctrine\DBAL\Driver\Mysqli\MysqliException;
+use mysqli;
+
+final class ConnectionError extends MysqliException
+{
+    public static function new(mysqli $connection) : self
+    {
+        return new self($connection->error, $connection->sqlstate ?: null, $connection->errno);
+    }
+}
diff --git a/lib/Doctrine/DBAL/Driver/Mysqli/Exception/FailedReadingStreamOffset.php b/lib/Doctrine/DBAL/Driver/Mysqli/Exception/FailedReadingStreamOffset.php
@@ -0,0 +1,16 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\DBAL\Driver\Mysqli\Exception;
+
+use Doctrine\DBAL\Driver\Mysqli\MysqliException;
+use function sprintf;
+
+final class FailedReadingStreamOffset extends MysqliException
+{
+    public static function new(int $offset) : self
+    {
+        return new self(sprintf('Failed reading the stream resource for parameter offset %d.', $offset));
+    }
+}
diff --git a/lib/Doctrine/DBAL/Driver/Mysqli/Exception/StatementError.php b/lib/Doctrine/DBAL/Driver/Mysqli/Exception/StatementError.php
@@ -0,0 +1,16 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\DBAL\Driver\Mysqli\Exception;
+
+use Doctrine\DBAL\Driver\Mysqli\MysqliException;
+use mysqli_stmt;
+
+final class StatementError extends MysqliException
+{
+    public static function new(mysqli_stmt $statement) : self
+    {
+        return new self($statement->error, $statement->sqlstate ?: null, $statement->errno);
+    }
+}
diff --git a/lib/Doctrine/DBAL/Driver/Mysqli/Exception/UnknownFetchMode.php b/lib/Doctrine/DBAL/Driver/Mysqli/Exception/UnknownFetchMode.php
@@ -0,0 +1,19 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\DBAL\Driver\Mysqli\Exception;
+
+use Doctrine\DBAL\Driver\Mysqli\MysqliException;
+use function sprintf;
+
+final class UnknownFetchMode extends MysqliException
+{
+    /**
+     * @param mixed $fetchMode
+     */
+    public static function new($fetchMode) : self
+    {
+        return new self(sprintf('Unknown fetch mode %d.', $fetchMode));
+    }
+}
diff --git a/lib/Doctrine/DBAL/Driver/Mysqli/Exception/UnknownType.php b/lib/Doctrine/DBAL/Driver/Mysqli/Exception/UnknownType.php
@@ -0,0 +1,19 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\DBAL\Driver\Mysqli\Exception;
+
+use Doctrine\DBAL\Driver\Mysqli\MysqliException;
+use function sprintf;
+
+final class UnknownType extends MysqliException
+{
+    /**
+     * @param mixed $type
+     */
+    public static function new($type) : self
+    {
+        return new self(sprintf('Unknown type, %d given.', $type));
+    }
+}
diff --git a/lib/Doctrine/DBAL/Driver/Mysqli/MysqliConnection.php b/lib/Doctrine/DBAL/Driver/Mysqli/MysqliConnection.php
@@ -5,6 +5,7 @@
 namespace Doctrine\DBAL\Driver\Mysqli;
 
 use Doctrine\DBAL\Driver\Connection;
+use Doctrine\DBAL\Driver\Mysqli\Exception\ConnectionError;
 use Doctrine\DBAL\Driver\PingableConnection;
 use Doctrine\DBAL\Driver\ResultStatement;
 use Doctrine\DBAL\Driver\ServerInfoAwareConnection;
@@ -73,7 +74,7 @@ public function __construct(array $params, $username, $password, array $driverOp
         });
         try {
             if (! $this->conn->real_connect($host, $username, $password, $dbname, $port, $socket, $flags)) {
-                throw MysqliException::fromConnectionError($this->conn);
+                throw ConnectionError::new($this->conn);
             }
         } finally {
             restore_error_handler();
@@ -161,7 +162,7 @@ public function quote(string $input) : string
     public function exec(string $statement) : int
     {
         if ($this->conn->query($statement) === false) {
-            throw MysqliException::fromConnectionError($this->conn);
+            throw ConnectionError::new($this->conn);
         }
 
         return $this->conn->affected_rows;
@@ -189,7 +190,7 @@ public function beginTransaction() : void
     public function commit() : void
     {
         if (! $this->conn->commit()) {
-            throw MysqliException::fromConnectionError($this->conn);
+            throw ConnectionError::new($this->conn);
         }
     }
 
@@ -199,7 +200,7 @@ public function commit() : void
     public function rollBack() : void
     {
         if (! $this->conn->rollback()) {
-            throw MysqliException::fromConnectionError($this->conn);
+            throw ConnectionError::new($this->conn);
         }
     }
 
@@ -242,7 +243,7 @@ private function setDriverOptions(array $driverOptions = [])
                 continue;
             }
 
-            throw MysqliException::fromConnectionError($this->conn);
+            throw ConnectionError::new($this->conn);
         }
     }
 

diff --git a/lib/Doctrine/DBAL/Driver/Mysqli/MysqliException.php b/lib/Doctrine/DBAL/Driver/Mysqli/MysqliException.php
@@ -5,21 +5,10 @@
 namespace Doctrine\DBAL\Driver\Mysqli;
 
 use Doctrine\DBAL\Driver\AbstractDriverException;
-use mysqli;
-use mysqli_stmt;
 
 /**
  * Exception thrown in case the mysqli driver errors.
  */
 class MysqliException extends AbstractDriverException
 {
-    public static function fromConnectionError(mysqli $connection) : self
-    {
-        return new self($connection->error, $connection->sqlstate ?: null, $connection->errno);
-    }
-
-    public static function fromStatementError(mysqli_stmt $statement) : self
-    {
-        return new self($statement->error, $statement->sqlstate ?: null, $statement->errno);
-    }
 }
diff --git a/lib/Doctrine/DBAL/Driver/Mysqli/MysqliStatement.php b/lib/Doctrine/DBAL/Driver/Mysqli/MysqliStatement.php
@@ -5,6 +5,11 @@
 namespace Doctrine\DBAL\Driver\Mysqli;
 
 use Doctrine\DBAL\Driver\DriverException;
+use Doctrine\DBAL\Driver\Mysqli\Exception\ConnectionError;
+use Doctrine\DBAL\Driver\Mysqli\Exception\FailedReadingStreamOffset;
+use Doctrine\DBAL\Driver\Mysqli\Exception\StatementError;
+use Doctrine\DBAL\Driver\Mysqli\Exception\UnknownFetchMode;
+use Doctrine\DBAL\Driver\Mysqli\Exception\UnknownType;
 use Doctrine\DBAL\Driver\Statement;
 use Doctrine\DBAL\Driver\StatementIterator;
 use Doctrine\DBAL\Exception\InvalidArgumentException;
@@ -25,7 +30,6 @@
 use function is_array;
 use function is_int;
 use function is_resource;
-use function sprintf;
 use function str_repeat;
 
 class MysqliStatement implements IteratorAggregate, Statement
@@ -87,7 +91,7 @@ public function __construct(mysqli $conn, $prepareString)
         $stmt = $conn->prepare($prepareString);
 
         if ($stmt === false) {
-            throw MysqliException::fromConnectionError($this->_conn);
+            throw ConnectionError::new($this->_conn);
         }
 
         $this->_stmt = $stmt;
@@ -109,7 +113,7 @@ public function bindParam($column, &$variable, $type = ParameterType::STRING, $l
         assert(is_int($column));
 
         if (! isset(self::$_paramTypeMap[$type])) {
-            throw new MysqliException(sprintf("Unknown type: '%s'", $type));
+            throw UnknownType::new($type);
         }
 
         $this->_bindedValues[$column] =& $variable;
@@ -124,7 +128,7 @@ public function bindValue($param, $value, $type = ParameterType::STRING) : void
         assert(is_int($param));
 
         if (! isset(self::$_paramTypeMap[$type])) {
-            throw new MysqliException(sprintf("Unknown type: '%s'", $type));
+            throw UnknownType::new($type);
         }
 
         $this->_values[$param]       = $value;
@@ -139,14 +143,14 @@ public function execute($params = null) : void
     {
         if ($params !== null && count($params) > 0) {
             if (! $this->bindUntypedValues($params)) {
-                throw MysqliException::fromStatementError($this->_stmt);
+                throw StatementError::new($this->_stmt);
             }
         } else {
             $this->bindTypedParameters();
         }
 
         if (! $this->_stmt->execute()) {
-            throw MysqliException::fromStatementError($this->_stmt);
+            throw StatementError::new($this->_stmt);
         }
 
         if ($this->_columnNames === null) {
@@ -193,7 +197,7 @@ public function execute($params = null) : void
             }
 
             if (! $this->_stmt->bind_result(...$refs)) {
-                throw MysqliException::fromStatementError($this->_stmt);
+                throw StatementError::new($this->_stmt);
             }
         }
 
@@ -232,7 +236,7 @@ private function bindTypedParameters()
         }
 
         if (count($values) > 0 && ! $this->_stmt->bind_param($types, ...$values)) {
-            throw MysqliException::fromStatementError($this->_stmt);
+            throw StatementError::new($this->_stmt);
         }
 
         $this->sendLongData($streams);
@@ -250,11 +254,11 @@ private function sendLongData($streams)
                 $chunk = fread($stream, 8192);
 
                 if ($chunk === false) {
-                    throw new MysqliException("Failed reading the stream resource for parameter offset ${paramNr}.");
+                    throw FailedReadingStreamOffset::new($paramNr);
                 }
 
                 if (! $this->_stmt->send_long_data($paramNr - 1, $chunk)) {
-                    throw MysqliException::fromStatementError($this->_stmt);
+                    throw StatementError::new($this->_stmt);
                 }
             }
         }
@@ -322,7 +326,7 @@ public function fetch($fetchMode = null, ...$args)
         }
 
         if ($values === false) {
-            throw MysqliException::fromStatementError($this->_stmt);
+            throw StatementError::new($this->_stmt);
         }
 
         if ($fetchMode === FetchMode::NUMERIC) {
@@ -344,7 +348,7 @@ public function fetch($fetchMode = null, ...$args)
                 return (object) $assoc;
 
             default:
-                throw new MysqliException(sprintf("Unknown fetch type '%s'", $fetchMode));
+                throw UnknownFetchMode::new($fetchMode);
         }
     }
 

diff --git a/lib/Doctrine/DBAL/Driver/OCI8/OCI8Statement.php b/lib/Doctrine/DBAL/Driver/OCI8/OCI8Statement.php
@@ -160,7 +160,7 @@ public static function convertPositionalToNamedPlaceholders($statement)
 
         if ($currentLiteralDelimiter) {
             throw new OCI8Exception(sprintf(
-                'The statement contains non-terminated string literal starting at offset %d',
+                'The statement contains non-terminated string literal starting at offset %d.',
                 $tokenOffset - 1
             ));
         }
@@ -411,7 +411,7 @@ public function fetch($fetchMode = null, ...$args)
         }
 
         if (! isset(self::$fetchModeMap[$fetchMode])) {
-            throw new InvalidArgumentException('Invalid fetch style: ' . $fetchMode);
+            throw new InvalidArgumentException(sprintf('Invalid fetch mode %d.', $fetchMode));
         }
 
         return oci_fetch_array(
@@ -438,7 +438,7 @@ public function fetchAll($fetchMode = null, ...$args)
         }
 
         if (! isset(self::$fetchModeMap[$fetchMode])) {
-            throw new InvalidArgumentException('Invalid fetch style: ' . $fetchMode);
+            throw new InvalidArgumentException(sprintf('Invalid fetch mode %d.', $fetchMode));
         }
 
         if (self::$fetchModeMap[$fetchMode] === OCI_BOTH) {