Page MenuHomePhabricator

CoreParserFunctions::revisionuser: Call to a member function getUser() on boolean
Closed, ResolvedPublicPRODUCTION ERROR

Description

Error

MediaWiki version: 1.35.0-wmf.31

message
Call to a member function getUser() on boolean

Impact

Have seen 2 of these since 1.35.0-wmf.30 rolled to group0 (T249963). Reproducible at the given URL.

Notes

Caused by rMWfd9c48d38eb6d104eec3c5ce0d06a804881089e9

Details

Request ID
XrHMRApAMNQAAYQk7mkAAAAS
Request URL
https://www.mediawiki.org/wiki/Help_talk:Magic_words
Stack Trace
exception.trace
2020-05-05 19:50:41 [XrHDkQpAICAAAA4M@igAAACK] mw1330 mediawikiwiki 1.35.0-wmf.31 exception ERROR: [XrHDkQpAICAAAA4M@igAAACK] /wiki/Help_talk:Magic_words   Error from line 1400 of /srv/mediawiki/php-1.35.0-wmf.31/includes/parser/CoreParserFunctions.php: Call to a member function getUser() on boolean {"exception_id":"XrHDkQpAICAAAA4M@igAAACK","exception_url":"/wiki/Help_talk:Magic_words","caught_by":"mwe_handler"} 
[Error Error] (/srv/mediawiki/php-1.35.0-wmf.31/includes/parser/CoreParserFunctions.php:1400) Call to a member function getUser() on boolean
#0 /srv/mediawiki/php-1.35.0-wmf.31/includes/parser/Parser.php(3322): CoreParserFunctions::revisionuser(Parser, string)
#1 /srv/mediawiki/php-1.35.0-wmf.31/includes/parser/Parser.php(3026): Parser->callParserFunction(PPFrame_Hash, string, array)
#2 /srv/mediawiki/php-1.35.0-wmf.31/includes/parser/PPFrame_Hash.php(253): Parser->braceSubstitution(array, PPFrame_Hash)
#3 /srv/mediawiki/php-1.35.0-wmf.31/includes/parser/Parser.php(2867): PPFrame_Hash->expand(PPNode_Hash_Tree, integer)
#4 /srv/mediawiki/php-1.35.0-wmf.31/includes/parser/Parser.php(1553): Parser->replaceVariables(string)
#5 /srv/mediawiki/php-1.35.0-wmf.31/includes/parser/Parser.php(640): Parser->internalParse(string)
#6 /srv/mediawiki/php-1.35.0-wmf.31/includes/content/WikitextContent.php(368): Parser->parse(string, Title, ParserOptions, boolean, boolean, integer)
#7 /srv/mediawiki/php-1.35.0-wmf.31/includes/content/AbstractContent.php(565): WikitextContent->fillParserOutput(Title, integer, ParserOptions, boolean, ParserOutput)
#8 /srv/mediawiki/php-1.35.0-wmf.31/includes/Revision/RenderedRevision.php(267): AbstractContent->getParserOutput(Title, integer, ParserOptions, boolean)
#9 /srv/mediawiki/php-1.35.0-wmf.31/includes/Revision/RenderedRevision.php(236): MediaWiki\Revision\RenderedRevision->getSlotParserOutputUncached(WikitextContent, boolean)
#10 /srv/mediawiki/php-1.35.0-wmf.31/includes/Revision/RevisionRenderer.php(215): MediaWiki\Revision\RenderedRevision->getSlotParserOutput(string)
#11 /srv/mediawiki/php-1.35.0-wmf.31/includes/Revision/RevisionRenderer.php(152): MediaWiki\Revision\RevisionRenderer->combineSlotOutput(MediaWiki\Revision\RenderedRevision, array)
#12 [internal function]: MediaWiki\Revision\RevisionRenderer->MediaWiki\Revision\{closure}(MediaWiki\Revision\RenderedRevision, array)
#13 /srv/mediawiki/php-1.35.0-wmf.31/includes/Revision/RenderedRevision.php(198): call_user_func(Closure, MediaWiki\Revision\RenderedRevision, array)
#14 /srv/mediawiki/php-1.35.0-wmf.31/includes/poolcounter/PoolWorkArticleView.php(204): MediaWiki\Revision\RenderedRevision->getRevisionParserOutput()
#15 /srv/mediawiki/php-1.35.0-wmf.31/includes/poolcounter/PoolCounterWork.php(128): PoolWorkArticleView->doWork()
#16 /srv/mediawiki/php-1.35.0-wmf.31/includes/page/Article.php(806): PoolCounterWork->execute()
#17 /srv/mediawiki/php-1.35.0-wmf.31/includes/actions/ViewAction.php(74): Article->view()
#18 /srv/mediawiki/php-1.35.0-wmf.31/includes/MediaWiki.php(519): ViewAction->show()
#19 /srv/mediawiki/php-1.35.0-wmf.31/includes/MediaWiki.php(305): MediaWiki->performAction(Article, Title)
#20 /srv/mediawiki/php-1.35.0-wmf.31/includes/MediaWiki.php(973): MediaWiki->performRequest()
#21 /srv/mediawiki/php-1.35.0-wmf.31/includes/MediaWiki.php(535): MediaWiki->main()
#22 /srv/mediawiki/php-1.35.0-wmf.31/index.php(47): MediaWiki->run()
#23 /srv/mediawiki/w/index.php(3): require(string)
#24 {main}

Event Timeline

brennen created this task.May 5 2020, 8:59 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMay 5 2020, 8:59 PM
brennen renamed this task from Call to a member function getUser() on boolean to CoreParserFunctions::revisionuser: Call to a member function getUser() on boolean.May 5 2020, 9:01 PM
brennen added a project: User-brennen.
Reedy added subscribers: DannyS712, Reedy.

rMWfd9c48d38eb6: Have CoreParserFunctions::getCachedRevisionObject return RevisionRecord was the last to touch it... but that added a safeguard for === null not false

Pinging @DannyS712 juuuuust in case. :)

(I note 28cb11e8 is the most recent change to includes/parser/Parser.php.)

DannyS712 added a comment.EditedMay 5 2020, 9:08 PM

rMWfd9c48d38eb6: Have CoreParserFunctions::getCachedRevisionObject return RevisionRecord was the last to touch it... but that added a safeguard for === null not false

CoreParserFunctions::getCachedRevisionObject is documented to return only a RevisionRecord or null, but I guess the previous check was if it was false (null or false) and now only checks if its null.

Change 594570 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/core@master] CoreParserFunctions::revisionuser - skip all falsey values

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

Change 594570 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/core@master] CoreParserFunctions::revisionuser - skip all falsey values

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

Unable to test, and commit message was updated; now only calls getUser if the value is instanceof RevisionRecord

DannyS712 moved this task from Unsorted to In progress on the User-DannyS712 board.

Pinging @DannyS712 juuuuust in case. :)

(I note 28cb11e8 is the most recent change to includes/parser/Parser.php.)

Doesn't involve getUser, but it was probably the other thing I did in CoreParserFunctions

brennen triaged this task as Unbreak Now! priority.May 5 2020, 9:21 PM
Restricted Application added a subscriber: Liuxinyu970226. · View Herald TranscriptMay 5 2020, 9:21 PM

Change 594572 had a related patch set uploaded (by Reedy; owner: DannyS712):
[mediawiki/core@wmf/1.35.0-wmf.31] CoreParserFunctions::revisionuser - only call getUser on RevisionRecord

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

DannyS712 claimed this task.May 5 2020, 9:39 PM
cscott added a subscriber: cscott.May 5 2020, 9:45 PM

I also refactored the magic words out in 0a53c9725ac44b88c628b7c6ddd543ae5cf50a8c, but that went out in wmf.30. Still, if it's a low occurrence log it's possible we didn't see it until now.

Change 594570 merged by jenkins-bot:
[mediawiki/core@master] CoreParserFunctions::revisionuser - only call getUser on RevisionRecord

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

DannyS712 updated the task description. (Show Details)May 5 2020, 9:49 PM

Change 594572 merged by jenkins-bot:
[mediawiki/core@wmf/1.35.0-wmf.31] CoreParserFunctions::revisionuser - only call getUser on RevisionRecord

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

cscott added a comment.May 5 2020, 9:55 PM

Nevermind. It's the RevisionRecord stuff from ParserOptions that @DannyS712 and I were reviewing back and forth a while ago. Parser::statelessFetchRevision() and Parser::statelessFetchRevisionRecord() can both return false. They are invoked via:

Parser::getRevisionRecordObject() -> calling the result of ParserOptions::getCurrentRevisionRecordCallback() -> Parser::statelessFetchRevisionRecord() -> RevisionLookup::getKnownCurrentRevision() which returns boolean false if the revision is missing.

Neither CoreParserFunctions::getCacheRevisionObject() nor Parser::getRevisionRecordObject() are declared as being able to return false.

My guess is that it's Parser::getRevisionRecordObject() while should sanitize the false returned from getCurrentRevisionRecordCallback() and turn it into a null. Then the patch in rMW2219ea3bece1 could be reverted.

Mentioned in SAL (#wikimedia-operations) [2020-05-05T21:57:27Z] <reedy@deploy1001> Synchronized php-1.35.0-wmf.31/includes/parser/CoreParserFunctions.php: T251952 (duration: 01m 05s)

Mentioned in SAL (#wikimedia-operations) [2020-05-05T22:00:27Z] <reedy@deploy1001> Synchronized php-1.35.0-wmf.31/includes/parser/CoreParserFunctions.php: T251952 take 2 (duration: 01m 06s)

Reedy added a comment.May 5 2020, 10:38 PM

Nevermind. It's the RevisionRecord stuff from ParserOptions that @DannyS712 and I were reviewing back and forth a while ago. Parser::statelessFetchRevision() and Parser::statelessFetchRevisionRecord() can both return false. They are invoked via:

Parser::getRevisionRecordObject() -> calling the result of ParserOptions::getCurrentRevisionRecordCallback() -> Parser::statelessFetchRevisionRecord() -> RevisionLookup::getKnownCurrentRevision() which returns boolean false if the revision is missing.

Neither CoreParserFunctions::getCacheRevisionObject() nor Parser::getRevisionRecordObject() are declared as being able to return false.

My guess is that it's Parser::getRevisionRecordObject() while should sanitize the false returned from getCurrentRevisionRecordCallback() and turn it into a null. Then the patch in rMW2219ea3bece1 could be reverted.

We should fork this into another task, as this one was specifically about it blocking the train..

Change 594752 had a related patch set uploaded (by C. Scott Ananian; owner: C. Scott Ananian):
[mediawiki/core@master] Fix impedance mismatch with Parser::getRevisionRecordObject()

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

brennen moved this task from Backlog to Done / Defunct on the User-brennen board.May 12 2020, 4:51 PM

Change 594752 merged by jenkins-bot:
[mediawiki/core@master] Fix impedance mismatch with Parser::getRevisionRecordObject()

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