Aaron Schulz has uploaded a new change for review.
https://gerrit.wikimedia.org/r/310720
Change subject: Avoid MWException and wf* log methods in DatabaseBase
......................................................................
Avoid MWException and wf* log methods in DatabaseBase
Change-Id: Idc418ae1088f87d6416e2552976d94f7d1e8f5db
---
M includes/db/Database.php
M includes/db/DatabaseMssql.php
M includes/db/DatabaseSqlite.php
M includes/db/loadbalancer/LBFactoryMulti.php
M includes/db/loadbalancer/LBFactorySimple.php
M includes/db/loadbalancer/LoadBalancer.php
6 files changed, 36 insertions(+), 36 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core
refs/changes/20/310720/1
diff --git a/includes/db/Database.php b/includes/db/Database.php
index 109dbfe..17761ff 100644
--- a/includes/db/Database.php
+++ b/includes/db/Database.php
@@ -69,6 +69,8 @@
protected $connLogger;
/** @var LoggerInterface */
protected $queryLogger;
+ /** @var callback Error logging callback */
+ protected $errorLogger;
/** @var resource Database connection */
protected $mConn = null;
@@ -318,7 +320,7 @@
* @param string $dbType A possible DB type
* @param array $p An array of options to pass to the constructor.
* Valid options are: host, user, password, dbname, flags,
tablePrefix, schema, driver
- * @throws MWException If the database driver or extension cannot be
found
+ * @throws InvalidArgumentException If the database driver or extension
cannot be found
* @return DatabaseBase|null DatabaseBase subclass or null
*/
final public static function factory( $dbType, $p = [] ) {
@@ -355,7 +357,7 @@
$driver = $dbType;
}
if ( $driver === false ) {
- throw new MWException( __METHOD__ .
+ throw new InvalidArgumentException( __METHOD__ .
" no viable database extension found for type
'$dbType'" );
}
@@ -390,6 +392,11 @@
if ( isset( $p['queryLogger'] ) ) {
$conn->queryLogger = $p['queryLogger'];
}
+ if ( isset( $p['errorLogger'] ) ) {
+ $conn->errorLogger = $p['errorLogger'];
+ } else {
+ $conn->errorLogger = [
MWExceptionHandler::class, 'logException' ];
+ }
} else {
$conn = null;
}
@@ -398,7 +405,7 @@
}
public function setLogger( LoggerInterface $logger ) {
- $this->quertLogger = $logger;
+ $this->queryLogger = $logger;
}
public function getServerInfo() {
@@ -747,7 +754,7 @@
protected function installErrorHandler() {
$this->mPHPError = false;
$this->htmlErrors = ini_set( 'html_errors', '0' );
- set_error_handler( [ $this, 'connectionErrorHandler' ] );
+ set_error_handler( [ $this, 'connectionerrorLogger' ] );
}
/**
@@ -772,12 +779,12 @@
* @param int $errno
* @param string $errstr
*/
- public function connectionErrorHandler( $errno, $errstr ) {
+ public function connectionerrorLogger( $errno, $errstr ) {
$this->mPHPError = $errstr;
}
/**
- * Create a log context to pass to wfLogDBError or other logging
functions.
+ * Create a log context to pass to PSR logging functions.
*
* @param array $extras Additional data to add to context
* @return array
@@ -796,11 +803,6 @@
public function close() {
if ( $this->mConn ) {
if ( $this->trxLevel() ) {
- if ( !$this->mTrxAutomatic ) {
- wfWarn( "Transaction still in progress
(from {$this->mTrxFname}), " .
- " performing implicit commit
before closing connection!" );
- }
-
$this->commit( __METHOD__,
self::FLUSHING_INTERNAL );
}
@@ -954,7 +956,8 @@
if ( $this->reconnect() ) {
$msg = __METHOD__ . ": lost connection to
{$this->getServer()}; reconnected";
$this->connLogger->warning( $msg );
- $this->queryLogger->warning( "$msg:\n" .
wfBacktrace( true ) );
+ $this->queryLogger->warning(
+ "$msg:\n" . ( new RuntimeException()
)->getTraceAsString() );
if ( !$recoverable ) {
# Callers may catch the exception and
continue to use the DB
@@ -1103,7 +1106,7 @@
$this->queryLogger->debug( "SQL ERROR (ignored):
$error\n" );
} else {
$sql1line = mb_substr( str_replace( "\n", "\\n", $sql
), 0, 5 * 1024 );
- wfLogDBError(
+ $this->queryLogger->error(
"{fname}\t{db_server}\t{errno}\t{error}\t{sql1line}",
$this->getLogContext( [
'method' => __METHOD__,
@@ -2812,7 +2815,7 @@
$this->clearFlag( DBO_TRX ); //
restore auto-commit
}
} catch ( Exception $ex ) {
- MWExceptionHandler::logException( $ex );
+ call_user_func( $this->errorLogger, $ex
);
$e = $e ?: $ex;
// Some callbacks may use
startAtomic/endAtomic, so make sure
// their transactions are ended so
other callbacks don't fail
@@ -2846,7 +2849,7 @@
list( $phpCallback ) = $callback;
call_user_func( $phpCallback );
} catch ( Exception $ex ) {
- MWExceptionHandler::logException( $ex );
+ call_user_func( $this->errorLogger, $ex
);
$e = $e ?: $ex;
}
}
@@ -2879,7 +2882,7 @@
list( $phpCallback ) = $callback;
$phpCallback( $trigger, $this );
} catch ( Exception $ex ) {
- MWExceptionHandler::logException( $ex );
+ call_user_func( $this->errorLogger, $ex );
$e = $e ?: $ex;
}
}
@@ -2943,15 +2946,13 @@
} else {
// @TODO: make this an exception at some point
$msg = "$fname: Implicit transaction already
active (from {$this->mTrxFname}).";
- wfLogDBError( $msg );
- wfWarn( $msg );
+ $this->queryLogger->error( $msg );
return; // join the main transaction set
}
} elseif ( $this->getFlag( DBO_TRX ) && $mode !==
self::TRANSACTION_INTERNAL ) {
// @TODO: make this an exception at some point
$msg = "$fname: Implicit transaction expected (DBO_TRX
set).";
- wfLogDBError( $msg );
- wfWarn( $msg );
+ $this->queryLogger->error( $msg );
return; // let any writes be in the main transaction
}
@@ -3010,13 +3011,12 @@
}
} else {
if ( !$this->mTrxLevel ) {
- wfWarn( "$fname: No transaction to commit,
something got out of sync." );
+ $this->queryLogger->error( "$fname: No
transaction to commit, something got out of sync." );
return; // nothing to do
} elseif ( $this->mTrxAutomatic ) {
// @TODO: make this an exception at some point
$msg = "$fname: Explicit commit of implicit
transaction.";
- wfLogDBError( $msg );
- wfWarn( $msg );
+ $this->queryLogger->error( $msg );
return; // wait for the main transaction set
commit round
}
}
@@ -3057,7 +3057,8 @@
}
} else {
if ( !$this->mTrxLevel ) {
- wfWarn( "$fname: No transaction to rollback,
something got out of sync." );
+ $this->queryLogger->error(
+ "$fname: No transaction to rollback,
something got out of sync." );
return; // nothing to do
} elseif ( $this->getFlag( DBO_TRX ) ) {
throw new DBUnexpectedError(
@@ -3126,7 +3127,7 @@
* @param string $newName Name of table to be created
* @param bool $temporary Whether the new table should be temporary
* @param string $fname Calling function name
- * @throws MWException
+ * @throws RuntimeException
* @return bool True if operation was successful
*/
public function duplicateTableStructure( $oldName, $newName, $temporary
= false,
@@ -3156,7 +3157,7 @@
*
* @param string $prefix Only show VIEWs with this prefix, eg.
unit_test_
* @param string $fname Name of calling function
- * @throws MWException
+ * @throws RuntimeException
* @return array
* @since 1.22
*/
@@ -3168,7 +3169,7 @@
* Differentiates between a TABLE and a VIEW
*
* @param string $name Name of the database-structure to test.
- * @throws MWException
+ * @throws RuntimeException
* @return bool
* @since 1.22
*/
@@ -3357,8 +3358,8 @@
* generated dynamically using $filename
* @param bool|callable $inputCallback Optional function called for each
* complete line sent
- * @throws Exception|MWException
* @return bool|string
+ * @throws Exception
*/
public function sourceFile(
$filename, $lineCallback = false, $resultCallback = false,
$fname = false, $inputCallback = false
diff --git a/includes/db/DatabaseMssql.php b/includes/db/DatabaseMssql.php
index 269a248..afccfd7 100644
--- a/includes/db/DatabaseMssql.php
+++ b/includes/db/DatabaseMssql.php
@@ -777,7 +777,6 @@
* @return bool
* @throws DBUnexpectedError
* @throws Exception
- * @throws MWException
*/
function update( $table, $values, $conds, $fname = __METHOD__, $options
= [] ) {
$table = $this->tableName( $table );
@@ -814,7 +813,7 @@
* @param array $binaryColumns Contains a list of column names that are
binary types
* This is a custom parameter only present for MS SQL.
*
- * @throws MWException|DBUnexpectedError
+ * @throws DBUnexpectedError
* @return string
*/
public function makeList( $a, $mode = LIST_COMMA, $binaryColumns = [] )
{
@@ -1075,7 +1074,7 @@
* Throws an exception if it is invalid.
* Reference:
http://msdn.microsoft.com/en-us/library/aa224033%28v=SQL.80%29.aspx
* @param string $identifier
- * @throws MWException
+ * @throws InvalidArgumentException
* @return string
*/
private function escapeIdentifier( $identifier ) {
diff --git a/includes/db/DatabaseSqlite.php b/includes/db/DatabaseSqlite.php
index ef08ab0..b60f343 100644
--- a/includes/db/DatabaseSqlite.php
+++ b/includes/db/DatabaseSqlite.php
@@ -950,12 +950,12 @@
}
/**
- * @throws MWException
* @param string $oldName
* @param string $newName
* @param bool $temporary
* @param string $fname
* @return bool|ResultWrapper
+ * @throws RuntimeException
*/
function duplicateTableStructure( $oldName, $newName, $temporary =
false, $fname = __METHOD__ ) {
$res = $this->query( "SELECT sql FROM sqlite_master WHERE
tbl_name=" .
diff --git a/includes/db/loadbalancer/LBFactoryMulti.php
b/includes/db/loadbalancer/LBFactoryMulti.php
index dd7737b..bbee3b8 100644
--- a/includes/db/loadbalancer/LBFactoryMulti.php
+++ b/includes/db/loadbalancer/LBFactoryMulti.php
@@ -164,7 +164,7 @@
/**
* @param array $conf
- * @throws MWException
+ * @throws InvalidArgumentException
*/
public function __construct( array $conf ) {
parent::__construct( $conf );
@@ -264,7 +264,7 @@
/**
* @param string $cluster
* @param bool|string $wiki
- * @throws MWException
+ * @throws InvalidArgumentException
* @return LoadBalancer
*/
protected function newExternalLB( $cluster, $wiki = false ) {
diff --git a/includes/db/loadbalancer/LBFactorySimple.php
b/includes/db/loadbalancer/LBFactorySimple.php
index d8590b7..908453c 100644
--- a/includes/db/loadbalancer/LBFactorySimple.php
+++ b/includes/db/loadbalancer/LBFactorySimple.php
@@ -102,10 +102,10 @@
}
/**
- * @throws MWException
* @param string $cluster
* @param bool|string $wiki
* @return LoadBalancer
+ * @throws InvalidArgumentException
*/
protected function newExternalLB( $cluster, $wiki = false ) {
global $wgExternalServers;
diff --git a/includes/db/loadbalancer/LoadBalancer.php
b/includes/db/loadbalancer/LoadBalancer.php
index 841231b..dea9973 100644
--- a/includes/db/loadbalancer/LoadBalancer.php
+++ b/includes/db/loadbalancer/LoadBalancer.php
@@ -776,7 +776,7 @@
* @param bool $dbNameOverride
* @return DatabaseBase
* @throws DBAccessError
- * @throws MWException
+ * @throws InvalidArgumentException
*/
protected function reallyOpenConnection( $server, $dbNameOverride =
false ) {
if ( $this->disabled ) {
--
To view, visit https://gerrit.wikimedia.org/r/310720
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Idc418ae1088f87d6416e2552976d94f7d1e8f5db
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Aaron Schulz <asch...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
