Page MenuHomePhabricator

Special:Log/block inaccessible on several wikis with fatal ParameterAssertionException "Bad value for parameter $dbkey: should not be empty"
Closed, ResolvedPublic

Description

When I try to load the log I get the following:

Internal error
[XPLNJwpAIC4AACzC5hEAAAAX] 2019-06-01 19:08:23: Fatal exception of type "Wikimedia\Assert\ParameterAssertionException"

Details

Related Gerrit Patches:

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJun 1 2019, 7:10 PM
Aklapper renamed this task from Block log on az.wikiquote does not load to Cannot open block log: "Bad value for parameter $dbkey: should not be empty unless namespace is main and fragment is non-empty".Jun 1 2019, 7:53 PM
Aklapper added a project: MediaWiki-Logging.
Bad value for parameter $dbkey: should not be empty unless namespace is main and fragment is non-empty
exception.trace
#0 /srv/mediawiki/php-1.34.0-wmf.7/includes/title/TitleValue.php(107): Wikimedia\Assert\Assert::parameter(boolean, string, string)
#1 /srv/mediawiki/php-1.34.0-wmf.7/includes/title/NamespaceInfo.php(162): TitleValue->__construct(integer, string)
#2 /srv/mediawiki/php-1.34.0-wmf.7/includes/Title.php(1523): NamespaceInfo->getTalkPage(Title)
#3 /srv/mediawiki/php-1.34.0-wmf.7/includes/logging/BlockLogFormatter.php(131): Title->getTalkPage()
#4 /srv/mediawiki/php-1.34.0-wmf.7/includes/logging/LogPager.php(385): BlockLogFormatter->getPreloadTitles()
#5 /srv/mediawiki/php-1.34.0-wmf.7/includes/pager/IndexPager.php(478): LogPager->getStartBody()
#6 /srv/mediawiki/php-1.34.0-wmf.7/includes/specials/SpecialLog.php(249): IndexPager->getBody()
#7 /srv/mediawiki/php-1.34.0-wmf.7/includes/specials/SpecialLog.php(139): SpecialLog->show(FormOptions, array)
#8 /srv/mediawiki/php-1.34.0-wmf.7/includes/specialpage/SpecialPage.php(570): SpecialLog->execute(NULL)
#9 /srv/mediawiki/php-1.34.0-wmf.7/includes/specialpage/SpecialPageFactory.php(575): SpecialPage->run(NULL)
#10 /srv/mediawiki/php-1.34.0-wmf.7/includes/MediaWiki.php(288): MediaWiki\Special\SpecialPageFactory->executePath(Title, RequestContext)
#11 /srv/mediawiki/php-1.34.0-wmf.7/includes/MediaWiki.php(865): MediaWiki->performRequest()
#12 /srv/mediawiki/php-1.34.0-wmf.7/includes/MediaWiki.php(515): MediaWiki->main()
#13 /srv/mediawiki/php-1.34.0-wmf.7/index.php(42): MediaWiki->run()
#14 /srv/mediawiki/w/index.php(3): include(string)
#15 {main}

Probably not T224095, might be T222628 or T224664#5223697, or something separate. I admit I lost track about all those similar issues lately.

Krinkle renamed this task from Cannot open block log: "Bad value for parameter $dbkey: should not be empty unless namespace is main and fragment is non-empty" to Fatal error from Special:Log/block: "Bad value for parameter $dbkey: should not be empty unless namespace is main and fragment is non-empty".Jun 1 2019, 11:30 PM
Krinkle triaged this task as High priority.EditedJun 1 2019, 11:38 PM
Krinkle added a project: Core Platform Team.
Krinkle added subscribers: Simetrical, daniel, Krinkle.

Likely regression from https://gerrit.wikimedia.org/r/507081 / e30cb5ba90860 which changed Title->getTalkPage to be much more strict (by virtue of using TitleValue), and possibly is missing support for this case.

The simplest fix would be to partially revert Title->getTalkPage back to the known-functional state without the new NamespaceInfo/TitleValue logic. But if more time is available, one could determine how to make it work in the new system (eg. validate within BlockLogFormatter by other means, or if it is genuinely unexpected, tracing where it comes from and fixing it in the database).

Tagging relevant author/reviewer/team from that commit.

Logstash query: +exception.trace:"BlockLogFormatter.php"

Krinkle renamed this task from Fatal error from Special:Log/block: "Bad value for parameter $dbkey: should not be empty unless namespace is main and fragment is non-empty" to Special:Log/block inaccessible on several wikis with fatal ParameterAssertionException "Bad value for parameter $dbkey: should not be empty".Jun 1 2019, 11:42 PM
Krinkle updated the task description. (Show Details)
Krinkle updated the task description. (Show Details)Jun 1 2019, 11:51 PM
This comment was removed by Krinkle.
Peachey88 updated the task description. (Show Details)Jun 2 2019, 12:00 AM
ashley added a subscriber: ashley.Jun 2 2019, 12:31 AM

On Inciclopedia (the Spanish Uncyclopedia, an Uncyclomedia project), we had a similar issue with the oldest log entry (a user rights log entry, also from 2006) having a bad log_title, which contained a trailing underscore for whatever reason (which MediaWiki correctly normalizes into a space, as it always does and as it should). Because "SomeUser " (note the space) is obviously not a valid Title, MW would barf.

Fixing that particular case was very easy -- but tracking down the root cause wasn't and made me wish that MW would just magically trim() the offending fields before attempting to construct a Title/TitleValue out of 'em. It's not just easier from a developer standpoint, but likely to be more friendly towards third parties as well because as witnessed by our case, it's possible that these bad entries go unnoticed in the database for 13 years before anyone stumbles upon them.
Having Special:Log break due to stuff like this and require manual sysadmin-level intervention isn't ideal; WMF and well-managed third-party sites can afford the sysadmin time required to "properly" fix the offending entries in the DB, but this may not be the case for smaller sites.

@ashley Can you confirm whether you ran into this before or after May 2019? I've been assuming right now that this broke because of the recently introduced use of TitleValue in the LogFormatter context. Previously this used Title::newFromText which is tolerant towards trailing spaces and normalises them. (Just like how they are tolerated in /wiki/_Urls_ and [[Links_]]) .

If you ran into this before the recent change, that would be good to know, because that means we need to find the issue elsewhere :)

ashley added a comment.Jun 2 2019, 1:06 PM

@ashley Can you confirm whether you ran into this before or after May 2019?

The original Trello ticket detailing this issue, complete with a user-submitted screenshot, was created on April 2nd and while my memory is a bit hazy, I think I fixed it within a couple days of reporting, which'd get us in the April 4-6 range or so; that being said, I'm quite sure we were using MW 1.32 at the time regardless.

daniel added a comment.Jun 3 2019, 8:48 PM

Fixing that particular case was very easy -- but tracking down the root cause wasn't and made me wish that MW would just magically trim() the offending fields before attempting to construct a Title/TitleValue out of 'em. It's not just easier from a developer standpoint, but likely to be more friendly towards third parties as well because as witnessed by our case, it's possible that these bad entries go unnoticed in the database for 13 years before anyone stumbles upon them.

Being more defensive about info read from the database seems desirable at a first glance, but may actually introduce more serious and even harder to track down issues. For instance, two entries in the database that only differe by the trailing whitespace would be treated as the same title internally, perhaps making it impossible to delete (or undelete) one of them, or causing confusion and corruption in secondary tables.

I can't think of a way to do this safely. If you can, please let me know.

daniel added a comment.Jun 3 2019, 8:58 PM

@ashley Can you confirm whether you ran into this before or after May 2019? I've been assuming right now that this broke because of the recently introduced use of TitleValue in the LogFormatter context. Previously this used Title::newFromText which is tolerant towards trailing spaces and normalises them. (Just like how they are tolerated in /wiki/_Urls_ and [[Links_]]) .

Can you point me to where Title::newFromText was previously used? As far as I can tell, Title::getTalkPage() was based on Title::makeTitle(), which doesn't do any normalization or validation at all.

Change 514167 had a related patch set uploaded (by Daniel Kinzler; owner: Daniel Kinzler):
[mediawiki/core@master] Tollerate invalid titles in some logs.

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

Change 514167 merged by jenkins-bot:
[mediawiki/core@master] Tolerate invalid titles in some ChangesFeed and LogFormatter code

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

@daniel: Your patch has been merged a week ago. Can this task be closed as resolved, or is there more to do here (and if so, is that still high priority)?

daniel closed this task as Resolved.Jul 2 2019, 12:54 PM
daniel claimed this task.

@Aklapper The links in the task description now work, so I'd say this is done.

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