Page MenuHomePhabricator

Fatal MWException in Babel: "Language::isValidBuiltInCode must be passed a string"
Closed, ResolvedPublic

Description

Seen 17 times in the last few hours. Started in wmf.13.

Can be reproduced in two ways:

type: mediawiki
channel: exception
exception.class: MWException

Language::isValidBuiltInCode must be passed a string, boolean given
#0 /srv/mediawiki/php-1.32.0-wmf.13/languages/Language.php(213): Language::isValidBuiltInCode(boolean)
#1 /srv/mediawiki/php-1.32.0-wmf.13/languages/Language.php(191): Language::newFromCode(boolean)
#2 /srv/mediawiki/php-1.32.0-wmf.13/extensions/Babel/includes/BabelBox/LanguageBabelBox.php(85): Language::factory(boolean)
#3 /srv/mediawiki/php-1.32.0-wmf.13/extensions/Babel/includes/Babel.php(190): MediaWiki\Babel\BabelBox\LanguageBabelBox->render()
#4 /srv/mediawiki/php-1.32.0-wmf.13/extensions/Babel/includes/Babel.php(109): Babel::mGenerateContent(Parser, string, array)
#5 /srv/mediawiki/php-1.32.0-wmf.13/extensions/Babel/includes/Babel.php(47): Babel::mGenerateContentTower(Parser, array)
#6 /srv/mediawiki/php-1.32.0-wmf.13/includes/parser/Parser.php(3434): Babel::Render(Parser, string, string, string, string, string)
#7 /srv/mediawiki/php-1.32.0-wmf.13/includes/parser/Parser.php(3138): Parser->callParserFunction(PPTemplateFrame_Hash, string, array)
#8 /srv/mediawiki/php-1.32.0-wmf.13/includes/parser/Preprocessor_Hash.php(1114): Parser->braceSubstitution(array, PPTemplateFrame_Hash)
#9 /srv/mediawiki/php-1.32.0-wmf.13/includes/parser/Preprocessor_Hash.php(1546): PPFrame_Hash->expand(PPNode_Hash_Tree, integer)
#10 /srv/mediawiki/php-1.32.0-wmf.13/includes/parser/Parser.php(3310): PPTemplateFrame_Hash->cachedExpand(string, PPNode_Hash_Tree)
#11 /srv/mediawiki/php-1.32.0-wmf.13/includes/parser/Preprocessor_Hash.php(1114): Parser->braceSubstitution(array, PPFrame_Hash)
#12 /srv/mediawiki/php-1.32.0-wmf.13/includes/parser/Parser.php(2953): PPFrame_Hash->expand(PPNode_Hash_Tree, integer)
#13 /srv/mediawiki/php-1.32.0-wmf.13/includes/parser/Parser.php(1297): Parser->replaceVariables(string)
#14 /srv/mediawiki/php-1.32.0-wmf.13/includes/parser/Parser.php(446): Parser->internalParse(string)
#15 /srv/mediawiki/php-1.32.0-wmf.13/includes/StubObject.php(112): Parser->parse(string, Title, ParserOptions, boolean, boolean, NULL)
#16 /srv/mediawiki/php-1.32.0-wmf.13/includes/StubObject.php(138): StubObject->_call(string, array)
#17 /srv/mediawiki/php-1.32.0-wmf.13/includes/content/WikitextContent.php(323): StubObject->__call(string, array)
#18 /srv/mediawiki/php-1.32.0-wmf.13/includes/content/AbstractContent.php(516): WikitextContent->fillParserOutput(Title, NULL, ParserOptions, boolean, ParserOutput)
#19 /srv/mediawiki/php-1.32.0-wmf.13/includes/api/ApiParse.php(268): AbstractContent->getParserOutput(Title, NULL, ParserOptions)
#20 /srv/mediawiki/php-1.32.0-wmf.13/includes/api/ApiMain.php(1584): ApiParse->execute()
#21 /srv/mediawiki/php-1.32.0-wmf.13/includes/api/ApiMain.php(535): ApiMain->executeAction()
#22 /srv/mediawiki/php-1.32.0-wmf.13/includes/api/ApiMain.php(506): ApiMain->executeActionWithErrorHandling()
#23 /srv/mediawiki/php-1.32.0-wmf.13/api.php(83): ApiMain->execute()
#24 /srv/mediawiki/w/api.php(3): include(string)
#25 {main}

Event Timeline

Krinkle created this task.Jul 18 2018, 7:51 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 18 2018, 7:51 PM
greg added a subscriber: greg.

+Editing-team per Developers/Maintainers mw.org page.

I've found two recent patches to Core that added some language code checks:

Not sure if they'd be related, etc. Just in case. Thanks.

Hm. As far as I can tell, we're falling through the wfDebug(...) in BabelLanguageCodes::getCode( $code ) and returning false. Probably because getCode() is being given a new BCP 47-normalized code which doesn't appear in Babel's codes.txt database?

I'm guessing the problem is with the code nrm, which we use for "Norman French", which used to be mapped in codes.txt to the language "Narom", but is now BCP 47-normalized to nrf (the actual code for norman french) which doesn't exist in codes.txt so is resulting in getCode() returning false. The simplest patch might be to add nrf to codes.txt.

Problem with this theory: there are plenty of other BCP-47 normalized codes (en-simple, en-x-simple, simple, en-x-rtl) which ought to fall through this same path. But perhaps the missing codes would always be caught by Language::fetchLanguageCode( $code ) before? Still working on this...

Change 446766 had a related patch set uploaded (by C. Scott Ananian; owner: C. Scott Ananian):
[mediawiki/extensions/Babel@master] Ensure that BCP 47-mapped codes can be mapped to MediaWiki-internal codes

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

This is blocking the train from moving forward. If the issue is resolved, please resolve the task. If there is a workaround, please remove this task from T191059: 1.32.0-wmf.13 deployment blockers subtasks.

zeljkofilipin triaged this task as High priority.Jul 19 2018, 1:13 PM

Raising the priority because this task is blocking the train.

Arrbee moved this task from Backlog to Other teams on the Language-Team board.Jul 19 2018, 2:07 PM
greg raised the priority of this task from High to Unbreak Now!.Jul 19 2018, 3:16 PM

If it blocks the train it's UBN!.

Restricted Application added subscribers: Liuxinyu970226, TerraCodes. · View Herald TranscriptJul 19 2018, 3:16 PM
greg added a comment.Jul 19 2018, 3:21 PM

From the description:

Seen 17 times in the last few hours. Started in wmf.13.

I'm not sure that qualifies as a train blocker. That's not very often, relative to us. What exact functionality/use case is not working for our users?

From the description:

Seen 17 times in the last few hours. Started in wmf.13.

I'm not sure that qualifies as a train blocker. That's not very often, relative to us. What exact functionality/use case is not working for our users?

API use on wikis by users with Babel boxes with non-industry standard language codes (like bat-smg) will be broken. Pretty minor, but not good.

Nemo_bis added a subscriber: Nemo_bis.EditedJul 19 2018, 4:09 PM

Giving a fatal to the target users is hardly an improvement, so it would be fine to just revert the recent change.

There's a patch with a C+1, but it's not merged (waiting for review from @Nikerabbit I think). I'm not sure what "recent change" @Nemo_bis is referring to; if you wanted to revert core patches I think you'd need to revert both I807dd55d49e9bd19443329231326a5b0d3e6c453 and I8468a56d5b88f5786abd0a17b67bda2f1687fd0c (the latter is on top of the former).

hashar added a subscriber: hashar.

Wikidata also had some funky languages issues, reported at T199983 . The site has been reverted back to wmf.12.

Krinkle updated the task description. (Show Details)Jul 20 2018, 7:01 PM
Rxy added a subscriber: Rxy.Jul 21 2018, 12:24 AM

Friendly reminder: the train is blocked because of this task. (It's not the only blocker at the time I write this.) Any updates?

Krinkle added a comment.EditedJul 23 2018, 3:50 PM

I've left some analysis on the patch at https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/Babel/+/446766/, but am waiting for CScott to reply.

Meanwhile, as mentioned by CSCott earlier, instead of blocking this on a change in Babel, might be best to revert these two core changes in master and WMF branches. The Babel code will naturally started working again as before if we do that. Then we can continue to investigate and find a way to make Babel play nice with this change and try it again at a later time after Babel is updated with whatever it needs.

greg added a comment.Jul 23 2018, 4:38 PM

Meanwhile, as mentioned by CSCott earlier, instead of blocking this on a change in Babel, might be best to revert these two core changes in master and WMF branches. The Babel code will naturally started working again as before if we do that. Then we can continue to investigate and find a way to make Babel play nice with this change and try it again at a later time after Babel is updated with whatever it needs.

I think this ^^ makes the most sense given it's Monday and we're coming up on being blocked for 5 days.

Change 447469 had a related patch set uploaded (by Greg Grossmeier; owner: Greg Grossmeier):
[mediawiki/core@master] Revert "Accept BCP 47 codes as aliases for nonstandard variants"

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

Change 447470 had a related patch set uploaded (by Greg Grossmeier; owner: Greg Grossmeier):
[mediawiki/core@master] Revert "Ensure LanguageCode::bcp47() returns a valid BCP 47 language code"

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

greg added a comment.Jul 23 2018, 4:47 PM

Someone (@Krinkle / @cscott) please code-review those reverts is that's the right way forward.

Change 447469 merged by jenkins-bot:
[mediawiki/core@master] Revert "Accept BCP 47 codes as aliases for nonstandard variants"

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

Change 447470 merged by jenkins-bot:
[mediawiki/core@master] Revert "Ensure LanguageCode::bcp47() returns a valid BCP 47 language code"

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

greg added a comment.Jul 23 2018, 8:29 PM

With those two merges, let's get this backported and pushed out.

Change 447506 had a related patch set uploaded (by Greg Grossmeier; owner: Greg Grossmeier):
[mediawiki/core@wmf/1.32.0-wmf.13] Revert "Accept BCP 47 codes as aliases for nonstandard variants"

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

Change 447507 had a related patch set uploaded (by Greg Grossmeier; owner: Greg Grossmeier):
[mediawiki/core@wmf/1.32.0-wmf.13] Revert "Ensure LanguageCode::bcp47() returns a valid BCP 47 language code"

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

Change 447506 merged by jenkins-bot:
[mediawiki/core@wmf/1.32.0-wmf.13] Revert "Accept BCP 47 codes as aliases for nonstandard variants"

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

Change 447507 merged by jenkins-bot:
[mediawiki/core@wmf/1.32.0-wmf.13] Revert "Ensure LanguageCode::bcp47() returns a valid BCP 47 language code"

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

Mentioned in SAL (#wikimedia-operations) [2018-07-23T21:53:44Z] <thcipriani@deploy1001> Started scap: [[gerrit:447506|Revert "Accept BCP 47 codes as aliases for nonstandard variants"]] T199941 [[gerrit:447507|Revert "Ensure LanguageCode::bcp47() returns a valid BCP 47 language code"]] T199941

Mentioned in SAL (#wikimedia-operations) [2018-07-23T21:53:44Z] <thcipriani@deploy1001> Started scap: [[gerrit:447506|Revert "Accept BCP 47 codes as aliases for nonstandard variants"]] T199941 [[gerrit:447507|Revert "Ensure LanguageCode::bcp47() returns a valid BCP 47 language code"]] T199941

Mentioned in SAL (#wikimedia-operations) [2018-07-23T22:27:05Z] <thcipriani@deploy1001> Finished scap: [[gerrit:447506|Revert "Accept BCP 47 codes as aliases for nonstandard variants"]] T199941 [[gerrit:447507|Revert "Ensure LanguageCode::bcp47() returns a valid BCP 47 language code"]] T199941 (duration: 33m 21s)

Mentioned in SAL (#wikimedia-operations) [2018-07-23T22:27:05Z] <thcipriani@deploy1001> Finished scap: [[gerrit:447506|Revert "Accept BCP 47 codes as aliases for nonstandard variants"]] T199941 [[gerrit:447507|Revert "Ensure LanguageCode::bcp47() returns a valid BCP 47 language code"]] T199941 (duration: 33m 21s)

This is the only remaining blocker for T191059: 1.32.0-wmf.13 deployment blockers. I see some activity at the task (commit merges, deployments) but the task is still open, UBN and a subtask of T191059, making it a blocker.

If the issue is resolved, please resolve the phabricator task. If there is still some cleanup to do, but it's not blocking the train, please remove the task from T191059 blockers.

@KartikMistry is the issue resolved then? Or not resolved, but not blocking the train?

zeljkofilipin added a comment.EditedJul 24 2018, 12:30 PM

@Krinkle can you please confirm this is resolved? It's the only remaining blocker for last week's train. We should start with this week's train today.

thcipriani closed this task as Resolved.Jul 24 2018, 1:03 PM
thcipriani assigned this task to Krinkle.
thcipriani added a subscriber: thcipriani.

I deployed the fix yesterday (my) evening. Should be resolved now.

Follow-up:

  • Started after https://gerrit.wikimedia.org/r/442200 got merged: this caused LanguageCode::bcp47('nrm') == 'nrf', among other normalizations.
  • As @Krinkle noted on https://gerrit.wikimedia.org/r/446766 the fatal exception came from mGenerateContent calling LanguageBabelBox::render() for a LanguageBabelBox instance having an invalid language code that Language::factory rejects.
    • In mParseParameter(): BabelLanguageCodes::getCode('nrm') == 'nrm' ("Narom")
    • In LanguageBabelBox::__construct(): bcp47('nrm') == 'nrf' ("Norman")
    • In LanguageBabelBox::render(): getCode('nrf') == false (not in LanguageCode::deprecatedLanguageCodeMapping() nor MediaWiki\Languages\Data\Names)
    • In LanguageBabelBox::render(): Language:getFactory(false) => CRASH

https://gerrit.wikimedia.org/r/446766 was proposed as fix (it would ensure getCode('nrf') == 'nrf', preventing the crash)

Incident report: https://wikitech.wikimedia.org/wiki/Incident_documentation/20180717-Train

Change 465763 had a related patch set uploaded (by C. Scott Ananian; owner: C. Scott Ananian):
[mediawiki/extensions/Babel@master] Tests for T199941

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

Change 465763 had a related patch set uploaded (by C. Scott Ananian; owner: C. Scott Ananian):
[mediawiki/extensions/Babel@master] Improve tests to cover more language code corner cases

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

Change 465763 merged by jenkins-bot:
[mediawiki/extensions/Babel@master] Improve tests to cover more language code corner cases

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

Change 446766 merged by jenkins-bot:
[mediawiki/extensions/Babel@master] Differentiate MediaWiki-internal codes from BCP 47 codes

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

mmodell changed the subtype of this task from "Task" to "Production Error".Wed, Aug 28, 11:08 PM