Page MenuHomePhabricator

Viewing Special:CentralNoticeBanners subpage can throw fatal BannerExistenceException
Closed, ResolvedPublicPRODUCTION ERROR

Description

Link toreproduce:

https://meta.wikimedia.org/wiki/Special:CentralNoticeBanners/edit/strategicplan2

log message:

{"id":"WBEZQwpAMEcAAE3-g0kAAAAO","type":"BannerExistenceException","file":"/srv/mediawiki/php-1.28.0-wmf.23/extensions/CentralNotice/includes/Banner.php","line":348,"message":"No banner exists where tmp_name = strategicplan2. Could not load.","code":0,"ur

Code throwing an exception:

Banner::populateBasicData
// Extract the dataz!
$row = $db->fetchObject( $rowRes );
if ( $row ) {
    
} else {
    $keystr = array();
    foreach ( $selector as $key => $value ) {
        $keystr[] = "{$key} = {$value}";
    }
    $keystr = implode( " AND ", $keystr );
    throw new BannerExistenceException( "No banner exists where {$keystr}. Could not load." );
}

Event Timeline

Dereckson renamed this task from [bug] CentralNoticeBanners fatal in 1.28.0-wmf.23 - to Don't serve an exception on Special:CentralNoticeBanners when the banner doesn't exist.Oct 26 2016, 9:10 PM
Dereckson added a subscriber: Dereckson.

It would probably worthwhile to check the existence of the banner before call this method, or to intercept the exception and in both cases, to display instead an user friendly message.

Wow, looks like this has been around for quite a while...

Should be an easy fix, anyone feel up to it?

Krinkle renamed this task from Don't serve an exception on Special:CentralNoticeBanners when the banner doesn't exist to Viewing Special:CentralNoticeBanners subpage can throw fatal BannerExistenceException.Sep 19 2018, 11:06 PM

Still seen. Looks like the the CentralNotice code may need to updated to acknowledge the user error (invalid/unknown banner ID).

Currently, the error is unhandled and bubbles to the application layer, where it raises our alert levels and would indicate "MediaWiki in production" is having issues.

Change 501049 had a related patch set uploaded (by Mepps; owner: Mepps):
[mediawiki/extensions/CentralNotice@master] Use banner instance variable sooner and check for errors at Page Level

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

Change 501049 merged by jenkins-bot:
[mediawiki/extensions/CentralNotice@master] Use banner instance variable sooner and check for errors in SpecialCentralNoticeBanners

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

mmodell changed the subtype of this task from "Task" to "Production Error".Aug 28 2019, 11:11 PM