rdbms: Replace reportConnectionError() with direct throws

When reading through DatabaseMysqlBase::open(), it was not
obvious that execution would not continue after the conditional
`!$this->conn` block, given it ends in a method call, without
return or throw. I considered adding a return statement after it
for clarity, but it seems in this case it might make more sense
to throw directly given $error here has already gone through a
fallback to getLastError() a few lines up.

Replace the other three calls to reportConnectionError() as well,
which previously passed a useful string that was overwritten
with lastError(). Instead, log both. And make their call to
queryLogger->error() match the previous ones to have an 'error' as well.

This leaves reportConnectionError() as being unused, except for
a call from LoadBalancer. That call was problematic because
it was inside a conditional for IDatabase, but the method isn't
part of that interface. Replace it with a direct throw as well.

Deprecate the method as its now unused in core, and also remove its
'# New method' comment which hasn't made sense since r75341 (16cded8b32).

Change-Id: I0f2ef00ba44bf7090a3ce54edeb8c7e8e543e46a

diff --git a/includes/libs/rdbms/database/Database.php b/includes/libs/rdbms/database/Database.php
index 5549f28..c6b99ef 100644 (file)
--- a/includes/libs/rdbms/database/Database.php
+++ b/includes/libs/rdbms/database/Database.php
@@ -991,17 +991,13 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
        abstract protected function closeConnection();
 
        /**
-        * @param string $error Fallback error message, used if none is given by DB
+        * @deprecated since 1.32
+        * @param string $error Fallback message, if none is given by DB
         * @throws DBConnectionError
         */
        public function reportConnectionError( $error = 'Unknown error' ) {
-               $myError = $this->lastError();
-               if ( $myError ) {
-                       $error = $myError;
-               }
-
-               # New method
-               throw new DBConnectionError( $this, $error );
+               call_user_func( $this->deprecationLogger, 'Use of ' . __METHOD__ . ' is deprecated.' );
+               throw new DBConnectionError( $this, $this->lastError() ?: $error );
        }
 
        /**

diff --git a/includes/libs/rdbms/database/DatabaseMysqlBase.php b/includes/libs/rdbms/database/DatabaseMysqlBase.php
index 3e6190c..0472139 100644 (file)
--- a/includes/libs/rdbms/database/DatabaseMysqlBase.php
+++ b/includes/libs/rdbms/database/DatabaseMysqlBase.php
@@ -152,9 +152,7 @@ abstract class DatabaseMysqlBase extends Database {
 
                # Always log connection errors
                if ( !$this->conn ) {
-                       if ( !$error ) {
-                               $error = $this->lastError();
-                       }
+                       $error = $error ?: $this->lastError();
                        $this->connLogger->error(
                                "Error connecting to {db_server}: {error}",
                                $this->getLogContext( [
@@ -166,7 +164,7 @@ abstract class DatabaseMysqlBase extends Database {
                                "Server: $server, User: $user, Password: " .
                                substr( $password, 0, 3 ) . "..., error: " . $error . "\n" );
 
-                       $this->reportConnectionError( $error );
+                       throw new DBConnectionError( $this, $error );
                }
 
                if ( strlen( $dbName ) ) {
@@ -174,22 +172,29 @@ abstract class DatabaseMysqlBase extends Database {
                        $success = $this->selectDB( $dbName );
                        Wikimedia\restoreWarnings();
                        if ( !$success ) {
+                               $error = $this->lastError();
                                $this->queryLogger->error(
-                                       "Error selecting database {db_name} on server {db_server}",
+                                       "Error selecting database {db_name} on server {db_server}: {error}",
                                        $this->getLogContext( [
                                                'method' => __METHOD__,
+                                               'error' => $error,
                                        ] )
                                );
-                               $this->queryLogger->debug(
-                                       "Error selecting database $dbName on server {$this->server}" );
-
-                               $this->reportConnectionError( "Error selecting database $dbName" );
+                               throw new DBConnectionError( $this, "Error selecting database $dbName: $error" );
                        }
                }
 
                // Tell the server what we're communicating with
                if ( !$this->connectInitCharset() ) {
-                       $this->reportConnectionError( "Error setting character set" );
+                       $error = $this->lastError();
+                       $this->queryLogger->error(
+                               "Error setting character set: {error}",
+                               $this->getLogContext( [
+                                       'method' => __METHOD__,
+                                       'error' => $this->lastError(),
+                               ] )
+                       );
+                       throw new DBConnectionError( $this, "Error setting character set: $error" );
                }
 
                // Abstract over any insane MySQL defaults
@@ -212,14 +217,15 @@ abstract class DatabaseMysqlBase extends Database {
                        // Use doQuery() to avoid opening implicit transactions (DBO_TRX)
                        $success = $this->doQuery( 'SET ' . implode( ', ', $set ) );
                        if ( !$success ) {
+                               $error = $this->lastError();
                                $this->queryLogger->error(
-                                       'Error setting MySQL variables on server {db_server} (check $wgSQLMode)',
+                                       'Error setting MySQL variables on server {db_server}: {error}',
                                        $this->getLogContext( [
                                                'method' => __METHOD__,
+                                               'error' => $error,
                                        ] )
                                );
-                               $this->reportConnectionError(
-                                       'Error setting MySQL variables on server {db_server} (check $wgSQLMode)' );
+                               throw new DBConnectionError( $this, "Error setting MySQL variables: $error" );
                        }
                }
 

diff --git a/includes/libs/rdbms/loadbalancer/LoadBalancer.php b/includes/libs/rdbms/loadbalancer/LoadBalancer.php
index f0175c9..8de6064 100644 (file)
--- a/includes/libs/rdbms/loadbalancer/LoadBalancer.php
+++ b/includes/libs/rdbms/loadbalancer/LoadBalancer.php
@@ -1130,8 +1130,7 @@ class LoadBalancer implements ILoadBalancer {
                                $context
                        );
 
-                       // throws DBConnectionError
-                       $conn->reportConnectionError( "{$this->lastError} ({$context['db_server']})" );
+                       throw new DBConnectionError( $conn, "{$this->lastError} ({$context['db_server']})" );
                } else {
                        // No last connection, probably due to all servers being too busy
                        $this->connLogger->error(