Maniphest T251952

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

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}

Project	Branch	Lines +/-	Subject
mediawiki/core	master	+9 -3	Fix impedance mismatch with Parser::getRevisionRecordObject()
mediawiki/core	master	+3 -1	CoreParserFunctions::revisionuser - only call getUser on RevisionRecord
mediawiki/core	wmf/1.35.0-wmf.31	+3 -1	CoreParserFunctions::revisionuser - only call getUser on RevisionRecord

Customize query in gerrit

Related Objects
Search...

		Status	Subtype	Assigned	Task
		Resolved	Release	brennen	T249963 1.35.0-wmf.31 deployment blockers
		Resolved	PRODUCTION ERROR	DannyS712	T251952 CoreParserFunctions::revisionuser: Call to a member function getUser() on boolean

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

Reedy added a project: Parsing-Team--ARCHIVED.May 5 2020, 9:01 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.

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.)

In T251952#6110700, @Reedy wrote:

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

gerritbot added a project: Patch-For-Review.May 5 2020, 9:09 PM

In T251952#6110713, @gerritbot wrote:

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

In T251952#6110709, @brennen wrote:

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

DannyS712 mentioned this in T249963: 1.35.0-wmf.31 deployment blockers.May 5 2020, 9:20 PM

brennen triaged this task as Unbreak Now! priority.May 5 2020, 9:21 PM

brennen added a parent task: T249963: 1.35.0-wmf.31 deployment blockers.

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

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

DannyS712 removed a project: Patch-For-Review.May 5 2020, 9:54 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)

ReleaseTaggerBot added a project: MW-1.35-notes (1.35.0-wmf.32; 2020-05-12).May 5 2020, 10:00 PM

https://www.mediawiki.org/wiki/Help_talk:Magic_words works

In T251952#6110868, @cscott wrote:

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

gerritbot added a project: Patch-For-Review.May 6 2020, 4:46 PM

DannyS712 moved this task from Apr 2020 to May 2020 on the Wikimedia-production-error board.May 6 2020, 5:28 PM

DannyS712 mentioned this in T249964: 1.35.0-wmf.32 deployment blockers.May 11 2020, 5:19 PM

brennen moved this task from Backlog to Done or Declined 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

ReleaseTaggerBot edited projects, added MW-1.35-notes (1.35.0-wmf.34; 2020-05-26); removed MW-1.35-notes (1.35.0-wmf.32; 2020-05-12).May 20 2020, 4:00 PM

Maintenance_bot removed a project: Patch-For-Review.May 20 2020, 4:10 PM

DannyS712 mentioned this in T253725: Call to a member function getUser() on boolean ( CoreParserFunctions::revisionuser ?).May 27 2020, 8:04 AM

DannyS712 moved this task from In progress to Resolved (deprecation) on the User-DannyS712 board.May 27 2020, 8:36 AM