From f0386d31835b8f65fb7fc1f275d1761cae339e97 Mon Sep 17 00:00:00 2001
From: Timo Tijhof <krinklemail@gmail.com>
Date: Wed, 9 Oct 2013 02:20:41 +0200
Subject: [PATCH] exception: Move logging logic to static method of
 MWExceptionHandler

Follows-up c28251a9fd, which unconditionally called logException
on objects that aren't always instances of MWException.

This makes it possible to, for example, log PHP errors or errors
from Less in ResourceLoader to be logged as well (which naturally
don't have a logException method).

As a result of the handling now being generic, non-MWException
errors will now be loggable, and also have their ID fixed between
HTML output and the debug log entry.

* Not yet removing MWException::getLogMessage (introduced in bcb9f9e1c)
  as it has been around since 2006 (released in 1.8.0).

* Not yet removing MWException::getLogId (introduced in 70841c5867)
  as it was part of the 1.20.0 release.

* Removed MWException::logException (introduced in c28251a9fd)
  as it was only added a few weeks ago within this release cycle.

Change-Id: Iab98e3a7a9b78d8602e69e0571b35cf107a96b72
---
 includes/Exception.php                     | 123 ++++++++++++++-------
 includes/resourceloader/ResourceLoader.php |  16 +--
 2 files changed, 88 insertions(+), 51 deletions(-)

diff --git a/includes/Exception.php b/includes/Exception.php
index b97697dedf..fba857f90d 100644
--- a/includes/Exception.php
+++ b/includes/Exception.php
@@ -30,7 +30,6 @@
  * @ingroup Exception
  */
 class MWException extends Exception {
-	var $logId;
 
 	/**
 	 * Should the exception use $wgOut to output the error?
@@ -131,7 +130,7 @@ class MWException extends Exception {
 				"</p>\n";
 		} else {
 			return "<div class=\"errorbox\">" .
-				'[' . $this->getLogId() . '] ' .
+				'[' . MWExceptionHandler::getLogId( $this ) . '] ' .
 				gmdate( 'Y-m-d H:i:s' ) .
 				": Fatal exception of type " . get_class( $this ) . "</div>\n" .
 				"<!-- Set \$wgShowExceptionDetails = true; " .
@@ -169,43 +168,28 @@ class MWException extends Exception {
 	}
 
 	/**
-	 * Get a random ID for this error.
-	 * This allows to link the exception to its corresponding log entry when
-	 * $wgShowExceptionDetails is set to false.
+	 * Get a the ID for this error.
 	 *
+	 * @since 1.20
+	 * @deprecated since 1.22 Use MWExceptionHandler::getLogId instead.
 	 * @return string
 	 */
 	function getLogId() {
-		if ( $this->logId === null ) {
-			$this->logId = wfRandomString( 8 );
-		}
-		return $this->logId;
+		wfDeprecated( __METHOD__, '1.22' );
+		return MWExceptionHandler::getLogId( $this );
 	}
 
 	/**
 	 * Return the requested URL and point to file and line number from which the
 	 * exception occurred
 	 *
+	 * @since 1.8
+	 * @deprecated since 1.22 Use MWExceptionHandler::getLogMessage instead.
 	 * @return string
 	 */
 	function getLogMessage() {
-		global $wgRequest;
-
-		$id = $this->getLogId();
-		$file = $this->getFile();
-		$line = $this->getLine();
-		$message = $this->getMessage();
-
-		if ( isset( $wgRequest ) && !$wgRequest instanceof FauxRequest ) {
-			$url = $wgRequest->getRequestURL();
-			if ( !$url ) {
-				$url = '[no URL]';
-			}
-		} else {
-			$url = '[no req]';
-		}
-
-		return "[$id] $url   Exception from line $line of $file: $message";
+		wfDeprecated( __METHOD__, '1.22' );
+		return MWExceptionHandler::getLogMessage( $this );
 	}
 
 	/**
@@ -249,7 +233,7 @@ class MWException extends Exception {
 	function report() {
 		global $wgMimeType;
 
-		$this->logException();
+		MWExceptionHandler::logException( $this );
 
 		if ( defined( 'MW_API' ) ) {
 			// Unhandled API exception, we can't be sure that format printer is alive
@@ -266,22 +250,6 @@ class MWException extends Exception {
 		}
 	}
 
-	/**
-	 * Log the error message to the exception log (if enabled)
-	 */
-	function logException() {
-		global $wgLogExceptionBacktrace;
-
-		$log = $this->getLogMessage();
-		if ( $log ) {
-			if ( $wgLogExceptionBacktrace ) {
-				wfDebugLog( 'exception', $log . "\n" . MWExceptionHandler::formatRedactedTrace( $this ) . "\n" );
-			} else {
-				wfDebugLog( 'exception', $log );
-			}
-		}
-	}
-
 	/**
 	 * Check whether we are in command line mode or not to report the exception
 	 * in the correct format.
@@ -767,4 +735,73 @@ class MWExceptionHandler {
 		}
 		return $finalExceptionText . '#' . ( $i + 1 ) . ' {main}';
 	}
+
+
+	/**
+	 * Get the ID for this error.
+	 *
+	 * The ID is saved so that one can match the one output to the user (when
+	 * $wgShowExceptionDetails is set to false), to the entry in the debug log.
+	 *
+	 * @since 1.22
+	 * @param Exception $e
+	 * @return string
+	 */
+	public static function getLogId( Exception $e ) {
+		if ( !isset( $e->_mwLogId ) ) {
+			$e->_mwLogId = wfRandomString( 8 );
+		}
+		return $e->_mwLogId;
+	}
+
+	/**
+	 * Return the requested URL and point to file and line number from which the
+	 * exception occurred.
+	 *
+	 * @since 1.22
+	 * @param Exception $e
+	 * @return string
+	 */
+	public static function getLogMessage( Exception $e ) {
+		global $wgRequest;
+
+		$id = self::getLogId( $e );
+		$file = $e->getFile();
+		$line = $e->getLine();
+		$message = $e->getMessage();
+
+		if ( isset( $wgRequest ) && !$wgRequest instanceof FauxRequest ) {
+			$url = $wgRequest->getRequestURL();
+			if ( !$url ) {
+				$url = '[no URL]';
+			}
+		} else {
+			$url = '[no req]';
+		}
+
+		return "[$id] $url   Exception from line $line of $file: $message";
+	}
+
+	/**
+	 * Log an exception to the exception log (if enabled).
+	 *
+	 * This method must not assume the exception is an MWException,
+	 * it is also used to handle PHP errors or errors from other libraries.
+	 *
+	 * @since 1.22
+	 * @param Exception $e
+	 */
+	public static function logException( Exception $e ) {
+		global $wgLogExceptionBacktrace;
+
+		$log = self::getLogMessage( $e );
+		if ( $log ) {
+			if ( $wgLogExceptionBacktrace ) {
+				wfDebugLog( 'exception', $log . "\n" . self::formatRedactedTrace( $e ) . "\n" );
+			} else {
+				wfDebugLog( 'exception', $log );
+			}
+		}
+	}
+
 }
diff --git a/includes/resourceloader/ResourceLoader.php b/includes/resourceloader/ResourceLoader.php
index 81390dc395..19d019d329 100644
--- a/includes/resourceloader/ResourceLoader.php
+++ b/includes/resourceloader/ResourceLoader.php
@@ -178,12 +178,12 @@ class ResourceLoader {
 
 			// Save filtered text to Memcached
 			$cache->set( $key, $result );
-		} catch ( Exception $exception ) {
-			$exception->logException();
-			wfDebugLog( 'resourceloader', __METHOD__ . ": minification failed: $exception" );
+		} catch ( Exception $e ) {
+			MWExceptionHandler::logException( $e );
+			wfDebugLog( 'resourceloader', __METHOD__ . ": minification failed: $e" );
 			$this->hasErrors = true;
 			// Return exception as a comment
-			$result = self::formatException( $exception );
+			$result = self::formatException( $e );
 		}
 
 		wfProfileOut( __METHOD__ );
@@ -477,7 +477,7 @@ class ResourceLoader {
 		try {
 			$this->preloadModuleInfo( array_keys( $modules ), $context );
 		} catch ( Exception $e ) {
-			$e->logException();
+			MWExceptionHandler::logException( $e );
 			wfDebugLog( 'resourceloader', __METHOD__ . ": preloading module info failed: $e" );
 			$this->hasErrors = true;
 			// Add exception to the output as a comment
@@ -497,7 +497,7 @@ class ResourceLoader {
 				// Calculate maximum modified time
 				$mtime = max( $mtime, $module->getModifiedTime( $context ) );
 			} catch ( Exception $e ) {
-				$e->logException();
+				MWExceptionHandler::logException( $e );
 				wfDebugLog( 'resourceloader', __METHOD__ . ": calculating maximum modified time failed: $e" );
 				$this->hasErrors = true;
 				// Add exception to the output as a comment
@@ -724,7 +724,7 @@ class ResourceLoader {
 			try {
 				$blobs = MessageBlobStore::get( $this, $modules, $context->getLanguage() );
 			} catch ( Exception $e ) {
-				$e->logException();
+				MWExceptionHandler::logException( $e );
 				wfDebugLog( 'resourceloader', __METHOD__ . ": pre-fetching blobs from MessageBlobStore failed: $e" );
 				$this->hasErrors = true;
 				// Add exception to the output as a comment
@@ -832,7 +832,7 @@ class ResourceLoader {
 						break;
 				}
 			} catch ( Exception $e ) {
-				$e->logException();
+				MWExceptionHandler::logException( $e );
 				wfDebugLog( 'resourceloader', __METHOD__ . ": generating module package failed: $e" );
 				$this->hasErrors = true;
 				// Add exception to the output as a comment
-- 
2.20.1