Page MenuHomePhabricator

PHP error "Revision::ensureTitle: Could not determine title for page " in SpecialMobileContributions
Closed, ResolvedPublic5 Story Points

Description

Logstash contains 290 recent hits (and counting) for the "Could not determine title for page ID" error, most of which originate from Special:Contributions (MobileFrontend):

PHP Warning: Revision::ensureTitle: Could not determine title for page ID 28383503 and revision ID 379236967 [Called from Revision::ensureTitle in /srv/mediawiki/php-1.32.0-wmf.10/includes/Revision.php at line 595]
#0 /srv/mediawiki/php-1.32.0-wmf.10/includes/debug/MWDebug.php(309): MWExceptionHandler::handleError(integer, string, string, integer, array, array)
#1 /srv/mediawiki/php-1.32.0-wmf.10/includes/debug/MWDebug.php(164): MWDebug::sendMessage(string, array, string, integer)
#2 /srv/mediawiki/php-1.32.0-wmf.10/includes/GlobalFunctions.php(1142): MWDebug::warning(string, integer, integer, string)
#3 /srv/mediawiki/php-1.32.0-wmf.10/includes/Revision.php(595): wfLogWarning(string)
#4 /srv/mediawiki/php-1.32.0-wmf.10/includes/Revision.php(552): Revision->ensureTitle(stdClass, integer, NULL)
#5 /srv/mediawiki/php-1.32.0-wmf.10/extensions/MobileFrontend/includes/specials/SpecialMobileContributions.php(99): Revision->__construct(stdClass)
#6 /srv/mediawiki/php-1.32.0-wmf.10/extensions/MobileFrontend/includes/specials/SpecialMobileContributions.php(80): SpecialMobileContributions->showContributions(Wikimedia\Rdbms\ResultWrapper)
#7 /srv/mediawiki/php-1.32.0-wmf.10/extensions/MobileFrontend/includes/specials/MobileSpecialPage.php(57): SpecialMobileContributions->executeWhenAvailable(string)
#8 /srv/mediawiki/php-1.32.0-wmf.10/extensions/MobileFrontend/includes/specials/MobileSpecialPageFeed.php(26): MobileSpecialPage->execute(string)
#9 /srv/mediawiki/php-1.32.0-wmf.10/includes/specialpage/SpecialPage.php(565): MobileSpecialPageFeed->execute(string)
#10 /srv/mediawiki/php-1.32.0-wmf.10/includes/specialpage/SpecialPageFactory.php(569): SpecialPage->run(string)
#11 /srv/mediawiki/php-1.32.0-wmf.10/includes/MediaWiki.php(288): SpecialPageFactory::executePath(Title, RequestContext)
#12 /srv/mediawiki/php-1.32.0-wmf.10/includes/MediaWiki.php(867): MediaWiki->performRequest()
#13 /srv/mediawiki/php-1.32.0-wmf.10/includes/MediaWiki.php(524): MediaWiki->main()
#14 /srv/mediawiki/php-1.32.0-wmf.10/index.php(42): MediaWiki->run()
#15 /srv/mediawiki/w/index.php(3): include(string)
#16 {main}

This can be consistently reproduced at the following URL:

https://en.m.wikipedia.org/wiki/Special:Contributions/Homer_saves_presidents

Which renders as follows (notice the strange entries attributed to "Special:Badtitle"):

The same view on the canonical domain:
https://en.wikipedia.org/wiki/Special:Contributions/Homer_saves_presidents

However, while the canonical view does not emit a PHP error, and does not show "Special:Badtitle", it also doesn't show the revisions in question. So not necessarily better, depending on what the underlying problem is...

QA steps

Test the Special:Contributions page on mobile (https://en.m.wikipedia.beta.wmflabs.org/wiki/Special:Contributions/Jdlrobson) for various users and confirm that you are seeing consistency with the desktop equivalent.

Event Timeline

Krinkle created this task.Jul 8 2018, 8:45 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 8 2018, 8:45 PM

This is ugly.

The core problem here is that SpecialMobileContributions doesn't extend core's ContribsPager but re-implements it. The relevant code that "fixes" this bug in core is:

/*
 * There may be more than just revision rows. To make sure that we'll only be processing
 * revisions here, let's _try_ to build a revision out of our row (without displaying
 * notices though) and then trying to grab data from the built object. If we succeed,
 * we're definitely dealing with revision data and we may proceed, if not, we'll leave it
 * to extensions to subscribe to the hook to parse the row.
 */
Wikimedia\suppressWarnings();
try {
	$rev = new Revision( $row );
	$validRevision = (bool)$rev->getId();
} catch ( Exception $e ) {
	$validRevision = false;
}
Wikimedia\restoreWarnings();

This code is… not good. Copying into MF seems unclean.

Change 446748 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[mediawiki/extensions/MobileFrontend@master] SpecialMobileContributions: Don't show fake revs, like page move "to" entries

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

phuedx added a subscriber: phuedx.Jul 19 2018, 8:56 AM

Thanks for investigating and submitting the intermediate fix, @Jdforrester-WMF. Once the error's dealt with, Readers Web should look at using ContribsPager rather than duplicating it.

Thanks for investigating and submitting the intermediate fix, @Jdforrester-WMF. Once the error's dealt with, Readers Web should look at using ContribsPager rather than duplicating it.

Totally, though ContribsPager's code isn't great either. :-(

phuedx triaged this task as Normal priority.Jul 19 2018, 9:00 AM
phuedx added a subscriber: ovasileva.

I'll triage this as Normal for now but bring it onto the board and into Needs Code Review as there's already a fix to be reviewed /cc @ovasileva.

Change 447612 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] Special:MobileContributions should use ContribsPager

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

Jdlrobson added a subscriber: Jdlrobson.EditedJul 24 2018, 12:54 PM

Since this has happened before in a different form, I'd like to use this opportunity to entertain using the Pager here given the low amount of errors rather than piling on more hacks. What do you think?

I've written https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/MobileFrontend/+/447612 but am not sure how to replicate the error right now to check if that alone solves this? Does it?

@Krinkle @Jdforrester-WMF any ideas on how to replicate this locally?

I had a way to reproduce it, but unable to at the moment due to T200269.

I haven't been able to re-create the kind of contribution history that we see on en.wikipedia.org. It might be that the "move" action creates revisions a different way now than it used to in 2010. Or it might be that what we see on enwiki is the result of multiple actions (e.g. the page later being renamed again, or being deleted/re-created or some such).

So aside from the example in the task description that we can test, I'm not sure how to test it on another wiki or locally.

Jdforrester-WMF added a comment.EditedJul 25 2018, 3:30 PM

I haven't been able to re-create the kind of contribution history that we see on en.wikipedia.org. It might be that the "move" action creates revisions a different way now than it used to in 2010.

Correct; this is a bug affecting how move used to work and how some extensions still work, I believe.

So aside from the example in the task description that we can test, I'm not sure how to test it on another wiki or locally.

I manually duped the revision DB rows from production to my local wiki for testing (which is cheating, but…).

Have also not been able to reproduce. If we take the approach in https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/MobileFrontend/+/447612 can we simply call formatRow ?

@Jdforrester-WMF are you able to test/work with the patch in https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/MobileFrontend/+/447612 since you can recreate this issue?

Change 448001 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/core@master] Refactor revision check out of formatRow

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

Change 446748 abandoned by Jforrester:
SpecialMobileContributions: Don't show fake revs, like page move "to" entries

Reason:
Better to go with Jon's proper fix.

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

pmiazga added a subscriber: pmiazga.

Added Technical-Debt as this patches FIXME: Make Special:MobileContributions use ContribsPager ASAP. that was added two years ago. Also moving to doing column to reflect that fact that we're currently working on this task.

ovasileva set the point value for this task to 5.Jul 30 2018, 5:24 PM

We discussed this in standup briefly and estimated it as a 5. Due to the larger estimate and the new approach, we decided to move back to upcoming for the time being.

Change 448001 merged by jenkins-bot:
[mediawiki/core@master] ContribsPager: Factor revision check out of formatRow

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

Change 446748 restored by Jdlrobson:
SpecialMobileContributions: Don't show fake revs, like page move "to" entries

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

phuedx removed a subscriber: phuedx.Sep 12 2018, 10:41 AM

We have two approaches to this:

Sorry, meant to put this in <= .19 instead.

SBisson moved this task from Inbox to External on the Growth-Team board.Sep 28 2018, 2:37 AM

Change 446748 abandoned by Jdlrobson:
SpecialMobileContributions: Don't show fake revs, like page move "to" entries

Reason:
From Timo's comments, I think we prefer the ContribsPager approach in https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/MobileFrontend/ /446748/ ?

Please restore if wrong.

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

Jdlrobson assigned this task to pmiazga.Oct 23 2018, 5:20 PM
pmiazga removed pmiazga as the assignee of this task.Oct 24 2018, 12:51 AM
pmiazga removed a project: Patch-For-Review.

Change 447612 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Special:MobileContributions should use ContribsPager

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

Jdlrobson updated the task description. (Show Details)Oct 24 2018, 1:15 AM
Jdlrobson added a project: Product-QA.
pmiazga claimed this task.Nov 14 2018, 6:16 PM
pmiazga added a subscriber: Ryasmeen.

This has been live in production for about a month now so you can QA in production...

No more badtitle (or production log errors):

Tested, looks correct.

pmiazga closed this task as Resolved.Nov 15 2018, 6:45 PM
pmiazga removed pmiazga as the assignee of this task.
Restricted Application added a project: User-Ryasmeen. · View Herald TranscriptNov 15 2018, 6:45 PM
mmodell changed the subtype of this task from "Task" to "Production Error".Aug 28 2019, 11:09 PM