| • mmodell | |
| Feb 5 2020, 8:48 PM |
| F31552585: image.png | |
| Feb 6 2020, 7:49 PM |
MediaWiki version: 1.35.0-wmf.18
PHP Notice: Undefined offset: 1
currently one of the most frequent errors in production logspam
This is not limited to 1.35.0-wmf.18, as there are a few errors on 1.35.0-wmf.16
https://logstash.wikimedia.org/goto/4ffa81152ef9436b5209932845f6aac4
#0 /srv/mediawiki/php-1.35.0-wmf.18/includes/resourceloader/ResourceLoaderSkinModule.php(264): MWExceptionHandler::handleError(integer, string, string, integer, array)
#1 /srv/mediawiki/php-1.35.0-wmf.18/includes/resourceloader/ResourceLoaderSkinModule.php(204): ResourceLoaderSkinModule->getLogoPreloadlinks()
#2 /srv/mediawiki/php-1.35.0-wmf.18/includes/resourceloader/ResourceLoaderModule.php(601): ResourceLoaderSkinModule->getPreloadLinks(ResourceLoaderContext)
#3 /srv/mediawiki/php-1.35.0-wmf.18/includes/resourceloader/ResourceLoaderModule.php(779): ResourceLoaderModule->getHeaders(ResourceLoaderContext)
#4 /srv/mediawiki/php-1.35.0-wmf.18/includes/resourceloader/ResourceLoaderModule.php(681): ResourceLoaderModule->buildContent(ResourceLoaderContext)
#5 /srv/mediawiki/php-1.35.0-wmf.18/includes/resourceloader/ResourceLoader.php(1084): ResourceLoaderModule->getModuleContent(ResourceLoaderContext)
#6 /srv/mediawiki/php-1.35.0-wmf.18/includes/resourceloader/ResourceLoader.php(791): ResourceLoader->makeModuleResponse(ResourceLoaderContext, array, array)
#7 /srv/mediawiki/php-1.35.0-wmf.18/load.php(43): ResourceLoader->respond(ResourceLoaderContext)
#8 /srv/mediawiki/w/load.php(3): require(string)
#9 {main}| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Resolved | Goal | ovasileva | T242131 [GOAL] Deploy new header to at least one test wiki | ||
| Resolved | Jdlrobson | T240856 [EPIC] New header for desktop improvements project | |||
| Resolved | ovasileva | T232140 Separate out logo handling into square image logos and long text/wordmark banner logos | |||
| Resolved | Release | • mmodell | T233866 1.35.0-wmf.18 deployment blockers | ||
| Resolved | PRODUCTION ERROR | Jdlrobson | T244405 PHP Notice: Undefined offset: 1 in ResourceLoaderSkinModule.php |
Probably broken by 8cd2e1336354ea8459f019cd989c4efe2516fd5f. Code later in that method expects that if $logos is an array, it will always have at least two elements (the offset '1' is hardcoded):
$media_query = 'not all and (min-resolution: ' . $logos[1]['dppx'] . 'dppx)';
Change 570434 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/core@master] ResourceLoaderSkinModule: Restore previous behavior in getLogoData()
We have weird transitional configuration right now, so on Wikimedia wikis, this error would appear on wikis that have wgMinervaCustomLogos set (even when not using Minerva).
If the changes were not meant change which logos are displayed I suggest we rollback those changes as the core patch already has compat code for it. We wrote the compat code so that there would not need to be any form of "transitional" config. The config is single-state only, and the transition happens in core at run-time. After the update is rolled out, wmf would go straight and clean from the old to the new format.
Yeah, the main patch in core wasn't backwards-compatible. Several patches have got us into a less-bad position, but it's not great.
Change 570666 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[operations/mediawiki-config@master] Set wgLogoHD before adding wordmark
Change 570666 merged by jenkins-bot:
[operations/mediawiki-config@master] Set wgLogoHD before adding wordmark
Mentioned in SAL (#wikimedia-operations) [2020-02-06T19:45:55Z] <jforrester@deploy1001> Synchronized wmf-config/CommonSettings.php: T244405 Set wgLogoHD before adding wordmark (duration: 01m 06s)
ff8e7421901c2a has fixed PHP Notice: Undefined offset: 1 in ResourceLoaderSkinModule, but has also introduced a new error:
PHP Notice: Undefined variable: wgLogos in /srv/mediawiki/wmf-config/CommonSettings.php on line 845
This is because in the currently deployed version of MediaWiki, wgLogos does not yet exist. And the wmf-config/InitialiseSetttings only provides a value on a subset of wikis, it is lacking a default (not even empty array or false).
This error was previously being masked because the $wgLogos['wordmark'] = assignment silently and magically lazy-created the global variable wgLogos even if it didn't exist yet with the implicit type of an array.
Unlike the ResourceLoaderSkinModule error, this is triggered for all http requests to MediaWiki for the affected wikis and raising priority as such due to making outages harder to investigate with the high-volume noise.
See T232140#5855798 for wider analysis and ideas.
Change 570729 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[operations/mediawiki-config@master] Don't trying to assign $wgLogos to $wgLogoHD if it's unset
Change 570729 merged by jenkins-bot:
[operations/mediawiki-config@master] Don't trying to assign $wgLogos to $wgLogoHD if it's unset
Mentioned in SAL (#wikimedia-operations) [2020-02-06T23:10:04Z] <jforrester@deploy1001> Synchronized wmf-config/CommonSettings.php: T244405 Don't trying to assign to if it's unset (duration: 01m 07s)
I'm not seeing any more errors in logstash
https://logstash.wikimedia.org/app/kibana#/dashboard/Fatal-Monitor?_g=h@cfc15f8&_a=h@b00e9b5
I'm truly sorry about how much of a mess I made this. Thank you @Jdforrester-WMF, @matmarex and @Krinkle for fixing my mistakes.
I'm now more familiar with how deprecating config variables works. If I did this again, knowing what I know now, I would have made config changes only for beta cluster and updated the config after the next train had rolled out.
I believe that Bartosz's change https://gerrit.wikimedia.org/r/570434 which I've +1ed needs to land before next branch cut to resolve the issue with the new configuration and new code. I've added a unit test but would appreciate another pair of eyes on that to +2.
Hey, it happens. Lots of things changing in one go, easy to lose track of details of a smallish part of them. :-)
I'm now more familiar with how deprecating config variables works. If I did this again, knowing what I know now, I would have made config changes only for beta cluster and updated the config after the next train had rolled out.
Also, I'm always available for a quick chat for anything related to config, production, or similar!
I believe that Bartosz's change https://gerrit.wikimedia.org/r/570434 which I've +1ed needs to land before next branch cut to resolve the issue with the new configuration and new code. I've added a unit test but would appreciate another pair of eyes on that to +2.
Yeah, I think that's a good change; C+2'ed.
Change 570434 merged by jenkins-bot:
[mediawiki/core@master] ResourceLoaderSkinModule: Restore previous behavior in getLogoData()
