HistoryAction/FeedUtils triggers deprecated "Unexpected clearActionName after getActionName"
Closed, ResolvedPublicPRODUCTION ERROR
Actions

Description

Error

mwversion: 1.40.0-wmf.10
reqId: fc5efdac-e88c-4201-97d4-1f7c020f0368
Find reqId in Logstash

normalized_message

[{reqId}] {exception_url}   PHP Notice: Unexpected clearActionName after getActionName already called

exception.trace

from /srv/mediawiki/php-1.40.0-wmf.10/includes/context/RequestContext.php(311)
#0 [internal function]: MWExceptionHandler::handleError(integer, string, string, integer, array)
#1 /srv/mediawiki/php-1.40.0-wmf.10/includes/context/RequestContext.php(311): trigger_error(string)
#2 /srv/mediawiki/php-1.40.0-wmf.10/includes/context/RequestContext.php(176): RequestContext->clearActionName()
#3 /srv/mediawiki/php-1.40.0-wmf.10/includes/Feed/FeedUtils.php(187): RequestContext->setTitle(Title)
#4 /srv/mediawiki/php-1.40.0-wmf.10/includes/actions/HistoryAction.php(447): MediaWiki\Feed\FeedUtils::formatDiffRow2(Title, integer, integer, string, string)
#5 /srv/mediawiki/php-1.40.0-wmf.10/includes/actions/HistoryAction.php(413): HistoryAction->feedItem(stdClass, string)
#6 /srv/mediawiki/php-1.40.0-wmf.10/includes/actions/HistoryAction.php(200): HistoryAction->feed(string)
#7 /srv/mediawiki/php-1.40.0-wmf.10/includes/actions/FormlessAction.php(48): HistoryAction->onView()
#8 /srv/mediawiki/php-1.40.0-wmf.10/includes/MediaWiki.php(539): FormlessAction->show()
#9 /srv/mediawiki/php-1.40.0-wmf.10/includes/MediaWiki.php(317): MediaWiki->performAction(Article, Title)
#10 /srv/mediawiki/php-1.40.0-wmf.10/includes/MediaWiki.php(901): MediaWiki->performRequest()
#11 /srv/mediawiki/php-1.40.0-wmf.10/includes/MediaWiki.php(559): MediaWiki->main()
#12 /srv/mediawiki/php-1.40.0-wmf.10/index.php(50): MediaWiki->run()
#13 /srv/mediawiki/php-1.40.0-wmf.10/index.php(46): wfIndexMain()
#14 /srv/mediawiki/w/index.php(3): require(string)
#15 {main}

Impact

~70 of these since deploying T320515: 1.40.0-wmf.10 deployment blockers to group0 a few minutes ago.

Notes

I'm guessing this is T314008: Promote dirty RequestContext::clearActionName to runtime warning, although most of the errors I'm seeing are from feeds specifically.

Tentatively treating as a blocker but not yet rolling back from group0, since it doesn't seem like the feeds themselves are broken.

Details

Request URL: https://simple.wikiquote.org/w/index.php?action=history&feed=*&title=*

	Subject	Repo	Branch	Lines +/-
	Feed: Use DerivativeContext and not clone main RequestContext	mediawiki/core	wmf/1.40.0-wmf.10	+6 -6
	Feed: Use DerivativeContext and not clone main RequestContext	mediawiki/core	master	+6 -6

Customize query in gerrit

Related Objects
Search...

		Status	Subtype	Assigned	Task
		Resolved	Release	brennen	T320515 1.40.0-wmf.10 deployment blockers
		Resolved	PRODUCTION ERROR	Umherirrender	T323153 HistoryAction/FeedUtils triggers deprecated "Unexpected clearActionName after getActionName"

Event Timeline

brennen triaged this task as Unbreak Now! priority.Nov 15 2022, 7:24 PM

brennen created this task.

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptNov 15 2022, 7:24 PM

brennen updated the task description. (Show Details)Nov 15 2022, 7:25 PM

brennen added a parent task: T320515: 1.40.0-wmf.10 deployment blockers.

Jdforrester-WMF subscribed.Nov 15 2022, 7:31 PM

Zabe subscribed.Nov 15 2022, 8:00 PM

The first call is from MediaWiki\Extension\InputBox\InputBoxHooks->onMediaWikiPerformAction, the FeedUtil is cloning the main request context and that clones also the action field. Just using a DerivativeContext seems better.

https://gerrit.wikimedia.org/r/c/mediawiki/extensions/InputBox/+/831153 is not a issue, but also included in this train, it fails before also with the patch from T314008

Change 857041 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[mediawiki/core@master] Feed: Use DerivativeContext and not clone main RequestContext

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

gerritbot added a project: Patch-For-Review.Nov 15 2022, 8:14 PM

Thanks @Umherirrender!

Perhaps we should forbid cloning of RequestContext to avoid similar issues in the future.

Krinkle removed a project: MediaWiki-General.Nov 15 2022, 10:18 PM

Change 856582 had a related patch set uploaded (by Tim Starling; author: Umherirrender):

[mediawiki/core@wmf/1.40.0-wmf.10] Feed: Use DerivativeContext and not clone main RequestContext

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

Change 857041 merged by jenkins-bot:

[mediawiki/core@master] Feed: Use DerivativeContext and not clone main RequestContext

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

ReleaseTaggerBot added a project: MW-1.40-notes (1.40.0-wmf.12; 2022-11-28).Nov 15 2022, 11:00 PM

In T323153#8397988, @Krinkle wrote:

Perhaps we should forbid cloning of RequestContext to avoid similar issues in the future.

There is no other code like that

It seems only a bad idea if the main get cloned, but the logging would also trigger for non-main RequestContext, but it is unlikly the action is reset there by a setter. Not sure if an exception on clone would help or better to make all RequestContext objects non-clonable.

Change 856582 merged by jenkins-bot:

[mediawiki/core@wmf/1.40.0-wmf.10] Feed: Use DerivativeContext and not clone main RequestContext

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

Mentioned in SAL (#wikimedia-operations) [2022-11-15T23:19:24Z] <brennen@deploy1002> Started scap: Backport for [[gerrit:856582|Feed: Use DerivativeContext and not clone main RequestContext (T323153)]]

Mentioned in SAL (#wikimedia-operations) [2022-11-15T23:19:50Z] <brennen@deploy1002> brennen and tstarling: Backport for [[gerrit:856582|Feed: Use DerivativeContext and not clone main RequestContext (T323153)]] synced to the testservers: mwdebug1002.eqiad.wmnet, mwdebug2002.codfw.wmnet, mwdebug2001.codfw.wmnet, mwdebug1001.eqiad.wmnet

Mentioned in SAL (#wikimedia-operations) [2022-11-15T23:25:53Z] <brennen@deploy1002> Finished scap: Backport for [[gerrit:856582|Feed: Use DerivativeContext and not clone main RequestContext (T323153)]] (duration: 06m 26s)

Maintenance_bot removed a project: Patch-For-Review.Nov 15 2022, 11:30 PM

Agusbou2015 subscribed.Nov 15 2022, 11:49 PM

ReleaseTaggerBot edited projects, added MW-1.40-notes (1.40.0-wmf.10; 2022-11-14); removed MW-1.40-notes (1.40.0-wmf.12; 2022-11-28).Nov 16 2022, 12:00 AM

brennen mentioned this in T323184: SpecialPage transclusion triggers deprecated "Unexpected clearActionName after getActionName".Nov 16 2022, 12:11 AM

brennen mentioned this in T314008: Promote dirty RequestContext::clearActionName to runtime warning.

brennen closed this task as Resolved.Nov 16 2022, 12:15 AM

In T323153#8398142, @Umherirrender wrote:

In T323153#8397988, @Krinkle wrote:

Perhaps we should forbid cloning of RequestContext to avoid similar issues in the future.

There is no other code like that

https://codesearch.wmcloud.org/search/?q=clone%5Cs%2B%5BA-Za-z0-9%5D%2BContext%5Cb&i=nope&files=&excludeFiles=&repos=

https://codesearch.wmcloud.org/search/?q=clone%5Cs%2B%5C%24%5Ba-zA-Z0-9%5D*%5BCc%5D(ontext%7Ctx)&i=nope&files=&excludeFiles=&repos=

It seems only a bad idea if the main get cloned, but the logging would also trigger for non-main RequestContext, but it is unlikly the action is reset there by a setter. Not sure if an exception on clone would help or better to make all RequestContext objects non-clonable.

Agreed, the above has close to no results. 1 match in a third-party repo (femwiki), and 2 matches in a unit test. RequestContext is effectively always the main one given that all its methods default to global state. We have DerivedContext for safely creating overriddes whilest continuing to re-use the same references from the parent context for non-overridden fields. I would support throwing fatal from __clone() with a message that DerivedContext should be used instead. That would also help new developers that might try this. While there is virtually no code that does this today, it seems likely to regress unless regularly reminded through code review, which is one more thing we can automate and ensure is discovered and passed on without relying on specific people to spread the word.

Krinkle mentioned this in T323187: Disallow cloning of RequestContext objects.Nov 16 2022, 1:44 AM

Krinkle renamed this task from PHP Notice: Unexpected clearActionName after getActionName already called to HistoryAction/FeedUtils triggers deprecated "Unexpected clearActionName after getActionName".Nov 17 2022, 9:10 AM

Krinkle moved this task from Untriaged to Nov 2022 on the Wikimedia-production-error board.Nov 17 2022, 4:17 PM

dancy mentioned this in T323338: Resolve revisiondelete/editchangetags debt around fake action/specialpage handling.Jan 30 2023, 9:24 PM

HistoryAction/FeedUtils triggers deprecated "Unexpected clearActionName after getActionName"Closed, ResolvedPublicPRODUCTION ERRORActions