Page MenuHomePhabricator
Create Task
Maniphest T263034

PHP Notice: Trying to get property 'textContent' of non-object
Closed, ResolvedPublic
Actions

Description

In the production logs today on https://wikivoyage.org/wiki/Gran_Canaria

[{exception_id}] {exception_url} ErrorException from line 257 of /srv/mediawiki/php-1.36.0-wmf.9/extensions/MobileFrontend/includes/Transforms/MoveLeadParagraphTransform.php: PHP Notice: Trying to get property 'textContent' of non-object

#0 /srv/mediawiki/php-1.36.0-wmf.9/extensions/MobileFrontend/includes/Transforms/MoveLeadParagraphTransform.php(257): MWExceptionHandler::handleError(integer, string, string, integer, array)
#1 /srv/mediawiki/php-1.36.0-wmf.9/extensions/MobileFrontend/includes/Transforms/MoveLeadParagraphTransform.php(134): MobileFrontend\Transforms\MoveLeadParagraphTransform->isNonLeadParagraph(DOMXPath, DOMElement)
#2 /srv/mediawiki/php-1.36.0-wmf.9/extensions/MobileFrontend/includes/Transforms/MoveLeadParagraphTransform.php(168): MobileFrontend\Transforms\MoveLeadParagraphTransform->identifyLeadParagraph(DOMXPath, DOMElement)
#3 /srv/mediawiki/php-1.36.0-wmf.9/extensions/MobileFrontend/includes/Transforms/MoveLeadParagraphTransform.php(37): MobileFrontend\Transforms\MoveLeadParagraphTransform->moveFirstParagraphBeforeInfobox(DOMElement, DOMDocument)
#4 /srv/mediawiki/php-1.36.0-wmf.9/extensions/MobileFrontend/includes/Transforms/MakeSectionsTransform.php(146): MobileFrontend\Transforms\MoveLeadParagraphTransform->apply(DOMElement)
#5 /srv/mediawiki/php-1.36.0-wmf.9/extensions/MobileFrontend/includes/Transforms/MakeSectionsTransform.php(260): MobileFrontend\Transforms\MakeSectionsTransform->makeSections(DOMElement, array)
#6 /srv/mediawiki/php-1.36.0-wmf.9/extensions/MobileFrontend/includes/MobileFormatter.php(158): MobileFrontend\Transforms\MakeSectionsTransform->apply(DOMElement)
#7 /srv/mediawiki/php-1.36.0-wmf.9/extensions/MobileFrontend/includes/ExtMobileFrontend.php(103): MobileFormatter->applyTransforms(boolean, boolean)
#8 /srv/mediawiki/php-1.36.0-wmf.9/extensions/MobileFrontend/includes/MobileFrontendHooks.php(286): ExtMobileFrontend::domParse(OutputPage, string, boolean)
#9 /srv/mediawiki/php-1.36.0-wmf.9/includes/HookContainer/HookContainer.php(330): MobileFrontendHooks::onOutputPageBeforeHTML(OutputPage, string)
#10 /srv/mediawiki/php-1.36.0-wmf.9/includes/HookContainer/HookContainer.php(137): MediaWiki\HookContainer\HookContainer->callLegacyHook(string, array, array, array)
#11 /srv/mediawiki/php-1.36.0-wmf.9/includes/HookContainer/HookRunner.php(2730): MediaWiki\HookContainer\HookContainer->run(string, array)
#12 /srv/mediawiki/php-1.36.0-wmf.9/includes/OutputPage.php(2005): MediaWiki\HookContainer\HookRunner->onOutputPageBeforeHTML(OutputPage, string)
#13 /srv/mediawiki/php-1.36.0-wmf.9/includes/OutputPage.php(2017): OutputPage->addParserOutputText(ParserOutput, array)
#14 /srv/mediawiki/php-1.36.0-wmf.9/includes/page/Article.php(742): OutputPage->addParserOutput(ParserOutput, array)
#15 /srv/mediawiki/php-1.36.0-wmf.9/includes/actions/ViewAction.php(74): Article->view()
#16 /srv/mediawiki/php-1.36.0-wmf.9/includes/MediaWiki.php(527): ViewAction->show()
#17 /srv/mediawiki/php-1.36.0-wmf.9/includes/MediaWiki.php(313): MediaWiki->performAction(Article, Title)
#18 /srv/mediawiki/php-1.36.0-wmf.9/includes/MediaWiki.php(940): MediaWiki->performRequest()
#19 /srv/mediawiki/php-1.36.0-wmf.9/includes/MediaWiki.php(543): MediaWiki->main()
#20 /srv/mediawiki/php-1.36.0-wmf.9/index.php(53): MediaWiki->run()
#21 /srv/mediawiki/php-1.36.0-wmf.9/index.php(46): wfIndexMain()
#22 /srv/medi

Details

SubjectRepoBranchLines +/-
Add test case for template styles inside paragraph with contentmediawiki/extensions/MobileFrontendmaster+6 -0
Check $coords matched some nodes before comparing contentsmediawiki/extensions/MobileFrontendmaster+1 -1
Check $coords matched some nodes before comparing contentsmediawiki/extensions/MobileFrontendwmf/1.36.0-wmf.9+1 -1
Customize query in gerrit

Event Timeline

Jdlrobson created this task.Sep 16 2020, 3:16 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 16 2020, 3:16 PM
brennen triaged this task as Unbreak Now! priority.EditedSep 16 2020, 3:33 PM
brennen added a project: User-brennen.
brennen added subscribers: • LarsWirzenius, brennen.

This looks to me like a probable train blocker; ~35 or so since rollout of wmf.9 and still happening at a low level.

Screenshot-2020-09-16-09:33:09.png (168×1 px, 9 KB)

cc: @LarsWirzenius.

brennen added a parent task: T257977: 1.36.0-wmf.9 deployment blockers.Sep 16 2020, 3:34 PM
brennen added a comment.Sep 16 2020, 4:03 PM

Seems reproducible at these URLs, at least with a mobile client. (I'm fuzzy on how MobileFrontend actually works.) I don't see obvious user-facing breakage, but I may not know what to look for.

Ammarpad added a subscriber: Ammarpad.EditedSep 16 2020, 4:07 PM

DOMNodeList::item returns DOMNode object or null on failure The codepath itself is fragile per the comment there (working on assumption)

if ( $nodeContent ) {
 // assume valid HTML and only one #coordinates element
 // this may not behave correctly if garbage in.
 $coordEl = $coords->item( 0 );
 // Is there content of this node in addition to the coordinates ?
 return $nodeContent === trim( $coordEl->textContent );
}
Esanders added a comment.Sep 16 2020, 4:16 PM

Investigating...

gerritbot added a comment.Sep 16 2020, 4:17 PM

Change 627877 had a related patch set uploaded (by Esanders; owner: Esanders):
[mediawiki/extensions/MobileFrontend@master] Check $coords matched some nodes before comparing contents

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

gerritbot added a project: Patch-For-Review.Sep 16 2020, 4:17 PM
Esanders updated the task description. (Show Details)Sep 16 2020, 4:18 PM

I verified the above fix locally by pointing my content API at https://wikivoyage.org/wiki/Gran_Canaria. I will write a test case later.

Jdlrobson claimed this task.Sep 16 2020, 4:27 PM

I can review and test

gerritbot added a comment.Sep 16 2020, 4:35 PM

Change 627793 had a related patch set uploaded (by Jdlrobson; owner: Esanders):
[mediawiki/extensions/MobileFrontend@wmf/1.36.0-wmf.9] Check $coords matched some nodes before comparing contents

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

Jdlrobson added a comment.Sep 16 2020, 4:36 PM

Thanks @Esanders for the testing instructions that made review super easy.
Thanks @Ammarpad for the detective work!

Over to you @brennen > https://gerrit.wikimedia.org/r/c/mediawiki/extensions/MobileFrontend/+/627793

Jdlrobson reassigned this task from Jdlrobson to brennen.Sep 16 2020, 4:36 PM
gerritbot added a comment.Sep 16 2020, 4:37 PM

Change 627881 had a related patch set uploaded (by Esanders; owner: Esanders):
[mediawiki/extensions/MobileFrontend@master] Add test case for template styles inside paragraph with content

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

Esanders added a comment.Sep 16 2020, 4:38 PM

Test case as promised ^. Looks like I didn't account for other content in the <p> wrapping a TemplateStyles <style> tag. Presumably this is an inline template using TemplateStyles.

Jdlrobson moved this task from Needs Analysis to Ready for Signoff on the Web-Team-Backlog (Kanbanana-FY-2020-21) board.Sep 16 2020, 5:04 PM
gerritbot added a comment.Sep 16 2020, 5:40 PM

Change 627793 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@wmf/1.36.0-wmf.9] Check $coords matched some nodes before comparing contents

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

Stashbot added a comment.Sep 16 2020, 6:00 PM

Mentioned in SAL (#wikimedia-operations) [2020-09-16T18:00:09Z] <brennen@deploy1001> Synchronized php-1.36.0-wmf.9/extensions/MobileFrontend: Backport: [[gerrit:627793|Check $coords matched some nodes before comparing contents (T263034)]] (duration: 01m 06s)

ReleaseTaggerBot added a project: MW-1.36-notes (1.36.0-wmf.9; 2020-09-15).Sep 16 2020, 6:04 PM
• holger.knust moved this task from Untriaged to Sep 2020 on the Wikimedia-production-error board.Sep 16 2020, 6:46 PM
brennen removed a parent task: T257977: 1.36.0-wmf.9 deployment blockers.Sep 16 2020, 6:54 PM

Fixed from wmf.9 train perspective.

brennen removed brennen as the assignee of this task.Sep 16 2020, 6:55 PM
brennen lowered the priority of this task from Unbreak Now! to Medium.
brennen moved this task from Backlog to Logs/Train on the User-brennen board.
gerritbot added a comment.Sep 16 2020, 11:40 PM

Change 627877 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Check $coords matched some nodes before comparing contents

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

gerritbot added a comment.Sep 16 2020, 11:40 PM

Change 627881 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Add test case for template styles inside paragraph with content

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

ReleaseTaggerBot edited projects, added MW-1.36-notes (1.36.0-wmf.10; 2020-09-22); removed MW-1.36-notes (1.36.0-wmf.9; 2020-09-15).Sep 17 2020, 12:00 AM
Maintenance_bot removed a project: Patch-For-Review.Sep 17 2020, 12:10 AM
• Niedzielski assigned this task to Jdlrobson.Sep 17 2020, 5:04 PM
Jdlrobson closed this task as Resolved.Sep 17 2020, 6:13 PM

No errors in last 12 hrs.

Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL