Page MenuHomePhabricator

MediaWiki internal error. RuntimeException ...MobileFrontend.hooks.php: wgMFDefaultSkinClass ...
Closed, InvalidPublic

Description

I did my monthly update.
My users say my sites are broken.
Indeed for mobile, they are all broken.

http://radioscanningtw.jidanni.org/index.php?title=%E9%A6%96%E9%A0%81&mobileaction=toggle_view_mobile

MediaWiki internal error.

Original exception: [WYcX-UBaKU4AAANnjIcAAAAF] /index.php?title=%E9%A6%96%E9%A0%81 RuntimeException from line 57 of /home/jidanni/mediawiki/extensions/MobileFrontend/includes/MobileFrontend.hooks.php: wgMFDefaultSkinClass is not setup correctly. It should point to the class name of a valid skin e.g. SkinMinerva, SkinVector
Backtrace:
#0 /home/jidanni/mediawiki/extensions/MobileFrontend/includes/MobileFrontend.hooks.php(126): MobileFrontendHooks::getDefaultMobileSkin(RequestContext, MobileContext)
#1 /home/jidanni/mediawiki/includes/Hooks.php(186): MobileFrontendHooks::onRequestContextCreateSkin(RequestContext, NULL)
#2 /home/jidanni/mediawiki/includes/context/RequestContext.php(406): Hooks::run(string, array)
#3 /home/jidanni/mediawiki/includes/context/ContextSource.php(154): RequestContext->getSkin()
#4 /home/jidanni/mediawiki/includes/OutputPage.php(2388): ContextSource->getSkin()
#5 /home/jidanni/mediawiki/includes/MediaWiki.php(875): OutputPage->output(boolean)
#6 /home/jidanni/mediawiki/includes/MediaWiki.php(887): MediaWiki->{closure}()
#7 /home/jidanni/mediawiki/includes/MediaWiki.php(523): MediaWiki->main()
#8 /home/jidanni/mediawiki/index.php(43): MediaWiki->run()
#9 {main}

Exception caught inside exception handler: [WYcX-UBaKU4AAANnjIcAAAAF] /index.php?title=%E9%A6%96%E9%A0%81 RuntimeException from line 57 of /home/jidanni/mediawiki/extensions/MobileFrontend/includes/MobileFrontend.hooks.php: wgMFDefaultSkinClass is not setup correctly. It should point to the class name of a valid skin e.g. SkinMinerva, SkinVector
Backtrace:
#0 /home/jidanni/mediawiki/extensions/MobileFrontend/includes/MobileFrontend.hooks.php(126): MobileFrontendHooks::getDefaultMobileSkin(RequestContext, MobileContext)
#1 /home/jidanni/mediawiki/includes/Hooks.php(186): MobileFrontendHooks::onRequestContextCreateSkin(RequestContext, NULL)
#2 /home/jidanni/mediawiki/includes/context/RequestContext.php(406): Hooks::run(string, array)
#3 /home/jidanni/mediawiki/includes/context/ContextSource.php(154): RequestContext->getSkin()
#4 /home/jidanni/mediawiki/includes/OutputPage.php(2388): ContextSource->getSkin()
#5 /home/jidanni/mediawiki/includes/exception/MWExceptionRenderer.php(135): OutputPage->output()
#6 /home/jidanni/mediawiki/includes/exception/MWExceptionRenderer.php(54): MWExceptionRenderer::reportHTML(RuntimeException)
#7 /home/jidanni/mediawiki/includes/exception/MWExceptionHandler.php(75): MWExceptionRenderer::output(RuntimeException, integer)
#8 /home/jidanni/mediawiki/includes/exception/MWExceptionHandler.php(130): MWExceptionHandler::report(RuntimeException)
#9 /home/jidanni/mediawiki/includes/MediaWiki.php(550): MWExceptionHandler::handleException(RuntimeException)
#10 /home/jidanni/mediawiki/index.php(43): MediaWiki->run()
#11 {main}

Event Timeline

Jidanni created this task.Aug 6 2017, 1:31 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 6 2017, 1:31 PM

OK added to LocalSettings.php:
$wgMFDefaultSkinClass = "SkinVector";
to bide for time.

It seems 100% of administrators doing straight / unattended upgrades will encounter this problem.

If you are splitting the skin out. Then at least have the program look around for skins that it can still use, and only if none are found, throw an exception.

It is much better to have users complain that the wiki looks a little different on their cellphone today, than throwing up a white screen of death.

For desktop
wfLoadSkin( 'Vector' );
is enough. The wiki does not break.

For mobile, similarly, if the user did not specify a skin, see if what is already loaded can be used, before throwing an exception.

Jidanni added a comment.EditedAug 6 2017, 2:49 PM

https://www.mediawiki.org/wiki/Manual:Skin_configuration#Setting_the_default_skin_for_a_wiki
"If you do not change this value, Vector will be the default skin."

You might as well make...I mean KEEP... it the default for MobileFrontend... 100 times better than throwing an exception.

Jdlrobson closed this task as Invalid.Aug 7 2017, 3:10 PM
Jdlrobson added a subscriber: Jdlrobson.

Hey, I'm sorry you encountered this issue. An email was sent to mediawiki-l, mobile-l and wikitech-l entitled "MobileFrontend disruption: Plans to break Minerva out into its own repository" at the start of July to avoid these issues. I recommend subscribing to one of those mailing lists to catch future big changes like this.

It seems 100% of administrators doing straight / unattended upgrades will encounter this problem.

Yes. That's correct. I've tried my best to communicate this issue via mailing lists and through the code itself. Are there places you would have expected to see this information but didn't?

It is much better to have users complain that the wiki looks a little different on their cellphone today, than throwing up a white screen of death.

The error is purposely a white screen of death to make sure this is correctly fixed. I'm not sure what your monthly deploy process looks like, but it should be pretty clear within seconds by looking at any of your logs or the mobile site immediately after deployment that there is a problem.

The error seems pretty descriptive to me:
wgMFDefaultSkinClass is not setup correctly. It should point to the class name of a valid skin e.g. SkinMinerva, SkinVector
and it looks like you've somewhat worked this out.

You might as well make...I mean KEEP... it the default for MobileFrontend... 100 times better than throwing an exception.

I have to disagree. Had you not had very visible exceptions, it would have been assumed you redesigned your mobile site, which I'd argue is even worse as it would have taken away your choice or even knowledge of that choice.

You'll probably want to download the MinervaNeue skin if you haven't already:

wfLoadSkin( 'MinervaNeue' );
$wgMFDefaultSkinClass = "SkinMinerva";

There is documentation here:
https://www.mediawiki.org/wiki/Extension:MobileFrontend#Setup_a_skin

Oops, I replied in T171062#3509979 .

The error is purposely a white screen of death to make sure this is correctly fixed. I'm not sure what your monthly deploy process looks like, but it should be pretty clear within seconds by looking at any of your logs or the mobile site immediately after deployment that there is a problem.

Gee, thanks for putting me in my place by embarassing me in front of hundreds of users over the holidays, that's exactly what I wanted for Christmas.

If you make a release that's guaranteed to break something this badly - you should bloody well screm about it in the release notes. Not burry it in an exception and a subsection of a wiki page I don't have any reason to go to.

No, I don't have automation testing the releases, I'm one guy who has an unrelated full time job. Meh, whatever. I guess I'l avoid new releases in the future, since aparently this is what I am supposed to expect.

Happy new year to you too!

Zoglun added a subscriber: Zoglun.Jan 4 2018, 12:57 AM

Is the "SkinMinerva" now (after MW 1.30) separate out of Extension:MobileFrontend? I got same problem after upgrade to Mediawiki 1.30

I'm sorry to hear about these problems :(

So this doesn't happen again, which channels would you expect to hear about breaking changes like this? Right now, extensions or skins don't really have release notes - only MediaWiki core, so it's hard to know where to put this information. Obviously MediaWiki.org and wikitech-l mailing list were not sufficient.

With your help, I'll make sure a preferred process for third parties is documented to avoid these kinds of issue. We don't often here from 3rd parties unless something breaks so more dialog there would be very useful. I'm not sure if that's something Wikimedia community liasons i.e. @CKoerner_WMF could help improve.

The configuration of the skin is documented in https://www.mediawiki.org/wiki/Extension:MobileFrontend#Setup_a_skin and I've added a new FAQ to cover this issue. Let me know if more information is needed via the talk page.

I'm not a mediawiki developer and don't know the details of what is possible and what is too hard. My guess is that the following message:

The Mobile Frontend extension is misconfigured. Please click here to see the desktop version of the page.

Would be not hard to implement, and infinitely nicer for both users and administrators than this:

MediaWiki internal error.
Original exception: [WYcX-UBaKU4AAANnjIcAAAAF] /index.php?title=%E9%A6%96%E9%A0%81 RuntimeException from line 57 of /home/jidanni/mediawiki/extensions/MobileFrontend/includes/MobileFrontend.hooks.php: wgMFDefaultSkinClass is not setup correctly. It should point to the class name of a valid skin e.g. SkinMinerva, SkinVector
Backtrace:
#0 /home/jidanni/mediawiki/extensions/MobileFrontend/includes/MobileFrontend.hooks.php(126): MobileFrontendHooks::getDefaultMobileSkin(RequestContext, MobileContext)
#1 /home/jidanni/mediawiki/includes/Hooks.php(186): MobileFrontendHooks::onRequestContextCreateSkin(RequestContext, NULL)
#2 /home/jidanni/mediawiki/includes/context/RequestContext.php(406): Hooks::run(string, array)
#3 /home/jidanni/mediawiki/includes/context/ContextSource.php(154): RequestContext->getSkin()
#4 /home/jidanni/mediawiki/includes/OutputPage.php(2388): ContextSource->getSkin()
#5 /home/jidanni/mediawiki/includes/MediaWiki.php(875): OutputPage->output(boolean)
#6 /home/jidanni/mediawiki/includes/MediaWiki.php(887): MediaWiki->{closure}()
#7 /home/jidanni/mediawiki/includes/MediaWiki.php(523): MediaWiki->main()
#8 /home/jidanni/mediawiki/index.php(43): MediaWiki->run()
#9 {main}

This would probably be great for all kinds of misconfiguration problems, perhaps even those you don't know in advance are going to cause headaches.

I see what you mean about the release notes. Ideally there should never be any unavoidable massive problems with the latest upgrade, but if one is really required: would be good to have a flashy note up where admins would go during their wiki upgrade and they will notice when getting the latest version of each extension.

The only thing I can think of is to add an "Upgrade" link next to the "Download snapshot" link which would have a click-through warning page leading to the same download page.

Yes!
And the update script should have
echo "**** ADMINISTRATOR: THERE WILL BE PROBLEMS....
echo ....
echo OK\?
read
exit 1

Yes we could definitely show a message in the UI saying it's misconfigured with a link to desktop. My concern with this approach however would be that if the site admin didn't notice this or wasn't made aware of this by looking at the mediawiki logs it could be weeks/months unnoticed as it relies on a user noticing and reporting the bug. The issue appearing in the server logs would be more noticeable. There is definitely a compromise here where a warning is logged in addition to the user message. In hindsight that may have been more preferential here.

And the update script should have

echo "**** ADMINISTRATOR: THERE WILL BE PROBLEMS....

The only thing I can think of is to add an "Upgrade" link next to the "Download snapshot" link which would have a click-through warning page leading to the same download page.

Thanks for these ideas. I've brought this up on the wikitech-l mailing list (pointing to this task) to ask for guidance. If nothing else, we should hopefully get some better guidance on how to handle these breaking changes better in future. Thanks for the feedback!

My take on this:

Failing hard and fast is a totally acceptable way to make a breaking change to an extension. Breaking changes will happen sometimes, and its better to be straight about it than half work. In my opinion the main lesson here is we should have gone further - instead of just white screening on mobile views he should have made it whitescreen on all mediawiki loads.

It would be inappropriate to put info about this change in mediawiki core's release notes as its an extension (unless mobile frontend is bundled with core, then an arg could be made but its not clear cut). Some extensions have their own release notes files, if mf did that it would of course make sense to include this there. But im also kind of doubtful wiki sysadmins would check all release notes for all extensions.

Using update.php to check extension config correctness is a nice idea. This is a bit of a departure from how we normally use update.php - but i dont see any reason why we couldnt use the load extension schema updates hook for this purpose and it would be convienent to system admins

demon added a subscriber: demon.Feb 6 2018, 3:06 AM

Using update.php to check extension config correctness is a nice idea. This is a bit of a departure from how we normally use update.php - but i dont see any reason why we couldnt use the load extension schema updates hook for this purpose and it would be convienent to system admins

Actually, I wonder if the pre-checks is a better place to add a hook. That allows the user to be notified and optionally continue already (it uses Status::warn() vs Status::error() to differentiate)

In my opinion the main lesson here is we should have gone further - instead of just white screening on mobile views he should have made it whitescreen on all mediawiki loads.

That's really good feedback. Thank you.