Page MenuHomePhabricator

Warning: Locally stored wiki page has unsupported content model (from JsonConfig)
Closed, ResolvedPublic

Description

Error message
[{exception_id}] {exception_url}   ErrorException from line 452 of /srv/mediawiki/php-1.36.0-wmf.2/includes/debug/MWDebug.php: PHP Warning: The locally stored wiki page '486:DateI18n.tab' has unsupported content model' [Called from JsonConfig\JCCache::loa
Stack Trace
#0 [internal function]: MWExceptionHandler::handleError(integer, string, string, integer, array)
#1 /srv/mediawiki/php-1.36.0-wmf.2/includes/debug/MWDebug.php(452): trigger_error(string, integer)
#2 /srv/mediawiki/php-1.36.0-wmf.2/includes/debug/MWDebug.php(196): MWDebug::sendMessage(string, string, integer)
#3 /srv/mediawiki/php-1.36.0-wmf.2/includes/GlobalFunctions.php(1087): MWDebug::warning(string, integer, integer, string)
#4 /srv/mediawiki/php-1.36.0-wmf.2/extensions/JsonConfig/includes/JCCache.php(144): wfLogWarning(string)
#5 /srv/mediawiki/php-1.36.0-wmf.2/extensions/JsonConfig/includes/JCCache.php(58): JsonConfig\JCCache->loadLocal()
#6 /srv/mediawiki/php-1.36.0-wmf.2/extensions/JsonConfig/includes/JCSingleton.php(374): JsonConfig\JCCache->get()
#7 /srv/mediawiki/php-1.36.0-wmf.2/extensions/JsonConfig/includes/JCLuaLibrary.php(58): JsonConfig\JCSingleton::getContent(JsonConfig\JCTitle)
#8 /srv/mediawiki/php-1.36.0-wmf.2/extensions/Scribunto/includes/engines/LuaSandbox/LuaSandboxCallback.php(26): JsonConfig\JCLuaLibrary->get(string, string)
#9 [internal function]: Scribunto_LuaSandboxCallback->__call(string, array)
#10 /srv/mediawiki/php-1.36.0-wmf.2/extensions/Scribunto/includes/engines/LuaSandbox/LuaSandboxInterpreter.php(113): LuaSandboxFunction->call(LuaSandboxFunction)
#11 /srv/mediawiki/php-1.36.0-wmf.2/extensions/Scribunto/includes/engines/LuaCommon/LuaEngine.php(291): Scribunto_LuaSandboxInterpreter->callFunction(LuaSandboxFunction, LuaSandboxFunction)
#12 /srv/mediawiki/php-1.36.0-wmf.2/extensions/Scribunto/includes/engines/LuaCommon/LuaModule.php(68): Scribunto_LuaEngine->executeFunctionChunk(LuaSandboxFunction, PPTemplateFrame_Hash)
#13 /srv/mediawiki/php-1.36.0-wmf.2/extensions/Scribunto/includes/common/Hooks.php(128): Scribunto_LuaModule->invoke(string, PPTemplateFrame_Hash)
#14 /srv/mediawiki/php-1.36.0-wmf.2/includes/parser/Parser.php(3340): ScribuntoHooks::invokeHook(Parser, PPTemplateFrame_Hash, array)
#15 /srv/mediawiki/php-1.36.0-wmf.2/includes/parser/Parser.php(3047): Parser->callParserFunction(PPTemplateFrame_Hash, string, array)
#16 /srv/mediawiki/php-1.36.0-wmf.2/includes/parser/PPFrame_Hash.php(253): Parser->braceSubstitution(array, PPTemplateFrame_Hash)
#17 /srv/mediawiki/php-1.36.0-wmf.2/includes/parser/Parser.php(3225): PPFrame_Hash->expand(PPNode_Hash_Tree)
#18 /srv/mediawiki/php-1.36.0-wmf.2/includes/parser/PPFrame_Hash.php(253): Parser->braceSubstitution(array, PPFrame_Hash)
#19 /srv/mediawiki/php-1.36.0-wmf.2/includes/parser/Parser.php(2887): PPFrame_Hash->expand(PPNode_Hash_Tree, integer)
#20 /srv/mediawiki/php-1.36.0-wmf.2/includes/parser/Parser.php(1556): Parser->replaceVariables(string)
#21 /srv/mediawiki/php-1.36.0-wmf.2/includes/parser/Parser.php(651): Parser->internalParse(string)
#22 /srv/mediawiki/php-1.36.0-wmf.2/includes/content/WikitextContent.php(374): Parser->parse(string, Title, ParserOptions, boolean, boolean, integer)
#23 /srv/mediawiki/php-1.36.0-wmf.2/includes/content/AbstractContent.php(590): WikitextContent->fillParserOutput(Title, integer, ParserOptions, boolean, ParserOutput)
#24 /srv/mediawiki/php-1.36.0-wmf.2/includes/Revision/RenderedRevision.php(266): AbstractContent->getParserOutput(Title, integer, ParserOptions, boolean)
#25 /srv/mediawiki/php-1.36.0-wmf.2/includes/Revision/RenderedRevision.php(235): MediaWiki\Revision\RenderedRevision->getSlotParserOutputUncached(WikitextContent, boolean)
#26 /srv/mediawiki/php-1.36.0-wmf.2/includes/Revision/RevisionRenderer.php(215): MediaWiki\Revision\RenderedRevision->getSlotParserOutput(string)
#27 /srv/mediawiki/php-1.36.0-wmf.2/includes/Revision/RevisionRenderer.php(152): MediaWiki\Revision\RevisionRenderer->combineSlotOutput(MediaWiki\Revision\RenderedRevision, array)
#28 [internal function]: MediaWiki\Revision\RevisionRenderer->MediaWiki\Revision\{closure}(MediaWiki\Revision\RenderedRevision, array)
#29 /srv/mediawiki/php-1.36.0-wmf.2/includes/Revision/RenderedRevision.php(197): call_user_func(Closure, MediaWiki\Revision\RenderedRevision, array)
#30 /srv/mediawiki/php-1.36.0-wmf.2/includes/poolcounter/PoolWorkArticleView.php(216): MediaWiki\Revision\RenderedRevision->getRevisionParserOutput()
#31 /srv/mediawiki/php-1.36.0-wmf.2/includes/poolcounter/PoolCounterWork.php(162): PoolWorkArticleView->doWork()
#32 /srv/mediawiki/php-1.36.0-wmf.2/includes/page/Article.php(811): PoolCounterWork->execute()
#33 /srv/mediawiki/php-1.36.0-wmf.2/includes/page/ImagePage.php(154): Article->view()
#34 /srv/mediawiki/php-1.36.0-wmf.2/includes/actions/ViewAction.php(74): ImagePage->view()
#35 /srv/mediawiki/php-1.36.0-wmf.2/includes/MediaWiki.php(527): ViewAction->show()
#36 /srv/mediawiki/php-1.36.0-wmf.2/includes/MediaWiki.php(313): MediaWiki->performAction(ImagePage, Title)
#37 /srv/mediawiki/php-1.36.0-wmf.2/includes/MediaWiki.php(940): MediaWiki->performRequest()
#38 /srv/mediawiki/php-1.36.0-wmf.2/includes/MediaWiki.php(543): MediaWiki->main()
#39 /srv/mediawiki/php-1.36.0-wmf.2/index.php(53): MediaWiki->run()
#40 /srv/mediawiki/php-1.36.0-wmf.2/index.php(46): wfIndexMain()
#41 /srv/mediawiki/w/index.php(3): require(string)
#42 {main}
Impact

This started happening (over a hundred in minutes) after I promoted train to group1 just now. I'm marking this as Unbreak Now and making it a train blocker, since this smells like it's going to happen a lot the more traffic this week's train is exposed to, so it should be fixed soon.

  1. Notes

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptWed, Jul 29, 1:25 PM
LarsWirzenius triaged this task as Unbreak Now! priority.Wed, Jul 29, 1:25 PM
Restricted Application added a subscriber: Mholloway. · View Herald TranscriptWed, Jul 29, 1:25 PM
Krinkle renamed this task from Locally stored wiki page has unsupported content model to Warning: Locally stored wiki page has unsupported content model (from JsonConfig).Wed, Jul 29, 3:20 PM
Krinkle edited Stack Trace. (Show Details)

Change 617163 had a related patch set uploaded (by MSantos; owner: MSantos):
[mediawiki/extensions/JsonConfig@master] Cache loadLocal shouldn't raise Exception

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

Mholloway added a subscriber: daniel.EditedWed, Jul 29, 7:30 PM

I believe this was triggered by this change: https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/core/+/a67cad6d0fc5b6a2aab08e5dd9b0391202ea3e9b

JsonConfig doesn't (yet) register its content handlers in extension.json, but rather handles them in a hook handler for ContentHandlerForModelID. As such, its content models aren't known to the ContentHandlerFactory. Previously, RevisionStore::loadSlotContent() would call ContentHandlerFactory::getContentHandler() without first checking if the content model was known, ContentHandlerFactory would first attempt to look up the handler from the known specs and fail, and it would then run the ContentHandlerForModelID hook, where JsonConfig would return the correct handler. Now, the new check short-circuits the hook used by ContentHandlerFactory as a fallback and instead returns a FallbackContent object for unregistered content models; and the error here is triggered when JsonConfig encounters a FallbackContent object, which isn't an instance of JCContent.

I think there's probably some work to be done to harmonize the new check with the behavior of ContentHandlerFactory (and maybe this wasn't the only extension relying on the fallback behavior?) but I think the easiest path forward to unbreak this is to update JsonConfig to register its content handlers in extension.json.

Change 617255 had a related patch set uploaded (by Mholloway; owner: Michael Holloway):
[mediawiki/extensions/JsonConfig@master] Register content handlers in extension.json

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

Change 617257 had a related patch set uploaded (by Mholloway; owner: Michael Holloway):
[mediawiki/extensions/Dashiki@master] Register content handler in extension.json

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

And T155144: unable to save json datasets using api.

The fundamental issue seems to be T155139: 'ContentHandlerForModelID' hook allows creating of handlers that aren't registered; based on discussion on that ticket, another possible approach to fixing this would be to have JsonConfig (and Dashiki) implement a getContentModels hook handler (that is, if there's some strong reason not to register their content handlers directly, as the discussion seems to imply there is).

I believe this was triggered by this change: https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/core/+/a67cad6d0fc5b6a2aab08e5dd9b0391202ea3e9b

JsonConfig doesn't (yet) register its content handlers in extension.json, but rather handles them in a hook handler for ContentHandlerForModelID. As such, its content models aren't known to the ContentHandlerFactory. Previously, RevisionStore::loadSlotContent() would call ContentHandlerFactory::getContentHandler() without first checking if the content model was known, ContentHandlerFactory would first attempt to look up the handler from the known specs and fail, and it would then run the ContentHandlerForModelID hook, where JsonConfig would return the correct handler. Now, the new check short-circuits the hook used by ContentHandlerFactory as a fallback and instead returns a FallbackContent object for unregistered content models; and the error here is triggered when JsonConfig encounters a FallbackContent object, which isn't an instance of JCContent.

I think there's probably some work to be done to harmonize the new check with the behavior of ContentHandlerFactory (and maybe this wasn't the only extension relying on the fallback behavior?) but I think the easiest path forward to unbreak this is to update JsonConfig to register its content handlers in extension.json.

Wouldn't it make sense to fix the ContentHandlerFactory / ContentHandlerForModelID hook so it works as it's supposed to? That seems like a breaking change that shouldn't have happened.

Maybe JsonConfig should implement the GetContentModels hook? That would ensure ContentHandlerFactory::isDefinedModel() returns true.

Change 617273 had a related patch set uploaded (by Legoktm; owner: Legoktm):
[mediawiki/extensions/JsonConfig@master] Implement GetContentModels hook

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

@Mholloway how does ^ look? I didn't test it. I think it's smaller and fits the expectations by ContentHandlerFactory (though I do think the core change might need to be re-evaluated).

Maybe JsonConfig should implement the GetContentModels hook? That would ensure ContentHandlerFactory::isDefinedModel() returns true.

Thanks, this is probably a better way to go, actually. I'll kick the tires and hit +2 if everything looks good.

(though I do think the core change might need to be re-evaluated)

Yeah, I went for the extension fix(es) in the first instance because I wasn't sure if this was part of a broader change in how content handlers are to be registered. I agree that some of these edge cases could use a bit more consideration. IMHO the ContentHandlerForModelID hook (usage) actually looks like a good candidate for deprecation and eventual removal, but that's going a bit out of scope for now, I think.

Change 617257 abandoned by Mholloway:
[mediawiki/extensions/Dashiki@master] Register content handler in extension.json

Reason:
Superseded by Ida2d2aa1dbd6e9e2041c1dbb351788a139eef0bb

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

Change 617255 abandoned by Mholloway:
[mediawiki/extensions/JsonConfig@master] Register content handlers in extension.json

Reason:
Superseded by Ida2d2aa1dbd6e9e2041c1dbb351788a139eef0bb

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

Change 617306 had a related patch set uploaded (by Mholloway; owner: Legoktm):
[mediawiki/extensions/JsonConfig@wmf/1.36.0-wmf.2] Implement GetContentModels hook

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

Change 617273 merged by jenkins-bot:
[mediawiki/extensions/JsonConfig@master] Implement GetContentModels hook

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

Change 617306 merged by jenkins-bot:
[mediawiki/extensions/JsonConfig@wmf/1.36.0-wmf.2] Implement GetContentModels hook

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

Mentioned in SAL (#wikimedia-operations) [2020-07-30T01:07:19Z] <mholloway-shell@deploy1001> Synchronized php-1.36.0-wmf.2/extensions/JsonConfig: Backport: Implement GetContentModels hook (T259126) (duration: 01m 07s)

Promoted back to group1, out of train window. Wanted to do this early so that if everything seems OK for a few hours, we (me or Brennen) can promote to group2 optimistic-opportunistically. If that fails, we roll back to group1 and try again on Monday if any problems found have been fixed.

This deployment's sound track was Queen's "We will rock you".

Change 617163 abandoned by MSantos:
[mediawiki/extensions/JsonConfig@master] Cache loadLocal unsupported model logging improvement

Reason:

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

Mholloway closed this task as Resolved.Thu, Jul 30, 11:51 AM

Thanks @Legoktm for the patch!