jenkins-bot has submitted this change and it was merged.
Change subject: exception: Use MWExceptionHandler::logException in more places
......................................................................
exception: Use MWExceptionHandler::logException in more places
Most code replaced wasn't exactly like what logException does
but most probably should be.
A few implementation differences with the code it replaced in
various places:
* MWException if-guards
Was there only to prevent a crash because getLogMessage is an
MWException method. Now that logException is generic, it seems
sensible to start logging those as well (follows-up a97f3550a0).
* Exception::getTraceAsString
Now using MWExceptionHandler::formatRedactedTrace instead.
It wasn't using it because that method didn't exist yet.
Notes:
* DatabaseError::getLogMessage
Removed as this override was no longer doing anything (we're using
MWExceptionHandler::getLogMessage instead of $e->getLogMessage).
Introduced isLoggable() to take over the responsibility of indicating
when an exception should not be logged (follows-up bcb9f9e1c0d).
* DeferredUpdates and Wiki.php
Both specificy MWException. Though ApiMain intends to catch all
and only logged MWException because it couldn't otherwise, these
actually only catch MWException (as opposed to catching all and
having an if-statement inside). Left those as-is to have them
continue propagate other exceptions.
* JobQueueFederated and JobQueueGroup
All specify to catch JobQueueError only.
Not sure whether it should catch other exceptions. It now can,
but I'll leave it as is in case it intends to have those be
handled elsewhere (or fatal).
Change-Id: I4578a0fe7d95a080f1a3b292ce7ae73a4d5fcaca
---
M includes/DeferredUpdates.php
M includes/Exception.php
M includes/Wiki.php
M includes/api/ApiMain.php
M includes/db/DatabaseError.php
M includes/job/JobQueueGroup.php
6 files changed, 24 insertions(+), 19 deletions(-)
Approvals:
Krinkle: Looks good to me, but someone else must approve
Anomie: Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/DeferredUpdates.php b/includes/DeferredUpdates.php
index a321414..c385f13 100644
--- a/includes/DeferredUpdates.php
+++ b/includes/DeferredUpdates.php
@@ -109,7 +109,7 @@
// be reported to the user since the output is
already sent.
// Instead we just log them.
if ( !$e instanceof ErrorPageError ) {
- wfDebugLog( 'exception',
$e->getLogMessage() );
+ MWExceptionHandler::logException( $e );
}
}
}
diff --git a/includes/Exception.php b/includes/Exception.php
index 46402f9..582ad93 100644
--- a/includes/Exception.php
+++ b/includes/Exception.php
@@ -44,6 +44,16 @@
}
/**
+ * Whether to log this exception in the exception debug log.
+ *
+ * @since 1.23
+ * @return boolean
+ */
+ function isLoggable() {
+ return true;
+ }
+
+ /**
* Can the extension use the Message class/wfMessage to get i18n-ed
messages?
*
* @return bool
@@ -801,8 +811,8 @@
public static function logException( Exception $e ) {
global $wgLogExceptionBacktrace;
- $log = self::getLogMessage( $e );
- if ( $log ) {
+ if ( !( $e instanceof MWException ) || $e->isLoggable() ) {
+ $log = self::getLogMessage( $e );
if ( $wgLogExceptionBacktrace ) {
wfDebugLog( 'exception', $log . "\n" .
self::formatRedactedTrace( $e ) . "\n" );
} else {
diff --git a/includes/Wiki.php b/includes/Wiki.php
index 403ef11..08567ee 100644
--- a/includes/Wiki.php
+++ b/includes/Wiki.php
@@ -688,7 +688,7 @@
// We don't want exceptions thrown during job
execution to
// be reported to the user since the output is
already sent.
// Instead we just log them.
- wfDebugLog( 'exception', $e->getLogMessage() );
+ MWExceptionHandler::logException( $e );
}
}
}
diff --git a/includes/api/ApiMain.php b/includes/api/ApiMain.php
index c10d85c..c11f16c 100644
--- a/includes/api/ApiMain.php
+++ b/includes/api/ApiMain.php
@@ -383,13 +383,8 @@
wfRunHooks( 'ApiMain::onException', array( $this, $e )
);
// Log it
- if ( $e instanceof MWException && !( $e instanceof
UsageException ) ) {
- global $wgLogExceptionBacktrace;
- if ( $wgLogExceptionBacktrace ) {
- wfDebugLog( 'exception',
$e->getLogMessage() . "\n" . $e->getTraceAsString() . "\n" );
- } else {
- wfDebugLog( 'exception',
$e->getLogMessage() );
- }
+ if ( !( $e instanceof UsageException ) ) {
+ MWExceptionHandler::logException( $e );
}
// Handle any kind of exception by outputting properly
formatted error message.
diff --git a/includes/db/DatabaseError.php b/includes/db/DatabaseError.php
index 937bea0..f14a502 100644
--- a/includes/db/DatabaseError.php
+++ b/includes/db/DatabaseError.php
@@ -133,10 +133,10 @@
}
/**
- * @return bool
+ * @return boolean
*/
- function getLogMessage() {
- // Don't send to the exception log
+ function isLoggable() {
+ // Don't send to the exception log, already in dberror log
return false;
}
@@ -310,15 +310,15 @@
}
/**
- * @return bool
+ * @return boolean
*/
- function getLogMessage() {
- # Don't send to the exception log
+ function isLoggable() {
+ // Don't send to the exception log, already in dberror log
return false;
}
/**
- * @return String
+ * @return string
*/
function getPageTitle() {
return $this->msg( 'databaseerror', 'Database error' );
diff --git a/includes/job/JobQueueGroup.php b/includes/job/JobQueueGroup.php
index a20ea4a..fa7fee5 100644
--- a/includes/job/JobQueueGroup.php
+++ b/includes/job/JobQueueGroup.php
@@ -376,7 +376,7 @@
++$count;
}
} catch ( JobQueueError $e ) {
- wfDebugLog( 'exception',
$e->getLogMessage() );
+
MWExceptionHandler::logException( $e );
}
}
}
--
To view, visit https://gerrit.wikimedia.org/r/89621
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I4578a0fe7d95a080f1a3b292ce7ae73a4d5fcaca
Gerrit-PatchSet: 9
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Krinkle <[email protected]>
Gerrit-Reviewer: Aaron Schulz <[email protected]>
Gerrit-Reviewer: Anomie <[email protected]>
Gerrit-Reviewer: Krinkle <[email protected]>
Gerrit-Reviewer: Ori.livneh <[email protected]>
Gerrit-Reviewer: Parent5446 <[email protected]>
Gerrit-Reviewer: Tim Starling <[email protected]>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
