(rt-testing) InvalidArgumentException: Multiple conflicting values given for wgKartographerLiveData
Closed, ResolvedPublicBUG REPORT
Actions

Assigned To

Authored By

	ssastry
	Mar 4 2022, 12:12 AM

Description

Error

mwversion: 1.38.0-wmf.24
reqId: YiFYKPkvjv167MwgPULw0wAAAA8
Find reqId in Logstash

normalized_message

[{reqId}] {exception_url}   InvalidArgumentException: Multiple conflicting values given for wgKartographerLiveData

exception.trace

from /srv/mediawiki/php-1.38.0-wmf.24/includes/parser/ParserOutput.php(1189)
#0 /srv/mediawiki/php-1.38.0-wmf.24/includes/parser/ParserOutput.php(2216): ParserOutput->setJsConfigVar(string, stdClass)
#1 /srv/parsoid-testing/extension/src/Config/DataAccess.php(327): ParserOutput->collectMetadata(ParserOutput)
#2 /srv/parsoid-testing/src/Wt2Html/TT/ExtensionHandler.php(136): MWParsoid\Config\DataAccess->parseWikitext(MWParsoid\Config\PageConfig, ParserOutput, string)
#3 /srv/parsoid-testing/src/Wt2Html/TT/ExtensionHandler.php(277): Wikimedia\Parsoid\Wt2Html\TT\ExtensionHandler->onExtension(Wikimedia\Parsoid\Tokens\SelfclosingTagTk)
#4 /srv/parsoid-testing/src/Wt2Html/TT/TokenHandler.php(150): Wikimedia\Parsoid\Wt2Html\TT\ExtensionHandler->onTag(Wikimedia\Parsoid\Tokens\SelfclosingTagTk)
#5 /srv/parsoid-testing/src/Wt2Html/TokenTransformManager.php(132): Wikimedia\Parsoid\Wt2Html\TT\TokenHandler->process(array)
#6 /srv/parsoid-testing/src/Wt2Html/TokenTransformManager.php(185): Wikimedia\Parsoid\Wt2Html\TokenTransformManager->processChunk(array)
#7 /srv/parsoid-testing/src/Wt2Html/ParserPipeline.php(140): Wikimedia\Parsoid\Wt2Html\TokenTransformManager->process(array, array)
#8 /srv/parsoid-testing/src/Utils/PipelineUtils.php(105): Wikimedia\Parsoid\Wt2Html\ParserPipeline->parse(string, array)
#9 /srv/parsoid-testing/src/Wt2Html/TT/TemplateHandler.php(597): Wikimedia\Parsoid\Utils\PipelineUtils::processContentInPipeline(Wikimedia\Parsoid\Config\Env, Wikimedia\Parsoid\Wt2Html\PageConfigFrame, string, array)
#10 /srv/parsoid-testing/src/Wt2Html/TT/TemplateHandler.php(1114): Wikimedia\Parsoid\Wt2Html\TT\TemplateHandler->processTemplateSource(array, array, string)
#11 /srv/parsoid-testing/src/Wt2Html/TT/TemplateHandler.php(1164): Wikimedia\Parsoid\Wt2Html\TT\TemplateHandler->onTemplate(Wikimedia\Parsoid\Tokens\SelfclosingTagTk)
#12 /srv/parsoid-testing/src/Wt2Html/TT/TokenHandler.php(150): Wikimedia\Parsoid\Wt2Html\TT\TemplateHandler->onTag(Wikimedia\Parsoid\Tokens\SelfclosingTagTk)
#13 /srv/parsoid-testing/src/Wt2Html/TokenTransformManager.php(132): Wikimedia\Parsoid\Wt2Html\TT\TokenHandler->process(array)
#14 /srv/parsoid-testing/src/Wt2Html/TokenTransformManager.php(195): Wikimedia\Parsoid\Wt2Html\TokenTransformManager->processChunk(array)
#15 /srv/parsoid-testing/src/Wt2Html/TokenTransformManager.php(193): Wikimedia\Parsoid\Wt2Html\TokenTransformManager->processChunkily(string, array)
#16 /srv/parsoid-testing/src/Wt2Html/TreeBuilder/TreeBuilderStage.php(487): Wikimedia\Parsoid\Wt2Html\TokenTransformManager->processChunkily(string, array)
#17 [internal function]: Wikimedia\Parsoid\Wt2Html\TreeBuilder\TreeBuilderStage->processChunkily(string, array)
#18 /srv/parsoid-testing/src/Wt2Html/DOMPostProcessor.php(903): Generator->current()
#19 /srv/parsoid-testing/src/Wt2Html/ParserPipeline.php(180): Wikimedia\Parsoid\Wt2Html\DOMPostProcessor->processChunkily(string, array)
#20 /srv/parsoid-testing/src/Wt2Html/ParserPipelineFactory.php(308): Wikimedia\Parsoid\Wt2Html\ParserPipeline->parseChunkily(string, array)
#21 /srv/parsoid-testing/src/Wikitext/ContentModelHandler.php(122): Wikimedia\Parsoid\Wt2Html\ParserPipelineFactory->parse(string)
#22 /srv/parsoid-testing/src/Parsoid.php(172): Wikimedia\Parsoid\Wikitext\ContentModelHandler->toDOM(Wikimedia\Parsoid\Ext\ParsoidExtensionAPI)
#23 /srv/parsoid-testing/src/Parsoid.php(210): Wikimedia\Parsoid\Parsoid->parseWikitext(MWParsoid\Config\PageConfig, ParserOutput, array)
#24 /srv/parsoid-testing/extension/src/Rest/Handler/ParsoidHandler.php(586): Wikimedia\Parsoid\Parsoid->wikitext2html(MWParsoid\Config\PageConfig, array, NULL, ParserOutput)
#25 /srv/parsoid-testing/extension/src/Rest/Handler/TransformHandler.php(119): MWParsoid\Rest\Handler\ParsoidHandler->wt2html(MWParsoid\Config\PageConfig, array, string)
#26 /srv/mediawiki/php-1.38.0-wmf.24/includes/Rest/Router.php(414): MWParsoid\Rest\Handler\TransformHandler->execute()
#27 /srv/mediawiki/php-1.38.0-wmf.24/includes/Rest/Router.php(338): MediaWiki\Rest\Router->executeHandler(MWParsoid\Rest\Handler\TransformHandler)
#28 /srv/mediawiki/php-1.38.0-wmf.24/includes/Rest/EntryPoint.php(167): MediaWiki\Rest\Router->execute(MediaWiki\Rest\RequestFromGlobals)
#29 /srv/mediawiki/php-1.38.0-wmf.24/includes/Rest/EntryPoint.php(132): MediaWiki\Rest\EntryPoint->execute()
#30 /srv/mediawiki/php-1.38.0-wmf.24/rest.php(31): MediaWiki\Rest\EntryPoint::main()
#31 /srv/mediawiki/w/rest.php(3): require(string)
#32 {main}

Impact

Notes

Details

Request URL: https://en.wikivoyage.org/w/rest.php/en.wikivoyage.org/v3/transform/wikitext/to/pagebundle/Tianjin

Related Changes in Gerrit:

	Subject	Repo	Branch	Lines +/-
	Still collect metadata on multiple writes	mediawiki/core	master	+8 -4
	ParserOutput::collectMetadata: Suppress hard failures from Parsoid	mediawiki/core	master	+14 -0

Customize query in gerrit

Related Objects
Search...

		Status	Subtype	Assigned	Task
		Open		None	T303630 ParserAfterParse hook can't be supported in Parsoid
		Resolved	BUG REPORT	cscott	T303015 (rt-testing) InvalidArgumentException: Multiple conflicting values given for wgKartographerLiveData

Event Timeline

ssastry created this task.Mar 4 2022, 12:12 AM

ssastry triaged this task as High priority.

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 4 2022, 12:12 AM

SunAfterRain changed Request URL from https://en.wikivoyage.orghttp//en.wikivoyage.org/w/rest.php/en.wikivoyage.org/v3/transform/wikitext/to/pagebundle/Tianjin to https://en.wikivoyage.org/w/rest.php/en.wikivoyage.org/v3/transform/wikitext/to/pagebundle/Tianjin.Mar 4 2022, 8:56 AM

We reverted the patch from master. Scott is going to fix up and test the code before we merge those patches again. So resolving this.

ssastry closed this task as Resolved.Mar 9 2022, 1:02 AM

ssastry claimed this task.

Reopening, because this is a more fundamental bug w/ our parsing strategy which CMC just happens to expose.

So this is probably an actual rendering bug in Parsoid, and an artifact of how Parsoid handles "fall back extension tags". Parsoid calls Parser::parseExtensionTagAsTopLevelDoc() here multiple times, once for each instance of <maplink> or <mapframe> on the page. Kartographer hooks the ParserAfterParse hook (which calls TagHandler::finalParseStep) in order to do post processing of the page, based on accumulated state it fetches from json_decode(ParserOutput::getExtensionData( 'kartographer' ) ).

So there's a fundamental issue here which is that this is deeply sequential. It assumes that as each tag is parsed it is going to deserialize an object from extension data, mutate it, and then reserialize it and re-store it in extension data. It then deserializes this object one final time in ParserAfterParse to update whole-page properties (like jsConfigVars).

Kartographer can probably make this asynchronous -- you'd start by changing the Kartographer State object to store more fine-grained data under multiple keys in the ParserOutput extension data, not just a monolithic serialized object, and then you'd ensure that each of those State properties was composable. (They mostly seem to be in practice, there are a bunch of set-once fields and each map added dumps data for itself.)

But the way Parsoid is calling ParserAfterParse after each map is *not* compatible here, because (both before the CMC patches and after) we call Parser::resetOutput() between each call to Parser::startExternalParse *and* we call ParserAfterParse after each extension tag. So Parsoid is overwriting the Kartographer state and only the "last" map is ever going to have it jsconfigvars set, etc.

Kartographer could/should probably use the OutputPageParserOutput hook instead of ParserAfterParse, since that's a better "after all the parsing is done, honest!" hook to use. But probably any/everything which uses the ParserAfterParse hook is going to break to some degree with Parsoid if there is more than one instance of that tag on the page.

(tl;dr Kartographer/Graph under Parsoid *always* overwrote/discarded the data from the first tag instance when a second kartographer/graph tag was encountered on the page; the CMC patches just made the overwrite obvious by complaining about it.)

cscott mentioned this in T303630: ParserAfterParse hook can't be supported in Parsoid.Mar 11 2022, 5:32 PM

cscott added a parent task: T303630: ParserAfterParse hook can't be supported in Parsoid.

Change 770016 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/core@master] ParserOutput::collectMetadata: Suppress hard failures from Parsoid

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

gerritbot added a project: Patch-For-Review.Mar 11 2022, 5:52 PM

Change 770016 merged by jenkins-bot:

[mediawiki/core@master] ParserOutput::collectMetadata: Suppress hard failures from Parsoid

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

ReleaseTaggerBot added a project: MW-1.38-notes (1.38.0-wmf.26; 2022-03-14).Mar 12 2022, 2:00 AM

Maintenance_bot removed a project: Patch-For-Review.Mar 12 2022, 2:10 AM

We knew this about Graph as part of T272942: Make Graph extension compatible with Parsoid where Isabelle submitted her patch to avoid clobbering config vars. We didn't know that Maps was doing this but presumably the work on T263762: Make Kartographer extension compatible with Parsoid will fix it and you have good suggestions for @MSantos above.