From 399ba2fecfe098b45b2064380eadcac0b24b9a4a Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Sun, 16 Nov 2014 12:49:25 +0100 Subject: [PATCH] MWException: Log stack traces for php errors (not exceptions) * Remove use of 'error' where it's redundant. * Remove call to logException from responsibility of MWException. Call from exception handler instead. Change-Id: I8764cf5df87b226813c9b9cf99f9b4f3fa4b7c92 --- includes/MediaWiki.php | 2 +- includes/exception/MWException.php | 2 - includes/exception/MWExceptionHandler.php | 62 ++++++++++++++++++----- tests/phpunit/phpunit.php | 6 +++ 4 files changed, 57 insertions(+), 15 deletions(-) diff --git a/includes/MediaWiki.php b/includes/MediaWiki.php index 9585c5f589..7ce6d1b48f 100644 --- a/includes/MediaWiki.php +++ b/includes/MediaWiki.php @@ -446,7 +446,7 @@ class MediaWiki { $this->triggerJobs(); $this->restInPeace(); } catch ( Exception $e ) { - MWExceptionHandler::handle( $e ); + MWExceptionHandler::handleException( $e ); } } diff --git a/includes/exception/MWException.php b/includes/exception/MWException.php index 074128f889..6fd6fb5f25 100644 --- a/includes/exception/MWException.php +++ b/includes/exception/MWException.php @@ -222,8 +222,6 @@ class MWException extends Exception { public function report() { global $wgMimeType; - MWExceptionHandler::logException( $this ); - if ( defined( 'MW_API' ) ) { // Unhandled API exception, we can't be sure that format printer is alive self::header( 'MediaWiki-API-Error: internal_api_error_' . get_class( $this ) ); diff --git a/includes/exception/MWExceptionHandler.php b/includes/exception/MWExceptionHandler.php index 1f1ba9c9ef..0d90e66949 100644 --- a/includes/exception/MWExceptionHandler.php +++ b/includes/exception/MWExceptionHandler.php @@ -23,11 +23,13 @@ * @ingroup Exception */ class MWExceptionHandler { + /** - * Install an exception handler for MediaWiki exception types. + * Install handlers with PHP. */ public static function installHandler() { - set_exception_handler( array( 'MWExceptionHandler', 'handle' ) ); + set_exception_handler( array( 'MWExceptionHandler', 'handleException' ) ); + set_error_handler( array( 'MWExceptionHandler', 'handleError' ) ); } /** @@ -45,7 +47,7 @@ class MWExceptionHandler { $e->report(); } catch ( Exception $e2 ) { // Exception occurred from within exception handler - // Show a simpler error message for the original exception, + // Show a simpler message for the original exception, // don't try to invoke report() $message = "MediaWiki internal error.\n\n"; @@ -83,7 +85,6 @@ class MWExceptionHandler { echo nl2br( htmlspecialchars( $message ) ) . "\n"; } - self::logException( $e ); } } @@ -108,6 +109,7 @@ class MWExceptionHandler { * If there are any open database transactions, roll them back and log * the stack trace of the exception that should have been caught so the * transaction could be aborted properly. + * * @since 1.23 * @param Exception $e */ @@ -133,13 +135,15 @@ class MWExceptionHandler { * } catch ( Exception $e ) { * echo $e->__toString(); * } + * + * @since 1.25 * @param Exception $e */ - public static function handle( $e ) { + public static function handleException( $e ) { global $wgFullyInitialised; self::rollbackMasterChangesAndLog( $e ); - + self::logException( $e ); self::report( $e ); // Final cleanup @@ -155,6 +159,22 @@ class MWExceptionHandler { exit( 1 ); } + /** + * @since 1.25 + * @param int $level Error level raised + * @param string $message + * @param string $file + * @param int $line + */ + public static function handleError( $level, $message, $file = null, $line = null ) { + $e = new ErrorException( $message, 0, $level, $file, $line ); + self::logError( $e ); + + // This handler is for logging only. Return false will instruct PHP + // to continue regular handling. + return false; + } + /** * Generate a string representation of an exception's stack trace * @@ -219,7 +239,7 @@ class MWExceptionHandler { } /** - * Get the ID for this error. + * Get the ID for this exception. * * 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. @@ -251,8 +271,7 @@ class MWExceptionHandler { } /** - * Return the requested URL and point to file and line number from which the - * exception occurred. + * Get a message formatting the exception message and its origin. * * @since 1.22 * @param Exception $e @@ -260,12 +279,13 @@ class MWExceptionHandler { */ public static function getLogMessage( Exception $e ) { $id = self::getLogId( $e ); + $type = get_class( $e ); $file = $e->getFile(); $line = $e->getLine(); $message = $e->getMessage(); $url = self::getURL() ?: '[no req]'; - return "[$id] $url Exception from line $line of $file: $message"; + return "[$id] $url $type from line $line of $file: $message"; } /** @@ -287,6 +307,7 @@ class MWExceptionHandler { * @code * { * "id": "c41fb419", + * "type": "MWException", * "file": "/var/www/mediawiki/includes/cache/MessageCache.php", * "line": 704, * "message": "Non-string key given", @@ -298,6 +319,7 @@ class MWExceptionHandler { * @code * { * "id": "dc457938", + * "type": "MWException", * "file": "/vagrant/mediawiki/includes/cache/MessageCache.php", * "line": 704, * "message": "Non-string key given", @@ -324,6 +346,7 @@ class MWExceptionHandler { $exceptionData = array( 'id' => self::getLogId( $e ), + 'type' => get_class( $e ), 'file' => $e->getFile(), 'line' => $e->getLine(), 'message' => $e->getMessage(), @@ -347,7 +370,7 @@ class MWExceptionHandler { * 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. + * it is also used to handle PHP exceptions or exceptions from other libraries. * * @since 1.22 * @param Exception $e @@ -368,7 +391,22 @@ class MWExceptionHandler { wfDebugLog( 'exception-json', $json, 'private' ); } } - } + /** + * Log an exception that wasn't thrown but made to wrap an error. + * + * @since 1.25 + * @param Exception $e + */ + protected static function logError( Exception $e ) { + global $wgLogExceptionBacktrace; + + $log = self::getLogMessage( $e ); + if ( $wgLogExceptionBacktrace ) { + wfDebugLog( 'error', $log . "\n" . $e->getTraceAsString() ); + } else { + wfDebugLog( 'error', $log ); + } + } } diff --git a/tests/phpunit/phpunit.php b/tests/phpunit/phpunit.php index 1125504353..e59b506370 100755 --- a/tests/phpunit/phpunit.php +++ b/tests/phpunit/phpunit.php @@ -93,6 +93,12 @@ class PHPUnitMaintClass extends Maintenance { public function execute() { global $IP; + // Deregister handler from MWExceptionHandler::installHandle so that PHPUnit's own handler + // stays in tact. + // Has to in execute() instead of finalSetup(), because finalSetup() runs before + // doMaintenance.php includes Setup.php, which calls MWExceptionHandler::installHandle(). + restore_error_handler(); + $this->forceFormatServerArgv(); # Make sure we have --configuration or PHPUnit might complain -- 2.20.1