From: Aaron Schulz <aschulz@wikimedia.org>
Date: Wed, 14 Sep 2016 09:28:18 +0000 (-0700)
Subject: Exception cleanups for LoadBalancer
X-Git-Tag: 1.31.0-rc.0~5567^2
X-Git-Url: https://git.cyclocoop.org/%28%28?a=commitdiff_plain;h=5820c6dcacb7c144b5ee8c292d12289ac1875df6;p=lhc%2Fweb%2Fwiklou.git

Exception cleanups for LoadBalancer

* 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
---

diff --git a/autoload.php b/autoload.php
index 66736b3d42..b9fd6bd9b1 100644
--- a/autoload.php
+++ b/autoload.php
@@ -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',
diff --git a/includes/db/loadbalancer/LoadBalancer.php b/includes/db/loadbalancer/LoadBalancer.php
index 841231b33c..b5c6f98f05 100644
--- a/includes/db/loadbalancer/LoadBalancer.php
+++ b/includes/db/loadbalancer/LoadBalancer.php
@@ -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
diff --git a/includes/exception/MWExceptionRenderer.php b/includes/exception/MWExceptionRenderer.php
index f455191c48..58b4ac705d 100644
--- a/includes/exception/MWExceptionRenderer.php
+++ b/includes/exception/MWExceptionRenderer.php
@@ -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;
 	}
 
 	/**
diff --git a/includes/libs/rdbms/exception/DBError.php b/includes/libs/rdbms/exception/DBError.php
index 2242c5af53..38887cfd71 100644
--- a/includes/libs/rdbms/exception/DBError.php
+++ b/includes/libs/rdbms/exception/DBError.php
@@ -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