Merge "Always log exceptions in rollbackMasterChangesAndLog()"
[lhc/web/wiklou.git] / includes / exception / MWExceptionHandler.php
index bef379e..ef81366 100644 (file)
@@ -20,6 +20,8 @@
 
 use MediaWiki\Logger\LoggerFactory;
 use MediaWiki\MediaWikiServices;
+use Psr\Log\LogLevel;
+use Wikimedia\Rdbms\DBError;
 
 /**
  * Handler class for MWExceptions
@@ -81,29 +83,32 @@ 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.
+        * Roll back any open database transactions and log the stack trace of the exception
+        *
+        * This method is used to attempt to recover from exceptions
         *
         * @since 1.23
         * @param Exception|Throwable $e
         */
        public static function rollbackMasterChangesAndLog( $e ) {
                $services = MediaWikiServices::getInstance();
-               if ( $services->isServiceDisabled( 'DBLoadBalancerFactory' ) ) {
-                       return; // T147599
+               if ( !$services->isServiceDisabled( 'DBLoadBalancerFactory' ) ) {
+                       // Rollback DBs to avoid transaction notices. This might fail
+                       // to rollback some databases due to connection issues or exceptions.
+                       // However, any sane DB driver will rollback implicitly anyway.
+                       try {
+                               $services->getDBLoadBalancerFactory()->rollbackMasterChanges( __METHOD__ );
+                       } catch ( DBError $e2 ) {
+                               // If the DB is unreacheable, rollback() will throw an error
+                               // and the error report() method might need messages from the DB,
+                               // which would result in an exception loop. PHP may escalate such
+                               // errors to "Exception thrown without a stack frame" fatals, but
+                               // it's better to be explicit here.
+                               self::logException( $e2, self::CAUGHT_BY_HANDLER );
+                       }
                }
 
-               $lbFactory = $services->getDBLoadBalancerFactory();
-               if ( $lbFactory->hasMasterChanges() ) {
-                       $logger = LoggerFactory::getInstance( 'Bug56269' );
-                       $logger->warning(
-                               'Exception thrown with an uncommited database transaction: ' .
-                               self::getLogMessage( $e ),
-                               self::getLogContext( $e )
-                       );
-               }
-               $lbFactory->rollbackMasterChanges( __METHOD__ );
+               self::logException( $e, self::CAUGHT_BY_HANDLER );
        }
 
        /**
@@ -121,23 +126,8 @@ class MWExceptionHandler {
         * @param Exception|Throwable $e
         */
        public static function handleException( $e ) {
-               try {
-                       // Rollback DBs to avoid transaction notices. This may fail
-                       // to rollback some DB due to connection issues or exceptions.
-                       // However, any sane DB driver will rollback implicitly anyway.
-                       self::rollbackMasterChangesAndLog( $e );
-               } catch ( DBError $e2 ) {
-                       // If the DB is unreacheable, rollback() will throw an error
-                       // and the error report() method might need messages from the DB,
-                       // which would result in an exception loop. PHP may escalate such
-                       // errors to "Exception thrown without a stack frame" fatals, but
-                       // it's better to be explicit here.
-                       self::logException( $e2, self::CAUGHT_BY_HANDLER );
-               }
-
-               self::logException( $e, self::CAUGHT_BY_HANDLER );
+               self::rollbackMasterChangesAndLog( $e );
                self::report( $e );
-
                // Exit value should be nonzero for the benefit of shell jobs
                exit( 1 );
        }
@@ -174,31 +164,37 @@ class MWExceptionHandler {
                switch ( $level ) {
                        case E_RECOVERABLE_ERROR:
                                $levelName = 'Error';
+                               $severity = LogLevel::ERROR;
                                break;
                        case E_WARNING:
                        case E_CORE_WARNING:
                        case E_COMPILE_WARNING:
                        case E_USER_WARNING:
                                $levelName = 'Warning';
+                               $severity = LogLevel::WARNING;
                                break;
                        case E_NOTICE:
                        case E_USER_NOTICE:
                                $levelName = 'Notice';
+                               $severity = LogLevel::INFO;
                                break;
                        case E_STRICT:
                                $levelName = 'Strict Standards';
+                               $severity = LogLevel::DEBUG;
                                break;
                        case E_DEPRECATED:
                        case E_USER_DEPRECATED:
                                $levelName = 'Deprecated';
+                               $severity = LogLevel::INFO;
                                break;
                        default:
                                $levelName = 'Unknown error';
+                               $severity = LogLevel::ERROR;
                                break;
                }
 
                $e = new ErrorException( "PHP $levelName: $message", 0, $level, $file, $line );
-               self::logError( $e, 'error' );
+               self::logError( $e, 'error', $severity );
 
                // This handler is for logging only. Return false will instruct PHP
                // to continue regular handling.
@@ -335,7 +331,7 @@ TXT;
                                $text .= "{$pad}#{$level} {$frame['file']}({$frame['line']}): ";
                        } else {
                                // 'file' and 'line' are unset for calls via call_user_func
-                               // (bug 55634) This matches behaviour of
+                               // (T57634) This matches behaviour of
                                // Exception::getTraceAsString to instead display "[internal
                                // function]".
                                $text .= "{$pad}#{$level} [internal function]: ";
@@ -621,8 +617,11 @@ TXT;
         * @since 1.25
         * @param ErrorException $e
         * @param string $channel
+        * @param string $level
        */
-       protected static function logError( ErrorException $e, $channel ) {
+       protected static function logError(
+               ErrorException $e, $channel, $level = LogLevel::ERROR
+       ) {
                $catcher = self::CAUGHT_BY_HANDLER;
                // The set_error_handler callback is independent from error_reporting.
                // Filter out unwanted errors manually (e.g. when
@@ -630,7 +629,8 @@ TXT;
                $suppressed = ( error_reporting() & $e->getSeverity() ) === 0;
                if ( !$suppressed ) {
                        $logger = LoggerFactory::getInstance( $channel );
-                       $logger->error(
+                       $logger->log(
+                               $level,
                                self::getLogMessage( $e ),
                                self::getLogContext( $e, $catcher )
                        );
@@ -640,7 +640,7 @@ TXT;
                $json = self::jsonSerializeException( $e, false, FormatJson::ALL_OK, $catcher );
                if ( $json !== false ) {
                        $logger = LoggerFactory::getInstance( "{$channel}-json" );
-                       $logger->error( $json, [ 'private' => true ] );
+                       $logger->log( $level, $json, [ 'private' => true ] );
                }
 
                Hooks::run( 'LogException', [ $e, $suppressed ] );