Page MenuHomePhabricator

Installer stylesheet fails to load on 1.36/master
Closed, ResolvedPublic5 Estimated Story PointsBUG REPORT

Description

Using rMWdbb1a9ef64604cedfd85b7c04a6b4699cad5c3c2, the installer is missing styles. Opening the console traces it to an exception throw when generating the stylesheet:

/mw-config/index.php?css=1 Wikimedia\Services\ServiceDisabledException from line 412 of mediawiki/vendor/wikimedia/services/src/ServiceContainer.php: Service disabled: DBLoadBalancer

Backtrace:

#0 mediawiki/includes/MediaWikiServices.php(709): Wikimedia\Services\ServiceContainer->getService(string)
#1 mediawiki/includes/GlobalFunctions.php(2467): MediaWiki\MediaWikiServices->getDBLoadBalancer()
#2 mediawiki/includes/GlobalFunctions.php(2453): wfGetLB(boolean)
#3 mediawiki/includes/resourceloader/MessageBlobStore.php(156): wfGetDB(integer)
#4 mediawiki/includes/resourceloader/MessageBlobStore.php(120): MessageBlobStore->recacheMessageBlob(string, ResourceLoaderSkinModule, string)
#5 mediawiki/includes/resourceloader/MessageBlobStore.php(85): MessageBlobStore->getBlobs(array, string)
#6 mediawiki/includes/resourceloader/ResourceLoaderModule.php(585): MessageBlobStore->getBlob(ResourceLoaderSkinModule, string)
#7 mediawiki/includes/resourceloader/ResourceLoaderLessVarFileModule.php(111): ResourceLoaderModule->getMessageBlob(ResourceLoaderContext)
#8 mediawiki/includes/resourceloader/ResourceLoaderSkinModule.php(514): ResourceLoaderLessVarFileModule->getLessVars(ResourceLoaderContext)
#9 mediawiki/includes/resourceloader/ResourceLoaderFileModule.php(1098): ResourceLoaderSkinModule->getLessVars(ResourceLoaderContext)
#10 mediawiki/includes/resourceloader/ResourceLoaderFileModule.php(990): ResourceLoaderFileModule->compileLessString(string, string, ResourceLoaderContext)
#11 mediawiki/includes/resourceloader/ResourceLoaderFileModule.php(966): ResourceLoaderFileModule->processStyle(string, string, ResourceLoaderFilePath, ResourceLoaderContext)
#12 mediawiki/includes/resourceloader/ResourceLoaderFileModule.php(943): ResourceLoaderFileModule->readStyleFile(ResourceLoaderFilePath, ResourceLoaderContext)
#13 mediawiki/includes/installer\WebInstallerOutput.php(160): ResourceLoaderFileModule->readStyleFiles(array, ResourceLoaderContext)
#14 mediawiki/includes/installer\WebInstaller.php(1218): WebInstallerOutput->getCSS()
#15 mediawiki/includes/installer\WebInstaller.php(178): WebInstaller->outputCss()
#16 mediawiki/mw-config/index.php(82): WebInstaller->execute(array)
#17 mediawiki/mw-config/index.php(40): wfInstallerMain()
#18 {main}

Problem:

When introducing the "toc" feature, we made ResourceLoaderSkinModule use LESS messages. Even though this feature is not in the installer it has an impact on caching and requires access to the MessageBlobStore. The MessageBlobStore access the database which is not available in the installer so an exception is thrown.

The required fix would be to make it possible for ResourceLoader to disable database access and/or to avoid the message lookup for ResourceLoaderSkinModules which do not use the toc feature.

Event Timeline

rMW9c253febe2323c8bcdb2be871e4c9e7699a7b847 works fine, rMWdb3c8ea16b3f5c9fa80a8778cef1e234c6efffff (T252774: Merge mediawiki.toc.styles styles into ResourceLoaderSkinModule) does not. Based on the stacktrace, it appears that ResourceLoaderSkinModule extending ResourceLoaderLessVarFileModule is the culprit.

Mainframe98 added a subscriber: Krinkle.

To be more precise: ResourceLoaderLessVarModule gets the message blob for ResourceLoaderSkinModule::LESS_MESSAGES, which after a bit of indirection gets that from MessageBlobStore, which eventually calls recacheMessageBlob. In recacheMessageBlob there's a call to wfGetDB, which isn't going to work in the installer.

I managed to fix this locally with an overridden instance of MessageBlobStore that changes getBlobs not to call recacheMessageBlob, and have WebInstallerOutput::getCSS set that as the MessageBlobStore on the ResourceLoader that is created in that method. @Krinkle is this an approach that is acceptable?

@Mainframe98 It's possible, but I don't recommend it. There is no need to make this work in the installer. The problem is that code is being activated that afaik has no place in the installer.

@Jdlrobson It looks like change 621040 as part of T252774 enabled the toc feature in the SkinModule used by the installer, which thus causes the localisation system and LessMessages to be activated there which are not safe for use during the installer and afaik don't need to be. Turning this stylesheet off there should fix it. Can you look into that?

Krinkle renamed this task from WebInstaller is served without styles to Installer stylesheet fails to load on 1.36 master.Dec 13 2020, 8:42 PM
Krinkle removed a subscriber: Krinkle.

Change 649409 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/core@master] WIP: Fix the styling of the installer

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

is there any estimated time to fix this issue? already one month gone till the date of reporting the issue. Can you tell me how should I continue the installation?

It should be possible to use the installer without styles, but you can apply the patch https://gerrit.wikimedia.org/r/649409 to your mediawiki instance in the mean time.

The 1.36 release is not for several months now, so this is not a high priority right now and our volunteer/staff are spread pretty thin.

Change 649409 abandoned by Jdlrobson:
[mediawiki/core@master] Fix the styling of the installer and deprecate its RL module

Reason:
This has grown a lot in scope since I first started it and now involves fixing ResourceLoader internals. I don't have time to focus time on this right now. Others should feel free to pick up the task.

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

@Jdlrobson added to the task description:

When introducing the "toc" feature, we made ResourceLoaderSkinModule use LESS messages. Even though this feature is not in the installer it has an impact on caching and requires access to the MessageBlobStore. The MessageBlobStore access the database which is not available in the installer so an exception is thrown.
The required fix would be to make it possible for ResourceLoader to disable database access and/or to avoid the message lookup for ResourceLoaderSkinModules which do not use the toc feature.

MessageBlobStore was always constructed by ResourceLoader, and does not require the database service during instantiation.

ResourceLoader does not intract with MessageBlobStore if a module has no messages. In addition, even if it did somehow ask for a blob from MessageBlobStore, that service does not perform any message cache lookups if getMessages() is empty as it is by default. I was not able to break this assumption in my local set up.

From what I can tell, the issue is simply that the SkinModule instance used in the installer, does not have an empty lessMessages array. The SkinModule class currently hardcodes [ 'hidetoc', 'showtoc' ] as lessMessages option, regardless of which features are enabled. Fixing that mean getMessages() returns empty and thus it all works again.

Reedy raised the priority of this task from Low to Needs Triage.Mar 13 2021, 4:01 PM
Reedy added a subscriber: Reedy.

1.36 is getting closer, and this is still broke. Resetting to Needs Triage; a pretty broken looking installer is not "low priority"

I hit this when I was making patches for the new mediawiki logo (T268230: Roll out the new logo of MediaWiki). The branch cut is pretty soon.

Jdlrobson triaged this task as Medium priority.Mar 16 2021, 8:06 PM

The associated patch I worked on (https://gerrit.wikimedia.org/r/c/649409) is mostly there. Feel free to restore it and tinker, otherwise, I'll plan to get round to this in about 2-4 weeks. Feel free to poke me and bump to high after then.

Change 649409 restored by Jdlrobson:
[mediawiki/core@master] Fix the styling of the installer and deprecate its RL module

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

Change 674457 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/core@master] Fix the styling of the installer and deprecate its RL module

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

Change 649409 abandoned by Jdlrobson:
[mediawiki/core@master] Fix the styling of the installer and deprecate its RL module

Reason:
see https://gerrit.wikimedia.org/r/c/mediawiki/core/ /674457

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

MessageBlobStore was always constructed by ResourceLoader, and does not require the database service during instantiation.

MessageBlobStore->getBlobs requires the database.

ResourceLoader does not intract with MessageBlobStore if a module has no messages. In addition, even if it did somehow ask for a blob from MessageBlobStore, that service does not perform any message cache lookups if getMessages() is empty as it is by default. I was not able to break this assumption in my local set up.

True. That's what https://gerrit.wikimedia.org/r/c/mediawiki/core/+/674457/2/includes/resourceloader/ResourceLoaderSkinModule.php change is trying to do, by making sure the lessMessages is defined as an empty array when toc feature is not present.

From what I can tell, the issue is simply that the SkinModule instance used in the installer, does not have an empty lessMessages array. The SkinModule class currently hardcodes [ 'hidetoc', 'showtoc' ] as lessMessages option, regardless of which features are enabled. Fixing that mean getMessages() returns empty and thus it all works again.

So yes and no. Yes, the array needs to be empty (see above), but it also seems that this is not enough. This results in:

[34f6a76a3a511efeabc44a70] /w/mw-config/index.php?css=1222 TypeError: Return value of ResourceLoaderLessVarFileModule::pluckFromMessageBlob() must be of the type array, null returned

The problem here is that ResourceLoaderLessVarFileModule::getLessVars calls parent::getMessageBlob which calls ResourceLoaderModule::getMessageBlob which can return null.

This is what the following tries to fix:
https://gerrit.wikimedia.org/r/c/mediawiki/core/+/674457/2/includes/resourceloader/ResourceLoaderLessVarFileModule.php

This is going to block the next release Olga which is in roughly 2 months time so I think we should look at this sooner rather than later in case it proves trickier than expected.

This is going to block the next release Olga which is in roughly 2 months time so I think we should look at this sooner rather than later in case it proves trickier than expected.

Not two months' time. The rc.0 cut is nominally scheduled for two weeks' time. I'd suggest this is raised to High verging on UBN.

ovasileva raised the priority of this task from Medium to High.Apr 14 2021, 5:13 PM

@Jdforrester-WMF The rc.0 cut schedule hit not only me by surprise. Thanks for adding the info! What calendars could I subscribe to know this with enough time ahead?

@Jdforrester-WMF The rc.0 cut schedule hit not only me by surprise. Thanks for adding the info! What calendars could I subscribe to know this with enough time ahead?

https://www.mediawiki.org/wiki/Version_lifecycle is the most high-profile place; that was changed from March 2021 to May 2021 in January after I guesstimated May for the release, as it was nine months after the previous one (normally we're every-six months, but COVID meant things were slower).

Ideally there'd have been an announcement a few days before the branch cut, but no-one remembered, sorry.

Change 674457 abandoned by Jdlrobson:

[mediawiki/core@master] Fix the styling of the installer and deprecate its RL module

Reason:

See https://gerrit.wikimedia.org/r/c/mediawiki/core/ /679503

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

Change 679505 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/core@master] Installer: Do not use `mediawiki.skinning.interface` module

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

Reedy renamed this task from Installer stylesheet fails to load on 1.36 master to Installer stylesheet fails to load on 1.36/master.Apr 15 2021, 6:03 PM

Change 679505 merged by jenkins-bot:

[mediawiki/core@master] Installer: Do not use `mediawiki.skinning.interface` module

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

Change 680861 had a related patch set uploaded (by Jforrester; author: Jdlrobson):

[mediawiki/core@REL1_36] Installer: Do not use `mediawiki.skinning.interface` module

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

Is this Resolved if we land it into REL1_36 or is there more to do?

Change 679503 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/core@master] ResourceLoaderSkinModule: When `toc` is enabled messages are empty

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

Change 680861 merged by jenkins-bot:

[mediawiki/core@REL1_36] Installer: Do not use `mediawiki.skinning.interface` module

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

Change 679503 merged by jenkins-bot:

[mediawiki/core@master] ResourceLoaderSkinModule: When `toc` is enabled messages are empty

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

Change 681148 had a related patch set uploaded (by Jforrester; author: Jdlrobson):

[mediawiki/core@REL1_36] ResourceLoaderSkinModule: When `toc` is enabled messages are empty

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

Change 681148 merged by jenkins-bot:

[mediawiki/core@REL1_36] ResourceLoaderSkinModule: When `toc` is enabled messages are empty

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

Screen Shot 2021-04-20 at 4.08.59 PM.png (1×2 px, 444 KB)

Thanks everyone. I can confirm this is finally working and backported.