RCFilters: Display specific error if query times out
authorMatthew Flaschen <mflaschen@wikimedia.org>
Mon, 18 Sep 2017 11:42:07 +0000 (07:42 -0400)
committerMatthew Flaschen <mflaschen@wikimedia.org>
Mon, 23 Oct 2017 11:15:23 +0000 (16:45 +0530)
This catches DBQueryTimeoutError, logs it, returns HTTP 500,
and displays an error message.

Live update just ignores it (effectively treating it the same as
a live update check that happens to have no updates).

Bug: T175776
Change-Id: If4d880e9e6a56989895956798fc6918a43841065

includes/specialpage/ChangesListSpecialPage.php
languages/i18n/en.json
languages/i18n/qqq.json
resources/Resources.php
resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.ChangesListViewModel.js
resources/src/mediawiki.rcfilters/mw.rcfilters.Controller.js
resources/src/mediawiki.rcfilters/mw.rcfilters.init.js
resources/src/mediawiki.rcfilters/styles/mw.rcfilters.less
resources/src/mediawiki.rcfilters/ui/mw.rcfilters.ui.ChangesListWrapperWidget.js
resources/src/mediawiki.rcfilters/ui/mw.rcfilters.ui.FormWrapperWidget.js

index df6a1c1..a3fee64 100644 (file)
@@ -21,6 +21,7 @@
  * @ingroup SpecialPage
  */
 use MediaWiki\Logger\LoggerFactory;
+use Wikimedia\Rdbms\DBQueryTimeoutError;
 use Wikimedia\Rdbms\ResultWrapper;
 use Wikimedia\Rdbms\FakeResultWrapper;
 use Wikimedia\Rdbms\IDatabase;
@@ -542,45 +543,57 @@ abstract class ChangesListSpecialPage extends SpecialPage {
 
                $this->considerActionsForDefaultSavedQuery();
 
-               $rows = $this->getRows();
                $opts = $this->getOptions();
-               if ( $rows === false ) {
-                       $rows = new FakeResultWrapper( [] );
-               }
+               try {
+                       $rows = $this->getRows();
+                       if ( $rows === false ) {
+                               $rows = new FakeResultWrapper( [] );
+                       }
 
-               // Used by Structured UI app to get results without MW chrome
-               if ( $this->getRequest()->getVal( 'action' ) === 'render' ) {
-                       $this->getOutput()->setArticleBodyOnly( true );
-               }
+                       // Used by Structured UI app to get results without MW chrome
+                       if ( $this->getRequest()->getVal( 'action' ) === 'render' ) {
+                               $this->getOutput()->setArticleBodyOnly( true );
+                       }
 
-               // Used by "live update" and "view newest" to check
-               // if there's new changes with minimal data transfer
-               if ( $this->getRequest()->getBool( 'peek' ) ) {
+                       // Used by "live update" and "view newest" to check
+                       // if there's new changes with minimal data transfer
+                       if ( $this->getRequest()->getBool( 'peek' ) ) {
                        $code = $rows->numRows() > 0 ? 200 : 204;
-                       $this->getOutput()->setStatusCode( $code );
-                       return;
-               }
+                               $this->getOutput()->setStatusCode( $code );
+                               return;
+                       }
 
-               $batch = new LinkBatch;
-               foreach ( $rows as $row ) {
-                       $batch->add( NS_USER, $row->rc_user_text );
-                       $batch->add( NS_USER_TALK, $row->rc_user_text );
-                       $batch->add( $row->rc_namespace, $row->rc_title );
-                       if ( $row->rc_source === RecentChange::SRC_LOG ) {
-                               $formatter = LogFormatter::newFromRow( $row );
-                               foreach ( $formatter->getPreloadTitles() as $title ) {
-                                       $batch->addObj( $title );
+                       $batch = new LinkBatch;
+                       foreach ( $rows as $row ) {
+                               $batch->add( NS_USER, $row->rc_user_text );
+                               $batch->add( NS_USER_TALK, $row->rc_user_text );
+                               $batch->add( $row->rc_namespace, $row->rc_title );
+                               if ( $row->rc_source === RecentChange::SRC_LOG ) {
+                                       $formatter = LogFormatter::newFromRow( $row );
+                                       foreach ( $formatter->getPreloadTitles() as $title ) {
+                                               $batch->addObj( $title );
+                                       }
                                }
                        }
-               }
-               $batch->execute();
+                       $batch->execute();
+
+                       $this->setHeaders();
+                       $this->outputHeader();
+                       $this->addModules();
+                       $this->webOutput( $rows, $opts );
 
-               $this->setHeaders();
-               $this->outputHeader();
-               $this->addModules();
-               $this->webOutput( $rows, $opts );
+                       $rows->free();
+               } catch ( DBQueryTimeoutError $timeoutException ) {
+                       MWExceptionHandler::logException( $timeoutException );
 
-               $rows->free();
+                       $this->setHeaders();
+                       $this->outputHeader();
+                       $this->addModules();
+
+                       $this->getOutput()->setStatusCode( 500 );
+                       $this->webOutputHeader( 0, $opts );
+                       $this->outputTimeout();
+               }
 
                if ( $this->getConfig()->get( 'EnableWANCacheReaper' ) ) {
                        // Clean up any bad page entries for titles showing up in RC
@@ -791,6 +804,17 @@ abstract class ChangesListSpecialPage extends SpecialPage {
                );
        }
 
+       /**
+        * Add the "timeout" message to the output
+        */
+       protected function outputTimeout() {
+               $this->getOutput()->addHTML(
+                       '<div class="mw-changeslist-timeout">' .
+                       $this->msg( 'recentchanges-timeout' )->parse() .
+                       '</div>'
+               );
+       }
+
        /**
         * Get the database result for this special page instance. Used by ApiFeedRecentChanges.
         *
@@ -1409,16 +1433,26 @@ abstract class ChangesListSpecialPage extends SpecialPage {
        }
 
        /**
-        * Send output to the OutputPage object, only called if not used feeds
+        * Send header output to the OutputPage object, only called if not using feeds
         *
-        * @param ResultWrapper $rows Database rows
+        * @param int $rowCount Number of database rows
         * @param FormOptions $opts
         */
-       public function webOutput( $rows, $opts ) {
+       private function webOutputHeader( $rowCount, $opts ) {
                if ( !$this->including() ) {
                        $this->outputFeedLinks();
-                       $this->doHeader( $opts, $rows->numRows() );
+                       $this->doHeader( $opts, $rowCount );
                }
+       }
+
+       /**
+        * Send output to the OutputPage object, only called if not used feeds
+        *
+        * @param ResultWrapper $rows Database rows
+        * @param FormOptions $opts
+        */
+       public function webOutput( $rows, $opts ) {
+               $this->webOutputHeader( $rows->numRows(), $opts );
 
                $this->outputChangesList( $rows, $opts );
        }
index 282b906..c20452b 100644 (file)
        "recentchanges-summary": "Track the most recent changes to the wiki on this page.",
        "recentchangestext": "-",
        "recentchanges-noresult": "No changes during the given period match these criteria.",
+       "recentchanges-timeout": "This search has timed out. You may wish to try different search parameters.",
        "recentchanges-feed-description": "Track the most recent changes to the wiki in this feed.",
        "recentchanges-label-newpage": "This edit created a new page",
        "recentchanges-label-minor": "This is a minor edit",
index 4d6cf93..8a645bc 100644 (file)
        "recentchanges-summary": "Summary of [[Special:RecentChanges]].",
        "recentchangestext": "Text in [[Special:RecentChanges]]",
        "recentchanges-noresult": "Used in [[Special:RecentChanges]], [[Special:RecentChangesLinked]], and [[Special:Watchlist]] when there are no changes to be shown.",
+       "recentchanges-timeout": "Used in [[Special:RecentChanges]], [[Special:RecentChangesLinked]], and [[Special:Watchlist]] when a query times out.",
        "recentchanges-feed-description": "Used in feed of RecentChanges. See example [{{canonicalurl:Special:RecentChanges|feed=atom}} feed].",
        "recentchanges-label-newpage": "# Used as tooltip for {{msg-mw|Newpageletter}}.\n# Also used as legend. Preceded by {{msg-mw|Newpageletter}} and followed by {{msg-mw|Recentchanges-legend-newpage}}.",
        "recentchanges-label-minor": "# Used as tooltip for {{msg-mw|Minoreditletter}}\n# Also used as legend. Preceded by {{msg-mw|Minoreditletter}}",
index 6d59d5c..055ffba 100644 (file)
@@ -1915,6 +1915,7 @@ return [
                        'namespaces',
                        'invert',
                        'recentchanges-noresult',
+                       'recentchanges-timeout',
                        'quotation-marks',
                ],
                'dependencies' => [
index debe0b9..3c03c70 100644 (file)
@@ -33,6 +33,7 @@
         * @event update
         * @param {jQuery|string} $changesListContent List of changes
         * @param {jQuery} $fieldset Server-generated form
+        * @param {boolean} isDatabaseTimeout Whether this is an error state due to a database query
         * @param {boolean} isInitialDOM Whether the previous dom variables are from the initial page load
         * @param {boolean} fromLiveUpdate These are new changes fetched via Live Update
         *
         *
         * @param {jQuery|string} changesListContent
         * @param {jQuery} $fieldset
+        * @param {boolean} isDatabaseTimeout Whether this is an error state due to a database query
+        *   timeout.
         * @param {boolean} [isInitialDOM] Using the initial (already attached) DOM elements
         * @param {boolean} [separateOldAndNew] Whether a logical separation between old and new changes is needed
         * @fires update
         */
-       mw.rcfilters.dm.ChangesListViewModel.prototype.update = function ( changesListContent, $fieldset, isInitialDOM, separateOldAndNew ) {
+       mw.rcfilters.dm.ChangesListViewModel.prototype.update = function ( changesListContent, $fieldset, isDatabaseTimeout, isInitialDOM, separateOldAndNew ) {
                var from = this.nextFrom;
                this.valid = true;
                this.extractNextFrom( $fieldset );
                this.checkForUnseenWatchedChanges( changesListContent );
-               this.emit( 'update', changesListContent, $fieldset, isInitialDOM, separateOldAndNew ? from : null );
+               this.emit( 'update', changesListContent, $fieldset, isDatabaseTimeout, isInitialDOM, separateOldAndNew ? from : null );
        };
 
        /**
index 3e71729..b805b24 100644 (file)
         * @param {Object} [tagList] Tag definition
         */
        mw.rcfilters.Controller.prototype.initialize = function ( filterStructure, namespaceStructure, tagList ) {
-               var parsedSavedQueries,
+               var parsedSavedQueries, pieces,
                        displayConfig = mw.config.get( 'StructuredChangeFiltersDisplayConfig' ),
                        defaultSavedQueryExists = mw.config.get( 'wgStructuredChangeFiltersDefaultSavedQueryExists' ),
                        controller = this,
                        views = {},
                        items = [],
-                       uri = new mw.Uri(),
-                       $changesList = $( '.mw-changeslist' ).first().contents();
+                       uri = new mw.Uri();
 
                // Prepare views
                if ( namespaceStructure ) {
                        // again
                        this.updateStateFromUrl( false );
 
+                       pieces = this._extractChangesListInfo( $( '#mw-content-text' ) );
+
                        // Update the changes list with the existing data
                        // so it gets processed
                        this.changesListModel.update(
-                               $changesList.length ? $changesList : 'NO_RESULTS',
-                               $( 'fieldset.cloptions' ).first(),
+                               pieces.changes,
+                               pieces.fieldset,
+                               pieces.noResultsDetails === 'NO_RESULTS_TIMEOUT',
                                true // We're using existing DOM elements
                        );
                }
                }
        };
 
+       /**
+        * Extracts information from the changes list DOM
+        *
+        * @param {jQuery} $root Root DOM to find children from
+        * @return {Object} Information about changes list
+        * @return {Object|string} return.changes Changes list, or 'NO_RESULTS' if there are no results
+        *   (either normally or as an error)
+        * @return {string} [return.noResultsDetails] 'NO_RESULTS_NORMAL' for a normal 0-result set,
+        *   'NO_RESULTS_TIMEOUT' for no results due to a timeout, or omitted for more than 0 results
+        * @return {jQuery} return.fieldset Fieldset
+        */
+       mw.rcfilters.Controller.prototype._extractChangesListInfo = function ( $root ) {
+               var info, isTimeout,
+                       $changesListContents = $root.find( '.mw-changeslist' ).first().contents(),
+                       areResults = !!$changesListContents.length;
+
+               info = {
+                       changes: $changesListContents.length ? $changesListContents : 'NO_RESULTS',
+                       fieldset: $root.find( 'fieldset.cloptions' ).first()
+               };
+
+               if ( !areResults ) {
+                       isTimeout = !!$root.find( '.mw-changeslist-timeout' ).length;
+                       info.noResultsDetails = isTimeout ? 'NO_RESULTS_TIMEOUT' : 'NO_RESULTS_NORMAL';
+               }
+
+               return info;
+       };
+
        /**
         * Create filter data from a number, for the filters that are numerical value
         *
                                        this.changesListModel.update(
                                                $changesListContent,
                                                $fieldset,
+                                               pieces.noResultsDetails === 'NO_RESULTS_TIMEOUT',
                                                false,
                                                // separator between old and new changes
                                                updateMode === this.SHOW_NEW_CHANGES || updateMode === this.LIVE_UPDATE
                return this._queryChangesList( 'updateChangesList' )
                        .then(
                                function ( data ) {
-                                       var $parsed = $( '<div>' ).append( $( $.parseHTML( data.content ) ) ),
-                                               pieces = {
-                                                       // Changes list
-                                                       changes: $parsed.find( '.mw-changeslist' ).first().contents(),
-                                                       // Fieldset
-                                                       fieldset: $parsed.find( 'fieldset.cloptions' ).first()
-                                               };
-
-                                       if ( pieces.changes.length === 0 ) {
-                                               pieces.changes = 'NO_RESULTS';
-                                       }
+                                       var $parsed = $( '<div>' ).append( $( $.parseHTML( data.content ) ) );
 
-                                       return pieces;
-                               }
+                                       return this._extractChangesListInfo( $parsed );
+
+                               }.bind( this )
                        );
        };
 
index bab8ee5..dd095dd 100644 (file)
                                savedLinksListWidget = new mw.rcfilters.ui.SavedLinksListWidget(
                                        controller, savedQueriesModel, { $overlay: $overlay }
                                ),
-                               specialPage = mw.config.get( 'wgCanonicalSpecialPageName' );
+                               specialPage = mw.config.get( 'wgCanonicalSpecialPageName' ),
+                               $changesListRoot = $( '.mw-changeslist, .mw-changeslist-empty, .mw-changeslist-timeout' );
 
                        // TODO: The changesListWrapperWidget should be able to initialize
                        // after the model is ready.
+
                        // eslint-disable-next-line no-new
                        new mw.rcfilters.ui.ChangesListWrapperWidget(
-                               filtersModel, changesListModel, controller, $( '.mw-changeslist, .mw-changeslist-empty' ) );
+                               filtersModel, changesListModel, controller, $changesListRoot );
 
                        // Remove the -loading class that may have been added on the server side.
                        // If we are in fact going to load a default saved query, this .initialize()
index ba7a70e..a2518e0 100644 (file)
        }
 
        .mw-changeslist {
-               &-empty {
-                       // Hide the 'empty' message when we load rcfilters
-                       // since we replace it anyways with a specific
-                       // message of our own
-                       display: none;
-               }
-
                // Reserve space for the highlight circles
                ul,
                table.mw-enhanced-rc {
                }
        }
 
+       // Temporarily hide any 'empty' or 'timeout' message while we
+       // load rcfilters.
+       .mw-changeslist-empty,
+       .mw-changeslist-timeout {
+               display: none;
+       }
+
        body.mw-rcfilters-ui-loading .mw-changeslist {
                opacity: 0.5;
        }
index 83e68a5..d4faf83 100644 (file)
@@ -46,6 +46,8 @@
                this.$element
                        .addClass( 'mw-rcfilters-ui-changesListWrapperWidget' )
                        // We handle our own display/hide of the empty results message
+                       // We keep the timeout class here and remove it later, since at this
+                       // stage it is still needed to identify that the timeout occurred.
                        .removeClass( 'mw-changeslist-empty' );
 
                this.setupNewChangesButtonContainer();
         *
         * @param {jQuery|string} $changesListContent The content of the updated changes list
         * @param {jQuery} $fieldset The content of the updated fieldset
+        * @param {boolean} isDatabaseTimeout Whether this is an error state due to a database query
         * @param {boolean} isInitialDOM Whether $changesListContent is the existing (already attached) DOM
         * @param {boolean} from Timestamp of the new changes
         */
        mw.rcfilters.ui.ChangesListWrapperWidget.prototype.onModelUpdate = function (
-               $changesListContent, $fieldset, isInitialDOM, from
+               $changesListContent, $fieldset, isDatabaseTimeout, isInitialDOM, from
        ) {
-               var conflictItem,
+               var conflictItem, noResultsKey,
                        $message = $( '<div>' )
                                .addClass( 'mw-rcfilters-ui-changesListWrapperWidget-results' ),
                        isEmpty = $changesListContent === 'NO_RESULTS',
                                                        .text( mw.message( conflictItem.getCurrentConflictResultMessage() ).text() )
                                        );
                        } else {
+                               noResultsKey = isDatabaseTimeout ?
+                                       'recentchanges-timeout' :
+                                       'recentchanges-noresult';
+
                                $message
                                        .append(
                                                $( '<div>' )
                                                        .addClass( 'mw-rcfilters-ui-changesListWrapperWidget-results-noresult' )
-                                                       .text( mw.message( 'recentchanges-noresult' ).text() )
+                                                       .text( mw.message( noResultsKey ).text() )
                                        );
+
+                               this.$element.removeClass( 'mw-changeslist-timeout' );
                        }
 
                        this.$element.append( $message );
index 83905d5..4edc272 100644 (file)
         *
         * @param {jQuery|string} $changesList Updated changes list
         * @param {jQuery} $fieldset Updated fieldset
+        * @param {boolean} isDatabaseTimeout Whether this is an error state due to a database query
         * @param {boolean} isInitialDOM Whether $changesListContent is the existing (already attached) DOM
         */
-       mw.rcfilters.ui.FormWrapperWidget.prototype.onChangesModelUpdate = function ( $changesList, $fieldset, isInitialDOM ) {
+       mw.rcfilters.ui.FormWrapperWidget.prototype.onChangesModelUpdate = function ( $changesList, $fieldset, isDatabaseTimeout, isInitialDOM ) {
                this.$submitButton.prop( 'disabled', false );
 
                // Replace the entire fieldset