PHP Deprecated: Use of OutputPageParserOutput hook to mutate TOC was deprecated in MediaWiki 1.41
Closed, ResolvedPublicPRODUCTION ERROR
Actions

Description

Error

service.version: 1.41.0-wmf.29
trace.id: dedd6496-39b0-4f09-8bc6-f1bb1ed12fff
Find trace.id in Logstash

labels.normalized_message

[{reqId}] {exception_url}   PHP Deprecated: Use of OutputPageParserOutput hook to mutate TOC was deprecated in MediaWiki 1.41. [Called from MediaWiki\Output\OutputPage::addParserOutput]

error.stack_trace

from /srv/mediawiki/php-1.41.0-wmf.29/includes/Output/OutputPage.php(2348)
#0 [internal function]: MWExceptionHandler::handleError(integer, string, string, string, array)
#1 /srv/mediawiki/php-1.41.0-wmf.29/includes/debug/MWDebug.php(385): trigger_error(string, integer)
#2 /srv/mediawiki/php-1.41.0-wmf.29/includes/debug/MWDebug.php(356): MWDebug::sendRawDeprecated(string, boolean, string)
#3 /srv/mediawiki/php-1.41.0-wmf.29/includes/debug/MWDebug.php(237): MWDebug::deprecatedMsg(string, string, string, integer)
#4 /srv/mediawiki/php-1.41.0-wmf.29/includes/GlobalFunctions.php(775): MWDebug::deprecated(string, string, string, integer)
#5 /srv/mediawiki/php-1.41.0-wmf.29/includes/Output/OutputPage.php(2289): wfDeprecated(string, string)
#6 /srv/mediawiki/php-1.41.0-wmf.29/includes/Output/OutputPage.php(2348): MediaWiki\Output\OutputPage->addParserOutputMetadata(ParserOutput)
#7 /srv/mediawiki/php-1.41.0-wmf.29/includes/page/Article.php(831): MediaWiki\Output\OutputPage->addParserOutput(ParserOutput, array)
#8 /srv/mediawiki/php-1.41.0-wmf.29/includes/page/Article.php(654): Article->doOutputFromParserCache(ParserOutput, MediaWiki\Output\OutputPage, array)
#9 /srv/mediawiki/php-1.41.0-wmf.29/includes/page/Article.php(550): Article->generateContentOutput(MediaWiki\User\User, ParserOptions, integer, MediaWiki\Output\OutputPage, array)
#10 /srv/mediawiki/php-1.41.0-wmf.29/includes/page/ImagePage.php(156): Article->view()
#11 /srv/mediawiki/php-1.41.0-wmf.29/includes/actions/ViewAction.php(78): ImagePage->view()
#12 /srv/mediawiki/php-1.41.0-wmf.29/includes/MediaWiki.php(584): ViewAction->show()
#13 /srv/mediawiki/php-1.41.0-wmf.29/includes/MediaWiki.php(363): MediaWiki->performAction(ImagePage, MediaWiki\Title\Title)
#14 /srv/mediawiki/php-1.41.0-wmf.29/includes/MediaWiki.php(961): MediaWiki->performRequest()
#15 /srv/mediawiki/php-1.41.0-wmf.29/includes/MediaWiki.php(614): MediaWiki->main()
#16 /srv/mediawiki/php-1.41.0-wmf.29/index.php(50): MediaWiki->run()
#17 /srv/mediawiki/php-1.41.0-wmf.29/index.php(46): wfIndexMain()
#18 /srv/mediawiki/w/index.php(3): require(string)
#19 {main}

Impact

Notes

A lot of this impacting mediawikiwiki

Details

Request URL: https://wikimania2014.wikimedia.org/wiki/File:WikiProject_Video_Games_Leaflet.png

Subject	Repo	Branch	Lines +/-
ParserOutput::hasTOCHTML(): Remove old back-compat code	mediawiki/core	master	+4 -25
Remove implicit setter for ParserOutput::mTOCHTML	mediawiki/core	master	+1 -11
Revert "Deprecate TOC mutation in OutputPageParserOutput hook"	mediawiki/core	wmf/1.41.0-wmf.29	+1 -5
Revert "Deprecate TOC mutation in OutputPageParserOutput hook"	mediawiki/core	master	+1 -5

Customize query in gerrit

Related Objects

Mentioned Here: T293513: Deprecate and remove ParserOutput::setTOCHTML()

Event Timeline

DAlangi_WMF created this task.Oct 4 2023, 12:22 PM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 4 2023, 12:22 PM

DAlangi_WMF added a project: Content-Transform-Team.Oct 4 2023, 3:08 PM

New deprecation message from https://gerrit.wikimedia.org/r/c/mediawiki/core/+/946987 / T293513

ReleaseNotes

* Calling ParserOutput::setTOCHTML() inside the OutputPageParserOutput hook
  as a means to toggle display of the TOC is deprecated.  Setting the
  ParserOutputFlags::NO_TOC flag is a supported alternative.

Usage of the hook: https://codesearch.wmcloud.org/deployed/?q=OutputPageParserOutput

There is no usage of setTOCHTML -https://codesearch.wmcloud.org/deployed/?q=%5CbsetTOCHTML%5Cb&files=&excludeFiles=&repos=

thcipriani added a parent task: T347080: 1.41.0-wmf.29 deployment blockers.Oct 4 2023, 6:11 PM

Also coming from a slightly different path:

from /srv/mediawiki/php-1.41.0-wmf.29/includes/api/ApiParse.php(505)
#0 [internal function]: MWExceptionHandler::handleError(integer, string, string, string, array)
#1 /srv/mediawiki/php-1.41.0-wmf.29/includes/debug/MWDebug.php(385): trigger_error(string, integer)
#2 /srv/mediawiki/php-1.41.0-wmf.29/includes/debug/MWDebug.php(356): MWDebug::sendRawDeprecated(string, boolean, string)
#3 /srv/mediawiki/php-1.41.0-wmf.29/includes/debug/MWDebug.php(237): MWDebug::deprecatedMsg(string, string, string, integer)
#4 /srv/mediawiki/php-1.41.0-wmf.29/includes/GlobalFunctions.php(775): MWDebug::deprecated(string, string, string, integer)
#5 /srv/mediawiki/php-1.41.0-wmf.29/includes/Output/OutputPage.php(2289): wfDeprecated(string, string)
#6 /srv/mediawiki/php-1.41.0-wmf.29/includes/api/ApiParse.php(505): MediaWiki\Output\OutputPage->addParserOutputMetadata(ParserOutput)
#7 /srv/mediawiki/php-1.41.0-wmf.29/includes/api/ApiMain.php(1931): ApiParse->execute()
#8 /srv/mediawiki/php-1.41.0-wmf.29/includes/api/ApiMain.php(908): ApiMain->executeAction()
#9 /srv/mediawiki/php-1.41.0-wmf.29/includes/api/ApiMain.php(879): ApiMain->executeActionWithErrorHandling()
#10 /srv/mediawiki/php-1.41.0-wmf.29/api.php(95): ApiMain->execute()
#11 /srv/mediawiki/php-1.41.0-wmf.29/api.php(48): wfApiMain()
#12 /srv/mediawiki/w/api.php(3): require(string)
#13 {main}

Seeing a massive number of these on commonswiki when we rolled forward 1.41.0-wmf.29, adding as trainblocker, setting as UBN.

Working on this. The suspicion I have is that there's a:

$this->mTOCHTML = $toc;

inside ParserOutput::getText() if the allowTOC and injectTOC options are set. I wonder if it's not a direct call to setTOCHTML but instead an early call to ParserOutput::getText() which is triggering this?

A related idea: since $mTOCHTML is being set implicitly by ::getText, perhaps it is then being serialized and stored to the ParserCache in that state. Then when you deserialize it from ParserCache you have an object with $mTOCHTML set...

On the other hand, I can't seem to reproduce this locally, which may mean that this is indeed related to some extension code executing on a hook - but the extension itself is not showing up in the backtraces.

Change 963397 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/core@master] Remove implicit setter for ParserOutput::mTOCHTML

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

gerritbot added a project: Patch-For-Review.Oct 4 2023, 6:51 PM

brennen subscribed.Oct 4 2023, 6:54 PM

Novem_Linguae subscribed.Oct 4 2023, 7:01 PM

^ Not certain about the above patch since I can't reproduce this locally yet.

Change 963350 had a related patch set uploaded (by Subramanya Sastry; author: Subramanya Sastry):

[mediawiki/core@master] Revert "Deprecate TOC mutation in OutputPageParserOutput hook"

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

Ok, @ssastry and I both bashed our heads against this for a while without making progress. "Something" is setting $mTOCHTML, and apparently it is doing this without calling ParserOutput::setTOCHTML(), which would have triggered a deprecation warning of its own.

The result of the deprecated code path is that $mEnableTOC is forced to true, where it was previously false. But the traces which aren't API pages are media pages like https://wikimania2014.wikimedia.org/wiki/File:WikiProject_Video_Games_Leaflet.png which shouldn't have a TOC anyway. So eliminating the deprecated path and letting $mEnableTOC stay false should be fine.

Our plan is to revert this for now to avoid blocking the train this week and the 1.41 branch in general. Post-1.41 we'll first merge https://gerrit.wikimedia.org/r/963397 and if that goes through w/o issue we'll re-roll this patch (https://gerrit.wikimedia.org/r/c/mediawiki/core/+/946987) and see if the hypothesis above (T348134#9226194) is correct. We believe we can probably just remove the deprecated pathway without going through hard deprecation if we need to.

Change 963350 merged by jenkins-bot:

[mediawiki/core@master] Revert "Deprecate TOC mutation in OutputPageParserOutput hook"

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

Change 963351 had a related patch set uploaded (by Subramanya Sastry; author: Subramanya Sastry):

[mediawiki/core@wmf/1.41.0-wmf.29] Revert "Deprecate TOC mutation in OutputPageParserOutput hook"

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

@thcipriani @brennen ^ I cherry-picked it to the deployment branch ... so, feel free to merge and roll it out whenever you all are ready.

Thanks @ssastry - rolling that out.

Change 963351 merged by jenkins-bot:

[mediawiki/core@wmf/1.41.0-wmf.29] Revert "Deprecate TOC mutation in OutputPageParserOutput hook"

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

Mentioned in SAL (#wikimedia-operations) [2023-10-04T21:53:14Z] <brennen@deploy2002> Started scap: Backport for [[gerrit:963351|Revert "Deprecate TOC mutation in OutputPageParserOutput hook" (T348134)]]

Mentioned in SAL (#wikimedia-operations) [2023-10-04T21:54:39Z] <brennen@deploy2002> brennen and ssastry: Backport for [[gerrit:963351|Revert "Deprecate TOC mutation in OutputPageParserOutput hook" (T348134)]] synced to the testservers (https://wikitech.wikimedia.org/wiki/Mwdebug)

ReleaseTaggerBot added a project: MW-1.41-notes (1.41.0-wmf.29; 2023-10-03).Oct 4 2023, 10:00 PM

Mentioned in SAL (#wikimedia-operations) [2023-10-04T22:02:28Z] <brennen@deploy2002> Finished scap: Backport for [[gerrit:963351|Revert "Deprecate TOC mutation in OutputPageParserOutput hook" (T348134)]] (duration: 09m 13s)

No warnings since backport deployed; removing as train blocker. Thanks for the help!

In T348134#9226194, @cscott wrote:

Working on this. The suspicion I have is […], perhaps it is then being serialized and stored to the ParserCache in that state. […]

On the other hand, I can't seem to reproduce this locally, which may mean that this is indeed related to some extension code executing on a hook - but the extension itself is not showing up in the backtraces.

I don't know the history here, but if this kind of hook and extension used to be in production and if you only recently fixed or otherwise migrated that code, then a more likely scenario would be that an extension hook did cause this, get cached normally in ParserCache, and we're now getting the second half of it (the runtime detection in getText) from a previously-normally-generated-and-ached ParserOutput object.

If you know we didn't have such hook in prod and/or if it was affecting prod on cache misses or otherwise newer cache entries as well, then my theory would be wrong.

The relevant hooks in core were changed in the 1.40 timeframe I believe -- much longer than a parser cache expiration time ago. This was supposed to be just a cleanup patch to notify third parties which (unlike WMF) still might have had some of those hooks lying around. But third parties /should/ already be getting hard-deprecation warnings from ::setTOCHTML(). This thing is (false?) triggering on something else, which is what we were banging our heads against. (My theory was a stray pre-cache call to ::getText() which was setting the tochtml as a side effect even though the client didn't specifically want/need it to -- if they "needed it to" they would have been calling ::getTOCHTML() afterward which is also hard-deprecated and would have triggered a warning.)

Umherirrender unsubscribed.Oct 5 2023, 6:22 PM

Change 963397 merged by jenkins-bot:

[mediawiki/core@master] Remove implicit setter for ParserOutput::mTOCHTML

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

ReleaseTaggerBot edited projects, added MW-1.41-notes (1.41.0-wmf.30; 2023-10-10); removed MW-1.41-notes (1.41.0-wmf.29; 2023-10-03).Oct 9 2023, 5:00 PM

Maintenance_bot removed a project: Patch-For-Review.Oct 9 2023, 5:10 PM

MSantos assigned this task to cscott.Oct 12 2023, 2:19 PM

MSantos edited projects, added Content-Transform-Team-WIP, Essential-Work; removed Content-Transform-Team.

Change 965524 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/core@master] ParserOutput::hasTOCHTML(): Remove old back-compat code

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

gerritbot added a project: Patch-For-Review.Oct 12 2023, 2:28 PM