Page MenuHomePhabricator

Feature MinervaTalkAtTop is not defined during logout
Closed, ResolvedPublic

Description

Error

MediaWiki version: 1.34.0-wmf.23

message
Feature MinervaTalkAtTop is not defined.

Impact

Notes

Details

Request ID
XY0IUApAAEAAAAM6pmAAAABR
Request URL
/wiki/Especial:Salida_del_usuario
Stack Trace
exception.trace
#0 /extensions/MobileFrontend/includes/features/FeaturesManager.php(102): MobileFrontend\Features\FeaturesManager->getFeature(string)
#1 /skins/MinervaNeue/includes/MinervaHooks.php(256): MobileFrontend\Features\FeaturesManager->isFeatureAvailableForCurrentUser(string)
#2 /skins/MinervaNeue/includes/MinervaHooks.php(297): MinervaHooks::setMinervaSkinOptions(MobileContext, SkinMinerva)
#3 /includes/Hooks.php(174): MinervaHooks::onUserLogoutComplete(User, string, string)
#4 /includes/Hooks.php(202): Hooks::callHook(string, array, array, NULL)
#5 /includes/specials/SpecialUserLogout.php(107): Hooks::run(string, array)
#6 /includes/specialpage/FormSpecialPage.php(186): SpecialUserLogout->onSuccess()
#7 /includes/specials/SpecialUserLogout.php(61): FormSpecialPage->execute(NULL)
#8 /includes/specialpage/SpecialPage.php(575): SpecialUserLogout->execute(NULL)
#9 /includes/specialpage/SpecialPageFactory.php(611): SpecialPage->run(NULL)
#10 /includes/MediaWiki.php(296): MediaWiki\Special\SpecialPageFactory->executePath(Title, RequestContext)
#11 /includes/MediaWiki.php(892): MediaWiki->performRequest()
#12 /includes/MediaWiki.php(523): MediaWiki->main()
#13 /index.php(44): MediaWiki->run()
#14 /srv/mediawiki/w/index.php(3): require(string)
#15 {main}
Related Gerrit Patches:
mediawiki/extensions/MobileFrontend : masterRegister all features immediately, not on RequestContextCreateSkin

Event Timeline

mmodell created this task.Sep 26 2019, 7:21 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 26 2019, 7:21 PM
Jdlrobson triaged this task as High priority.Sep 26 2019, 7:41 PM
Jdlrobson added a subscriber: Jdlrobson.

This will occur if the UserLogoutComplete hook runs before RequestContextCreateSkin - not entirely sure when this might occur.

Jdlrobson renamed this task from Feature MinervaTalkAtTop is not defined. to Feature MinervaTalkAtTop is not defined during logout.Sep 26 2019, 7:41 PM
Etonkovidova added a subscriber: Etonkovidova.EditedOct 1 2019, 10:19 PM

The issue is present in testwiki wmf.25 too.

I can consistently reproduce the issue in betalabs (with AMC enabled, not sure if it has some effect).

  1. Log in - enable MinervaNeue.
  2. Click - Log out. You'll be presented with the confirmation screen for Log out. Click to confirm.

The Internal error screen will be present with the exception trace.

Internal error
[XZPMugpAADwAAEHpIMAAAAAA] /wiki/Special:UserLogout RuntimeException from line 90 of /srv/mediawiki/php-1.34.0-wmf.25/extensions/MobileFrontend/includes/features/FeaturesManager.php: Feature MinervaTalkAtTop is not defined.

Backtrace:

#0 /srv/mediawiki/php-1.34.0-wmf.25/extensions/MobileFrontend/includes/features/FeaturesManager.php(102): MobileFrontend\Features\FeaturesManager->getFeature(string)
#1 /srv/mediawiki/php-1.34.0-wmf.25/skins/MinervaNeue/includes/MinervaHooks.php(203): MobileFrontend\Features\FeaturesManager->isFeatureAvailableForCurrentUser(string)
#2 /srv/mediawiki/php-1.34.0-wmf.25/skins/MinervaNeue/includes/MinervaHooks.php(244): MinervaHooks::setMinervaSkinOptions(MobileContext, SkinMinerva)
#3 /srv/mediawiki/php-1.34.0-wmf.25/includes/Hooks.php(174): MinervaHooks::onUserLogoutComplete(User, string, string)
#4 /srv/mediawiki/php-1.34.0-wmf.25/includes/Hooks.php(202): Hooks::callHook(string, array, array, NULL)
#5 /srv/mediawiki/php-1.34.0-wmf.25/includes/specials/SpecialUserLogout.php(107): Hooks::run(string, array)
#6 /srv/mediawiki/php-1.34.0-wmf.25/includes/specialpage/FormSpecialPage.php(186): SpecialUserLogout->onSuccess()
#7 /srv/mediawiki/php-1.34.0-wmf.25/includes/specials/SpecialUserLogout.php(61): FormSpecialPage->execute(NULL)
#8 /srv/mediawiki/php-1.34.0-wmf.25/includes/specialpage/SpecialPage.php(575): SpecialUserLogout->execute(NULL)
#9 /srv/mediawiki/php-1.34.0-wmf.25/includes/specialpage/SpecialPageFactory.php(611): SpecialPage->run(NULL)
#10 /srv/mediawiki/php-1.34.0-wmf.25/includes/MediaWiki.php(296): MediaWiki\Special\SpecialPageFactory->executePath(Title, RequestContext)
#11 /srv/mediawiki/php-1.34.0-wmf.25/includes/MediaWiki.php(900): MediaWiki->performRequest()
#12 /srv/mediawiki/php-1.34.0-wmf.25/includes/MediaWiki.php(527): MediaWiki->main()
#13 /srv/mediawiki/php-1.34.0-wmf.25/index.php(44): MediaWiki->run()
#14 /srv/mediawiki/w/index.php(3): require(string)
#15 {main}

Change 540836 had a related patch set uploaded (by Pmiazga; owner: Pmiazga):
[mediawiki/extensions/MobileFrontend@master] Register all features immediately, not on RequestContextCreateSkin

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

So the problem is:

  • The FeatureManager is a part of MobileFrontend.
  • Feature registration happens when MobileFrontend kicks in (via RequestContextCreateSkin
  • During RequestContextCreateSkin we trigger MobileFrontendFeaturesRegistration hook that allows Minerva to register it's own features
  • because we use Minerva as desktop skin, the mobile mode doesn't kick in, thus code from RequestContextCreateSkin is not called, therefore Minerva doesn't register it's features

This issue happens only for desktop users who use Minerva as their default desktop skin. The solution is simple, register all features on first FeatureManager service access.
The proper fix is to move the FeatureManager from MobileFrontend to somwhere else (core/BetaFeatures ?) so it's coupled with MobileFrontend.

Thanks for investigating.

because we use Minerva as desktop skin, the mobile mode doesn't kick in, thus code from RequestContextCreateSkin is not called, therefore Minerva doesn't register it's features

That doesn't sound right to me. The RequestContextCreateSkin hook is called for all skins. We use RequestContextCreateSkinMobile in Minerva which is only run in the mobile domain.

Given the contents of that hook, I think we just need to turn onRequestContextCreateSkinMobile into onRequestContextCreateSkin and create an if statement wrapping it to check MobileFrontend extension is installed.

Does that make sense?

The problem is not Minerva, the problem is how/when MobileFrontend initializes FeatureManager. Just changing hook from RequestContextCreateSkinMobile to RequestContextCreateSkin won't help. The problem will be the same - The FeatureManager won't be initialized (won't send the hook).

So now the FeatureManager listens to RequestContextCreateSkin and when that happens it initializes FeatureManager (calls the hook to register features). If MobileFrontend doesn't come into play in that single request, then the FeatureManager won't be initialized (this line won't be called: https://github.com/wikimedia/mediawiki-extensions-MobileFrontend/blob/master/includes/MobileFrontendHooks.php#L97)

So now, the FeatureManager wasn't initialized (because it's desktop skin). During MinervaHooks::onLogoutComplete we reset the SkinOptions assuming that FeatureManager is registered. That's why it fails.
There are 3 solutions to this problem:

  • the biggest change that at the end makes code simpler: -> always initialize FeatureManager, once code asks for FeatureManager service, call the hook and collect all services
  • the easy fix, move $services->getService( 'MobileFrontend.FeaturesManager' )->setup(); line from MobileFrontendHooks before early exit, old behavior, just patched
  • the hack - in MinervaHooks::onLogoutComplete method do not reset skin options if it's desktop, Minerva.

I went with the first approach to fix it properly. The second fix is nice and easy, should work, but we might encounter similar feature sometime in the future. It's better to protect future us from today's lazy us.

Another thing that is necessary to fix is to make MinervaHooks::setMinervaSkinOptions immune to the situation when MobileFrontend is not present (thus MobileFrontend.FeatureManager is not present).

Change 540836 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Register all features immediately, not on RequestContextCreateSkin

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

Jdlrobson claimed this task.Oct 9 2019, 5:04 PM
Jdlrobson closed this task as Resolved.Oct 12 2019, 12:41 AM
Restricted Application added a subscriber: Masumrezarock100. · View Herald TranscriptOct 12 2019, 12:41 AM