Page MenuHomePhabricator
Create Task
Maniphest T354697

TypeError: Argument 6 passed to {closure}() must be an instance of MediaWiki
Closed, ResolvedPublicPRODUCTION ERROR
Actions

Description

On deployment to testwikis we got a 500 error when checking special:Version. This was visible when navigating to the page on the browser.

[12d0e1fc-6580-4a06-9559-df8cc8b839f3] /wiki/Special:Version TypeError: Argument 6 passed to {closure}() must be an instance of MediaWiki, instance of MediaWiki\Actions\ActionEntryPoint given, called in /srv/mediawiki/php-1.42.0-wmf.13/includes/HookContainer/HookContainer.php on line 161

Backtrace:

from /srv/mediawiki/private/PrivateSettings.php(227)
#0 /srv/mediawiki/php-1.42.0-wmf.13/includes/HookContainer/HookContainer.php(161): {closure}(MediaWiki\Title\Title, NULL, MediaWiki\Output\OutputPage, MediaWiki\User\User, MediaWiki\Request\WebRequest, MediaWiki\Actions\ActionEntryPoint)
#1 /srv/mediawiki/php-1.42.0-wmf.13/includes/HookContainer/HookRunner.php(934): MediaWiki\HookContainer\HookContainer->run(string, array)
#2 /srv/mediawiki/php-1.42.0-wmf.13/includes/actions/ActionEntryPoint.php(384): MediaWiki\HookContainer\HookRunner->onBeforeInitialize(MediaWiki\Title\Title, NULL, MediaWiki\Output\OutputPage, MediaWiki\User\User, MediaWiki\Request\WebRequest, MediaWiki\Actions\ActionEntryPoint)
#3 /srv/mediawiki/php-1.42.0-wmf.13/includes/actions/ActionEntryPoint.php(151): MediaWiki\Actions\ActionEntryPoint->performRequest()
#4 /srv/mediawiki/php-1.42.0-wmf.13/includes/MediaWikiEntryPoint.php(169): MediaWiki\Actions\ActionEntryPoint->execute()
#5 /srv/mediawiki/php-1.42.0-wmf.13/index.php(50): MediaWiki\MediaWikiEntryPoint->run()
#6 /srv/mediawiki/w/index.php(3): require(string)
#7 {main}

Related Objects
Search...

StatusSubtypeAssignedTask
 ResolvedReleasejeenaT350089 1.42.0-wmf.13 deployment blockers
 ResolvedPRODUCTION ERRORdanielT354697 TypeError: Argument 6 passed to {closure}() must be an instance of MediaWiki

Event Timeline

thcipriani created this task.Jan 9 2024, 7:20 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJan 9 2024, 7:20 PM
thcipriani removed a project: Security-Team.Jan 9 2024, 7:20 PM
thcipriani added a subscriber: jeena.
jeena added a parent task: T350089: 1.42.0-wmf.13 deployment blockers.Jan 9 2024, 7:21 PM
jeena added subscribers: daniel, tstarling.Jan 9 2024, 7:23 PM
Jdforrester-WMF subscribed.Jan 9 2024, 7:24 PM

We can probably just drop the type hint for this?

jeena triaged this task as Unbreak Now! priority.Jan 9 2024, 7:25 PM
jeena added a project: MediaWiki-Engineering.
jeena updated the task description. (Show Details)
Jdforrester-WMF added a comment.Jan 9 2024, 7:27 PM

(Code is from T336027#8950453.)

daniel added a comment.Jan 9 2024, 7:41 PM
In T354697#9447859, @Jdforrester-WMF wrote:

(Code is from T336027#8950453.)

Yes. This kind of things is the reason that our hook interfaces do not have type hints.

Looking now...

Jdforrester-WMF closed this task as Resolved.Jan 9 2024, 8:50 PM
Jdforrester-WMF assigned this task to daniel.

Looks like this has been Resolved?

daniel added a comment.Jan 10 2024, 10:06 AM
In T354697#9448050, @Jdforrester-WMF wrote:

Looks like this has been Resolved?

Yep. It was fun, editing secret sauce code with vim over ssh :)

Lucas_Werkmeister_WMDE subscribed.Jan 10 2024, 11:23 AM
In T354697#9447910, @daniel wrote:
In T354697#9447859, @Jdforrester-WMF wrote:

(Code is from T336027#8950453.)

Yes. This kind of things is the reason that our hook interfaces do not have type hints.

If this is supposed to be discouraged then maybe Manual:Hooks/BeforeInitialize shouldn’t include the parameter types in the function signature? I assume this is where [committer name redacted] copied them from – the old signature in PrivateSettings exactly matches this old revision of the documentation.

Jdforrester-WMF added a comment.Jan 10 2024, 4:34 PM

Yup; added in https://www.mediawiki.org/w/index.php?diff=6226326&oldid=4726365&title=Manual:Hooks/BeforeInitialize and I've now partially reverted: https://www.mediawiki.org/w/index.php?diff=6302034&oldid=6226326&title=Manual:Hooks/BeforeInitialize

daniel added a comment.Jan 10 2024, 6:53 PM

New hooks that only expose narrow interfaces and value objects in the signature can have type hints.

Type hints should be avoided for any of the old monster classes and anything "internal", since we may want to break it up or deprecate it, and supply a shim to the hook handlers.

Lucas_Werkmeister_WMDE added a comment.Jan 11 2024, 11:21 AM

If we have a list of these “old monster classes” somewhere, it would be great if we could add it e.g. to Manual:Coding conventions/PHP – I’ve seen this explanation a few times on Phabricator now, but I still wouldn’t know a great place to refer someone else to when they use one of those classes in code review or something. (I’m guessing it’s at least Title, Article, WikiPage, User; evidently MediaWiki; OutputPage and WebRequest too?)

daniel added a comment.Jan 11 2024, 11:22 PM
In T354697#9452756, @Lucas_Werkmeister_WMDE wrote:

I’ve seen this explanation a few times on Phabricator now, but I still wouldn’t know a great place to refer someone else to when they use one of those classes in code review or something. (I’m guessing it’s at least Title, Article, WikiPage, User; evidently MediaWiki; OutputPage and WebRequest too?)

These are the "everywhere" monsters. But hooks should also not bind to stuff like EditPage, since it may be split up. Or OutputPage, for the same reason. Basically, the larger the class, the more problematic it is for hooks to bind to it.

Lucas_Werkmeister_WMDE added a comment.Jan 12 2024, 10:03 AM

Alright, added something at https://www.mediawiki.org/w/index.php?title=Manual:Coding_conventions/PHP&diff=prev&oldid=6305196, feel free to edit.

daniel added a comment.Jan 12 2024, 10:57 AM
In T354697#9455688, @Lucas_Werkmeister_WMDE wrote:

Alright, added something at https://www.mediawiki.org/w/index.php?title=Manual:Coding_conventions/PHP&diff=prev&oldid=6305196, feel free to edit.

Thank you. Tweaked it a bit.

Krinkle added projects: Wikimedia-Site-requests, Unstewarded-production-error, Wikimedia-production-error.Jan 14 2025, 10:31 PM
Krinkle changed the visibility from "Custom Policy" to "Public (No Login Required)".
Krinkle changed the edit policy from "Custom Policy" to "All Users".
Restricted Application changed the subtype of this task from "Security Issue" to "Production Error". · View Herald TranscriptJan 14 2025, 10:31 PM
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 · Credits