Krinkle has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/89621


Change subject: [WIP] exception: Use new MWExceptionHandler::logException in 
more places
......................................................................

[WIP] exception: Use new 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
  Rethink the undocumented interface of getLogMessage returning
  false to stop logging.
  FIXME

* 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/Wiki.php
M includes/api/ApiMain.php
M includes/db/DatabaseError.php
M includes/job/JobQueueFederated.php
M includes/job/JobQueueGroup.php
6 files changed, 13 insertions(+), 17 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core 
refs/changes/21/89621/1

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/Wiki.php b/includes/Wiki.php
index edfbba2..ae75bf3 100644
--- a/includes/Wiki.php
+++ b/includes/Wiki.php
@@ -687,7 +687,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 da391d7..9ab83c1 100644
--- a/includes/db/DatabaseError.php
+++ b/includes/db/DatabaseError.php
@@ -138,6 +138,7 @@
         */
        function getLogMessage() {
                # Don't send to the exception log
+               # FIXME: Returning false here doesn't change whether it is 
logged or not.
                return false;
        }
 
diff --git a/includes/job/JobQueueFederated.php 
b/includes/job/JobQueueFederated.php
index d788c98..e915765 100644
--- a/includes/job/JobQueueFederated.php
+++ b/includes/job/JobQueueFederated.php
@@ -144,7 +144,7 @@
                                        return false;
                                }
                        } catch ( JobQueueError $e ) {
-                               wfDebugLog( 'exception', $e->getLogMessage() );
+                               MWExceptionHandler::logException( $e );
                        }
                }
 
@@ -186,7 +186,7 @@
                        try {
                                $count += $queue->$method();
                        } catch ( JobQueueError $e ) {
-                               wfDebugLog( 'exception', $e->getLogMessage() );
+                               MWExceptionHandler::logException( $e );
                        }
                }
 
@@ -256,7 +256,7 @@
                                $ok = $queue->doBatchPush( $jobBatch, $flags );
                        } catch ( JobQueueError $e ) {
                                $ok = false;
-                               wfDebugLog( 'exception', $e->getLogMessage() );
+                               MWExceptionHandler::logException( $e );
                        }
                        if ( $ok ) {
                                $key = $this->getCacheKey( 'empty' );
@@ -277,7 +277,7 @@
                                        $ok = $queue->doBatchPush( $jobBatch, 
$flags );
                                } catch ( JobQueueError $e ) {
                                        $ok = false;
-                                       wfDebugLog( 'exception', 
$e->getLogMessage() );
+                                       MWExceptionHandler::logException( $e );
                                }
                                if ( $ok ) {
                                        $key = $this->getCacheKey( 'empty' );
@@ -312,7 +312,7 @@
                                $job = $queue->pop();
                        } catch ( JobQueueError $e ) {
                                $job = false;
-                               wfDebugLog( 'exception', $e->getLogMessage() );
+                               MWExceptionHandler::logException( $e );
                        }
                        if ( $job ) {
                                $job->metadata['QueuePartition'] = $partition;
@@ -364,7 +364,7 @@
                        try {
                                $queue->doDelete();
                        } catch ( JobQueueError $e ) {
-                               wfDebugLog( 'exception', $e->getLogMessage() );
+                               MWExceptionHandler::logException( $e );
                        }
                }
        }
@@ -374,7 +374,7 @@
                        try {
                                $queue->waitForBackups();
                        } catch ( JobQueueError $e ) {
-                               wfDebugLog( 'exception', $e->getLogMessage() );
+                               MWExceptionHandler::logException( $e );
                        }
                }
        }
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: newchange
Gerrit-Change-Id: I4578a0fe7d95a080f1a3b292ce7ae73a4d5fcaca
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Krinkle <[email protected]>

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to