Exception cleanups for LoadBalancer
authorAaron Schulz <aschulz@wikimedia.org>
Wed, 14 Sep 2016 09:28:18 +0000 (02:28 -0700)
committerAaron Schulz <aschulz@wikimedia.org>
Thu, 15 Sep 2016 01:32:43 +0000 (01:32 +0000)
* Make the error message itself be dumb/raw.
* Make the exception exposer show the same GUI message.
* Remove overzelous wgLang check in MWExceptionRenderer.

Change-Id: Ifffff3b3cc785ea3080e4975efe33b3c2cf304d6

autoload.php
includes/db/loadbalancer/LoadBalancer.php
includes/exception/MWExceptionRenderer.php
includes/libs/rdbms/exception/DBError.php

index 66736b3..b9fd6bd 100644 (file)
@@ -311,6 +311,7 @@ $wgAutoloadLocalClasses = [
        'DBReplicationWaitError' => __DIR__ . '/includes/libs/rdbms/exception/DBError.php',
        'DBSiteStore' => __DIR__ . '/includes/site/DBSiteStore.php',
        'DBTransactionError' => __DIR__ . '/includes/libs/rdbms/exception/DBError.php',
+       'DBTransactionSizeError' => __DIR__ . '/includes/libs/rdbms/exception/DBError.php',
        'DBUnexpectedError' => __DIR__ . '/includes/libs/rdbms/exception/DBError.php',
        'DataUpdate' => __DIR__ . '/includes/deferred/DataUpdate.php',
        'Database' => __DIR__ . '/includes/db/Database.php',
index 841231b..b5c6f98 100644 (file)
@@ -784,7 +784,8 @@ class LoadBalancer implements ILoadBalancer {
                }
 
                if ( !is_array( $server ) ) {
-                       throw new InvalidArgumentException( 'You must update your load-balancing configuration. ' .
+                       throw new InvalidArgumentException(
+                               'You must update your load-balancing configuration. ' .
                                'See DefaultSettings.php entry for $wgDBservers.' );
                }
 
@@ -1034,10 +1035,10 @@ class LoadBalancer implements ILoadBalancer {
                        // If this fails, then all DB transactions will be rollback back together.
                        $time = $conn->pendingWriteQueryDuration( $conn::ESTIMATE_DB_APPLY );
                        if ( $limit > 0 && $time > $limit ) {
-                               throw new DBTransactionError(
+                               throw new DBTransactionSizeError(
                                        $conn,
                                        "Transaction spent $time second(s) in writes, exceeding the $limit limit.",
-                                       wfMessage( 'transaction-duration-limit-exceeded', $time, $limit )->text()
+                                       [ $time, $limit ]
                                );
                        }
                        // If a connection sits idle while slow queries execute on another, that connection
index f455191..58b4ac7 100644 (file)
@@ -134,15 +134,13 @@ class MWExceptionRenderer {
         */
        private static function useOutputPage( Exception $e ) {
                // Can the extension use the Message class/wfMessage to get i18n-ed messages?
-               $useMessageCache = ( $GLOBALS['wgLang'] instanceof Language );
                foreach ( $e->getTrace() as $frame ) {
                        if ( isset( $frame['class'] ) && $frame['class'] === 'LocalisationCache' ) {
-                               $useMessageCache = false;
+                               return false;
                        }
                }
 
                return (
-                       $useMessageCache &&
                        !empty( $GLOBALS['wgFullyInitialised'] ) &&
                        !empty( $GLOBALS['wgOut'] ) &&
                        !defined( 'MEDIAWIKI_INSTALL' )
@@ -172,6 +170,10 @@ class MWExceptionRenderer {
                        if ( $hookResult ) {
                                $wgOut->addHTML( $hookResult );
                        } else {
+                               // Show any custom GUI message before the details
+                               if ( $e instanceof MessageSpecifier ) {
+                                       $wgOut->addHtml( Message::newFromSpecifier( $e )->escaped() );
+                               }
                                $wgOut->addHTML( self::getHTML( $e ) );
                        }
 
@@ -209,14 +211,14 @@ class MWExceptionRenderer {
         */
        private static function getHTML( Exception $e ) {
                if ( self::showBackTrace( $e ) ) {
-                       return '<p>' .
+                       $html = "<div class=\"errorbox\"><p>" .
                                nl2br( htmlspecialchars( MWExceptionHandler::getLogMessage( $e ) ) ) .
                                '</p><p>Backtrace:</p><p>' .
                                nl2br( htmlspecialchars( MWExceptionHandler::getRedactedTraceAsString( $e ) ) ) .
-                               "</p>\n";
+                               "</p></div>\n";
                } else {
                        $logId = WebRequest::getRequestId();
-                       return "<div class=\"errorbox\">" .
+                       $html = "<div class=\"errorbox\">" .
                                '[' . $logId . '] ' .
                                gmdate( 'Y-m-d H:i:s' ) . ": " .
                                self::msg( "internalerror-fatal-exception",
@@ -229,6 +231,8 @@ class MWExceptionRenderer {
                        "at the bottom of LocalSettings.php to show detailed " .
                        "debugging information. -->";
                }
+
+               return $html;
        }
 
        /**
index 2242c5a..38887cf 100644 (file)
@@ -26,7 +26,7 @@
  * @ingroup Database
  */
 class DBError extends Exception {
-       /** @var IDatabase */
+       /** @var IDatabase|null */
        public $db;
 
        /**
@@ -47,7 +47,22 @@ class DBError extends Exception {
  * @ingroup Database
  * @since 1.23
  */
-class DBExpectedError extends DBError {
+class DBExpectedError extends DBError implements MessageSpecifier {
+       /** @var string[] Message parameters */
+       protected $params;
+
+       function __construct( IDatabase $db = null, $error, array $params = [] ) {
+               parent::__construct( $db, $error );
+               $this->params = $params;
+       }
+
+       public function getKey() {
+               return 'databaseerror-text';
+       }
+
+       public function getParams() {
+               return $this->params;
+       }
 }
 
 /**
@@ -123,6 +138,15 @@ class DBReadOnlyError extends DBExpectedError {
 class DBTransactionError extends DBExpectedError {
 }
 
+/**
+ * @ingroup Database
+ */
+class DBTransactionSizeError extends DBTransactionError {
+       function getKey() {
+               return 'transaction-duration-limit-exceeded';
+       }
+}
+
 /**
  * Exception class for replica DB wait timeouts
  * @ingroup Database