From: Matthew Flaschen Date: Mon, 18 Sep 2017 11:42:07 +0000 (-0400) Subject: RCFilters: Display specific error if query times out X-Git-Tag: 1.31.0-rc.0~1688^2 X-Git-Url: http://git.cyclocoop.org/%27.parametre_url%28%20%20%20generer_action_auteur%28%27charger_plugin%27%2C%20%27update_flux%27%29%2C%27update_flux%27%2C%20%27oui%27%29.%27?a=commitdiff_plain;h=63235f2d7f42f94337d1f12aaaab8322868732b9;p=lhc%2Fweb%2Fwiklou.git RCFilters: Display specific error if query times out 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 --- diff --git a/includes/specialpage/ChangesListSpecialPage.php b/includes/specialpage/ChangesListSpecialPage.php index df6a1c173c..a3fee64d19 100644 --- a/includes/specialpage/ChangesListSpecialPage.php +++ b/includes/specialpage/ChangesListSpecialPage.php @@ -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( + '
' . + $this->msg( 'recentchanges-timeout' )->parse() . + '
' + ); + } + /** * 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 ); } diff --git a/languages/i18n/en.json b/languages/i18n/en.json index 282b906f31..c20452b173 100644 --- a/languages/i18n/en.json +++ b/languages/i18n/en.json @@ -1339,6 +1339,7 @@ "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", diff --git a/languages/i18n/qqq.json b/languages/i18n/qqq.json index 4d6cf93a4c..8a645bc711 100644 --- a/languages/i18n/qqq.json +++ b/languages/i18n/qqq.json @@ -1532,6 +1532,7 @@ "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}}", diff --git a/resources/Resources.php b/resources/Resources.php index 6d59d5ce6d..055ffbae97 100644 --- a/resources/Resources.php +++ b/resources/Resources.php @@ -1915,6 +1915,7 @@ return [ 'namespaces', 'invert', 'recentchanges-noresult', + 'recentchanges-timeout', 'quotation-marks', ], 'dependencies' => [ diff --git a/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.ChangesListViewModel.js b/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.ChangesListViewModel.js index debe0b904d..3c03c70a0d 100644 --- a/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.ChangesListViewModel.js +++ b/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.ChangesListViewModel.js @@ -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 * @@ -72,16 +73,18 @@ * * @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 ); }; /** diff --git a/resources/src/mediawiki.rcfilters/mw.rcfilters.Controller.js b/resources/src/mediawiki.rcfilters/mw.rcfilters.Controller.js index 3e7172930a..b805b24de5 100644 --- a/resources/src/mediawiki.rcfilters/mw.rcfilters.Controller.js +++ b/resources/src/mediawiki.rcfilters/mw.rcfilters.Controller.js @@ -41,14 +41,13 @@ * @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 ) { @@ -247,11 +246,14 @@ // 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 ); } @@ -265,6 +267,35 @@ } }; + /** + * 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 * @@ -964,6 +995,7 @@ 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 @@ -1124,20 +1156,11 @@ return this._queryChangesList( 'updateChangesList' ) .then( function ( data ) { - var $parsed = $( '
' ).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 = $( '
' ).append( $( $.parseHTML( data.content ) ) ); - return pieces; - } + return this._extractChangesListInfo( $parsed ); + + }.bind( this ) ); }; diff --git a/resources/src/mediawiki.rcfilters/mw.rcfilters.init.js b/resources/src/mediawiki.rcfilters/mw.rcfilters.init.js index bab8ee5d14..dd095dd06b 100644 --- a/resources/src/mediawiki.rcfilters/mw.rcfilters.init.js +++ b/resources/src/mediawiki.rcfilters/mw.rcfilters.init.js @@ -29,13 +29,15 @@ 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() diff --git a/resources/src/mediawiki.rcfilters/styles/mw.rcfilters.less b/resources/src/mediawiki.rcfilters/styles/mw.rcfilters.less index ba7a70ebc4..a2518e06d2 100644 --- a/resources/src/mediawiki.rcfilters/styles/mw.rcfilters.less +++ b/resources/src/mediawiki.rcfilters/styles/mw.rcfilters.less @@ -48,13 +48,6 @@ } .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 { @@ -62,6 +55,13 @@ } } + // 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; } diff --git a/resources/src/mediawiki.rcfilters/ui/mw.rcfilters.ui.ChangesListWrapperWidget.js b/resources/src/mediawiki.rcfilters/ui/mw.rcfilters.ui.ChangesListWrapperWidget.js index 83e68a52c6..d4faf836df 100644 --- a/resources/src/mediawiki.rcfilters/ui/mw.rcfilters.ui.ChangesListWrapperWidget.js +++ b/resources/src/mediawiki.rcfilters/ui/mw.rcfilters.ui.ChangesListWrapperWidget.js @@ -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(); @@ -117,13 +119,14 @@ * * @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 = $( '
' ) .addClass( 'mw-rcfilters-ui-changesListWrapperWidget-results' ), isEmpty = $changesListContent === 'NO_RESULTS', @@ -151,12 +154,18 @@ .text( mw.message( conflictItem.getCurrentConflictResultMessage() ).text() ) ); } else { + noResultsKey = isDatabaseTimeout ? + 'recentchanges-timeout' : + 'recentchanges-noresult'; + $message .append( $( '
' ) .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 ); diff --git a/resources/src/mediawiki.rcfilters/ui/mw.rcfilters.ui.FormWrapperWidget.js b/resources/src/mediawiki.rcfilters/ui/mw.rcfilters.ui.FormWrapperWidget.js index 83905d5be9..4edc272d1d 100644 --- a/resources/src/mediawiki.rcfilters/ui/mw.rcfilters.ui.FormWrapperWidget.js +++ b/resources/src/mediawiki.rcfilters/ui/mw.rcfilters.ui.FormWrapperWidget.js @@ -95,9 +95,10 @@ * * @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