Page MenuHomePhabricator

BadMethodCallException from line 3671 of /srv/mediawiki/php-1.32.0-wmf.18/includes/parser/Parser.php: Call to a member function getId() on a non-object (boolean)
Closed, ResolvedPublic

Description

Seen after rollout of wmf.18 to group1

Stack:

#0 /srv/mediawiki/php-1.32.0-wmf.18/includes/parser/Parser.php(3603): Parser::statelessFetchTemplate(Title, Parser)
#1 /srv/mediawiki/php-1.32.0-wmf.18/includes/parser/Parser.php(3537): Parser->fetchTemplateAndTitle(Title)
#2 /srv/mediawiki/php-1.32.0-wmf.18/includes/parser/Parser.php(3297): Parser->getTemplateDom(Title)
#3 /srv/mediawiki/php-1.32.0-wmf.18/includes/parser/Preprocessor_Hash.php(1114): Parser->braceSubstitution(array, PPFrame_Hash)
#4 /srv/mediawiki/php-1.32.0-wmf.18/includes/parser/Parser.php(2999): PPFrame_Hash->expand(PPNode_Hash_Tree, integer)
#5 /srv/mediawiki/php-1.32.0-wmf.18/includes/parser/Parser.php(1345): Parser->replaceVariables(string)
#6 /srv/mediawiki/php-1.32.0-wmf.18/includes/parser/Parser.php(682): Parser->internalParse(string, boolean, boolean)
#7 /srv/mediawiki/php-1.32.0-wmf.18/extensions/ProofreadPage/includes/Parser/PagesTagParser.php(326): Parser->recursiveTagParse(string)
#8 /srv/mediawiki/php-1.32.0-wmf.18/extensions/ProofreadPage/ProofreadPage.body.php(125): ProofreadPage\Parser\PagesTagParser->render(NULL, array)
#9 /srv/mediawiki/php-1.32.0-wmf.18/includes/parser/Parser.php(3942): Closure$ProofreadPage::onParserFirstCallInit#2(NULL, array, Parser, PPFrame_Hash)
#10 /srv/mediawiki/php-1.32.0-wmf.18/includes/parser/Preprocessor_Hash.php(1188): Parser->extensionSubstitution(array, PPFrame_Hash)
#11 /srv/mediawiki/php-1.32.0-wmf.18/includes/parser/Parser.php(2999): PPFrame_Hash->expand(PPNode_Hash_Tree, integer)
#12 /srv/mediawiki/php-1.32.0-wmf.18/includes/parser/Parser.php(1345): Parser->replaceVariables(string)
#13 /srv/mediawiki/php-1.32.0-wmf.18/includes/parser/Parser.php(474): Parser->internalParse(string)
#14 /srv/mediawiki/php-1.32.0-wmf.18/includes/content/WikitextContent.php(341): Parser->parse(string, Title, ParserOptions, boolean, boolean, integer)
#15 /srv/mediawiki/php-1.32.0-wmf.18/includes/content/AbstractContent.php(516): WikitextContent->fillParserOutput(Title, integer, ParserOptions, boolean, ParserOutput)
#16 /srv/mediawiki/php-1.32.0-wmf.18/extensions/FlaggedRevs/backend/FlaggedRevs.class.php(642): AbstractContent->getParserOutput(Title, integer, ParserOptions)
#17 /srv/mediawiki/php-1.32.0-wmf.18/extensions/FlaggedRevs/backend/FlaggedRevs.class.php(566): FlaggedRevs::parseStableRevision(FlaggedRevision, ParserOptions)
#18 /srv/mediawiki/php-1.32.0-wmf.18/includes/poolcounter/PoolCounterWorkViaCallback.php(69): Closure$FlaggedRevs::parseStableRevisionPooled()
#19 /srv/mediawiki/php-1.32.0-wmf.18/includes/poolcounter/PoolCounterWork.php(123): PoolCounterWorkViaCallback->doWork()
#20 /srv/mediawiki/php-1.32.0-wmf.18/extensions/FlaggedRevs/backend/FlaggedRevs.class.php(582): PoolCounterWork->execute()
#21 /srv/mediawiki/php-1.32.0-wmf.18/extensions/FlaggedRevs/frontend/FlaggablePageView.php(749): FlaggedRevs::parseStableRevisionPooled(FlaggedRevision, ParserOptions)
#22 /srv/mediawiki/php-1.32.0-wmf.18/extensions/FlaggedRevs/frontend/FlaggablePageView.php(391): FlaggablePageView->showStableVersion(FlaggedRevision, string, string)
#23 /srv/mediawiki/php-1.32.0-wmf.18/extensions/FlaggedRevs/frontend/FlaggedRevsUI.hooks.php(204): FlaggablePageView->setPageContent(boolean, boolean)
#24 /srv/mediawiki/php-1.32.0-wmf.18/includes/Hooks.php(174): FlaggedRevsUIHooks::onArticleViewHeader(Article, boolean, boolean)
#25 /srv/mediawiki/php-1.32.0-wmf.18/includes/Hooks.php(202): Hooks::callHook(string, array, array, NULL)
#26 /srv/mediawiki/php-1.32.0-wmf.18/includes/page/Article.php(524): Hooks::run(string, array)
#27 /srv/mediawiki/php-1.32.0-wmf.18/includes/actions/ViewAction.php(68): Article->view()
#28 /srv/mediawiki/php-1.32.0-wmf.18/includes/MediaWiki.php(501): ViewAction->show()
#29 /srv/mediawiki/php-1.32.0-wmf.18/includes/MediaWiki.php(294): MediaWiki->performAction(Article, Title)
#30 /srv/mediawiki/php-1.32.0-wmf.18/includes/MediaWiki.php(868): MediaWiki->performRequest()
#31 /srv/mediawiki/php-1.32.0-wmf.18/includes/MediaWiki.php(525): MediaWiki->main()
#32 /srv/mediawiki/php-1.32.0-wmf.18/index.php(42): MediaWiki->run()
#33 /srv/mediawiki/w/index.php(3): include(string)
#34 {main}

Details

Related Gerrit Patches:
mediawiki/extensions/FlaggedRevs : wmf/1.32.0-wmf.18Fix incorrect return value in CurrentRevisionCallback
mediawiki/extensions/FlaggedRevs : wmf/1.32.0-wmf.18Revert "Replace old and busted hook with the new hotness of a callback"
mediawiki/extensions/FlaggedRevs : masterFix incorrect return value in CurrentRevisionCallback
mediawiki/extensions/FlaggedRevs : masterRevert "Replace old and busted hook with the new hotness of a callback"

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 22 2018, 8:07 PM
thcipriani triaged this task as High priority.Aug 22 2018, 8:13 PM

Setting as high priority. This message is very noisy in the error logs.

thcipriani raised the priority of this task from High to Unbreak Now!.Aug 22 2018, 8:27 PM

Rolledback 1.32.0-wmf.18 from group1 wikis. Added this task as a train blocker. Setting UBN! since I added as a blocker.

Restricted Application added subscribers: Liuxinyu970226, TerraCodes. · View Herald TranscriptAug 22 2018, 8:28 PM

I would agree, that it comes from that patch set.

line 3671 of Parser is $rev_id = $rev ? $rev->getId() : 0;, looks safe for use of falsy values.

A possible truthy value here can only returned from Parser::fetchCurrentRevisionOfTitle which callback is overridden in FlaggedRevs and returns true.
The old code with return true was inside a hook, but now it is not a hook and must return a Revision or null

Change 454835 had a related patch set uploaded (by Thcipriani; owner: Thcipriani):
[mediawiki/extensions/FlaggedRevs@master] Revert "Replace old and busted hook with the new hotness of a callback"

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

I posted a revert of the patchset identified in this ticket. Is this the best route forward for this train, i.e., revert this patch in wmf.18?

Change 454839 had a related patch set uploaded (by Thcipriani; owner: Thcipriani):
[mediawiki/extensions/FlaggedRevs@wmf/1.32.0-wmf.18] Revert "Replace old and busted hook with the new hotness of a callback"

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

Change 454835 abandoned by Thcipriani:
Revert "Replace old and busted hook with the new hotness of a callback"

Reason:
For wmf.18 only. Moved to I392f042d6a1792d24ede7faf92596f13750e3e47

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

Change 454844 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/FlaggedRevs@master] Fix incorrect return value in CurrentRevisionCallback

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

I'm sorry, that's a trivial mistake from me. Instead of returning true (which meant "use the default behavior" in the hook), we should call the callback function implementing the default behavior and return its result. I didn't update this code when copy-pasting it.

I posted a revert of the patchset identified in this ticket. Is this the best route forward for this train, i.e., revert this patch in wmf.18?

That would be fine, or alternatively my patch above. I won't be available today to help you verify the fix in production, so please do whatever you're comfortable with.

I posted a revert of the patchset identified in this ticket. Is this the best route forward for this train, i.e., revert this patch in wmf.18?

That would be fine, or alternatively my patch above. I won't be available today to help you verify the fix in production, so please do whatever you're comfortable with.

If someone else can review that patch for master before the window in an hour or so I'll gladly backport that fix. If not I'll go ahead and revert. Thank you for the patch!

Change 454844 merged by jenkins-bot:
[mediawiki/extensions/FlaggedRevs@master] Fix incorrect return value in CurrentRevisionCallback

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

Change 454868 had a related patch set uploaded (by Thcipriani; owner: Bartosz Dziewoński):
[mediawiki/extensions/FlaggedRevs@wmf/1.32.0-wmf.18] Fix incorrect return value in CurrentRevisionCallback

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

Change 454839 abandoned by Thcipriani:
Revert "Replace old and busted hook with the new hotness of a callback"

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

Change 454868 merged by jenkins-bot:
[mediawiki/extensions/FlaggedRevs@wmf/1.32.0-wmf.18] Fix incorrect return value in CurrentRevisionCallback

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

Mentioned in SAL (#wikimedia-operations) [2018-08-23T18:28:38Z] <thcipriani@deploy1001> Synchronized php-1.32.0-wmf.18/extensions/FlaggedRevs/backend/FlaggedRevs.class.php: [[gerrit:454868|Fix incorrect return value in CurrentRevisionCallback]] T202580 (duration: 00m 54s)

thcipriani closed this task as Resolved.Aug 23 2018, 6:45 PM
thcipriani assigned this task to matmarex.

Seems resolved after rolling wmf.18 to group1 once again.

Thank you for the quick work!

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