Overhauled error handling in Database classes
authorAlexander Ebert <ebert@woltlab.com>
Mon, 15 Dec 2014 15:56:35 +0000 (16:56 +0100)
committerAlexander Ebert <ebert@woltlab.com>
Mon, 15 Dec 2014 15:56:35 +0000 (16:56 +0100)
Since we're no longer using prepared statement emulation, prepares fail at another stage which did not properly report the actual error message.

Additionally I've changed the error mode to actually throw exceptions rather than bogus non-success return values like 'false'.

wcfsetup/install/files/lib/system/database/Database.class.php
wcfsetup/install/files/lib/system/database/DatabaseException.class.php
wcfsetup/install/files/lib/system/database/MySQLDatabase.class.php

index 5ef87d8e0d796a3179656e9947946680ea4db42d..e8f727ea2dc7f700ce668700b2cb6833ec1644c0 100644 (file)
@@ -124,7 +124,7 @@ abstract class Database {
                        return $this->pdo->lastInsertId();
                }
                catch (\PDOException $e) {
-                       throw new DatabaseException("Cannot fetch last insert id", $this);
+                       throw new DatabaseException("Cannot fetch last insert id: " . $e->getMessage(), $this);
                }
        }
        
@@ -150,7 +150,7 @@ abstract class Database {
                        return $result;
                }
                catch (\PDOException $e) {
-                       throw new DatabaseException("Cannot begin transaction", $this);
+                       throw new DatabaseException("Cannot begin transaction: " . $e->getMessage(), $this);
                }
        }
        
@@ -179,7 +179,7 @@ abstract class Database {
                        return $result;
                }
                catch (\PDOException $e) {
-                       throw new DatabaseException("Cannot commit transaction", $this);
+                       throw new DatabaseException("Cannot commit transaction: " . $e->getMessage(), $this);
                }
        }
        
@@ -207,7 +207,7 @@ abstract class Database {
                        return $result;
                }
                catch (\PDOException $e) {
-                       throw new DatabaseException("Cannot rollback transaction", $this);
+                       throw new DatabaseException("Cannot rollback transaction: " . $e->getMessage(), $this);
                }
        }
        
@@ -224,13 +224,11 @@ abstract class Database {
                
                try {
                        $pdoStatement = $this->pdo->prepare($statement);
-                       if ($pdoStatement instanceof \PDOStatement) {
-                               return new $this->preparedStatementClassName($this, $pdoStatement, $statement);
-                       }
-                       throw new DatabaseException("Cannot prepare statement: ".$statement, $this);
+                       
+                       return new $this->preparedStatementClassName($this, $pdoStatement, $statement);
                }
                catch (\PDOException $e) {
-                       throw new DatabaseException("Cannot prepare statement: ".$statement, $this);
+                       throw new DatabaseException($e->getMessage(), $this, null, $statement);
                }
        }
        
index 4a4c8372d2905663b9f9095a78953320fd127b88..ae44fa31a91c168447e2e9e3224c1d967453c312 100644 (file)
@@ -51,17 +51,25 @@ class DatabaseException extends SystemException {
         */
        protected $preparedStatement = null;
        
+       /**
+        * SQL query if prepare() failed
+        * @var string
+        */
+       protected $sqlQuery = null;
+       
        /**
         * Creates a new DatabaseException.
         * 
         * @param       string                                                  $message                error message
         * @param       \wcf\system\database\Database                           $db                     affected db object
-        * @param       \wcf\system\database\statement\PreparedStatement                $preparedStatement      affected prepared statement
+        * @param       \wcf\system\database\statement\PreparedStatement        $preparedStatement      affected prepared statement
+        * @param       string                                                  $sqlQuery               SQL query if prepare() failed
         */
-       public function __construct($message, Database $db, PreparedStatement $preparedStatement = null) {
+       public function __construct($message, Database $db, PreparedStatement $preparedStatement = null, $sqlQuery = null) {
                $this->db = $db;
                $this->DBType = $db->getDBType();
                $this->preparedStatement = $preparedStatement;
+               $this->sqlQuery = $sqlQuery;
                
                // prefer errors from prepared statement
                if ($this->preparedStatement !== null && $this->preparedStatement->getErrorNumber()) {
@@ -138,6 +146,9 @@ class DatabaseException extends SystemException {
                                }
                        }
                }
+               else if ($this->sqlQuery !== null) {
+                       $this->information .= '<b>sql query:</b> ' . StringUtil::encodeHTML($this->sqlQuery) . '<br />';
+               }
                
                parent::show();
        }
index 8dc49a713563427109278b11389d6c99f17d30da..fbd38893e7677d561b1cd669ab55277c48dc3c31 100644 (file)
@@ -36,6 +36,9 @@ class MySQLDatabase extends Database {
                        // disable prepared statement emulation since MySQL 5.1.17 is the minimum required version
                        $driverOptions[\PDO::ATTR_EMULATE_PREPARES] = false;
                        
+                       // throw PDOException instead of dumb false return values
+                       $driverOptions[\PDO::ATTR_ERRMODE] = \PDO::ERRMODE_EXCEPTION;
+                       
                        $this->pdo = new \PDO('mysql:host='.$this->host.';port='.$this->port.';dbname='.$this->database, $this->user, $this->password, $driverOptions);
                        $this->setAttributes();
                }