https://logstash.wikimedia.org/app/discover#/doc/logstash-*/logstash-deploy-1-7.0.0-1-2023.09.13?id=fGL6jooBjC2iVgTOZkaK
UnexpectedValueException: If $revId is 0, $content must be given. If we can't load the content from a revision, we have to stash it.

from /srv/mediawiki/php-1.41.0-wmf.26/includes/edit/SelserContext.php(32)
#0 /srv/mediawiki/php-1.41.0-wmf.26/includes/Rest/Handler/Helper/HtmlOutputRendererHelper.php(456): MediaWiki\Edit\SelserContext->__construct(Wikimedia\Parsoid\Core\PageBundle, integer, NULL)
#1 /srv/mediawiki/php-1.41.0-wmf.26/extensions/VisualEditor/includes/DirectParsoidClient.php(157): MediaWiki\Rest\Handler\Helper\HtmlOutputRendererHelper->getHtml()
#2 /srv/mediawiki/php-1.41.0-wmf.26/extensions/VisualEditor/includes/ApiParsoidTrait.php(110): MediaWiki\Extension\VisualEditor\DirectParsoidClient->getPageHtml(MediaWiki\Revision\RevisionStoreRecord, LanguageEn)
#3 /srv/mediawiki/php-1.41.0-wmf.26/extensions/VisualEditor/includes/ApiVisualEditor.php(231): MediaWiki\Extension\VisualEditor\ApiVisualEditor->requestRestbasePageHtml(MediaWiki\Revision\RevisionStoreRecord)
#4 /srv/mediawiki/php-1.41.0-wmf.26/includes/api/ApiMain.php(1929): MediaWiki\Extension\VisualEditor\ApiVisualEditor->execute()
#5 /srv/mediawiki/php-1.41.0-wmf.26/includes/api/ApiMain.php(906): ApiMain->executeAction()
#6 /srv/mediawiki/php-1.41.0-wmf.26/includes/api/ApiMain.php(877): ApiMain->executeActionWithErrorHandling()
#7 /srv/mediawiki/php-1.41.0-wmf.26/api.php(95): ApiMain->execute()
#8 /srv/mediawiki/php-1.41.0-wmf.26/api.php(48): wfApiMain()
#9 /srv/mediawiki/w/api.php(3): require(string)
#10 {main}

That's a very weird error for sure, but maybe the real problem is the fact that we're trying to show a visual diff on a .js page? That should not happen.

In T346252#9164203, @matmarex wrote:

That's a very weird error for sure, but maybe the real problem is the fact that we're trying to show a visual diff on a .js page? That should not happen.

Yes. I don't understand why the wikitext/visual switcher appears, for example, on this diff https://www.mediawiki.org/w/index.php?title=MediaWiki:Gadget-BugStatusUpdate.js&diff=prev&oldid=1408376 but not on this diff https://www.mediawiki.org/w/index.php?title=MediaWiki:Gadget-BugStatusUpdate.js&diff=next&oldid=1408378.

This error has also started happening for wikidata with 1.41.0-wmf.26, one example:

• service.version: 1.41.0-wmf.26
• trace.id: 21c29b04-f913-4589-a278-0e43c8b4703a
• Find trace.id in Logstash

[{reqId}] {exception_url}   UnexpectedValueException: If $revId is 0, $content must be given. If we can't load the content from a revision, we have to stash it.

from /srv/mediawiki/php-1.41.0-wmf.26/includes/edit/SelserContext.php(32)
#0 /srv/mediawiki/php-1.41.0-wmf.26/includes/Rest/Handler/Helper/HtmlOutputRendererHelper.php(456): MediaWiki\Edit\SelserContext->__construct(Wikimedia\Parsoid\Core\PageBundle, integer, NULL)
#1 /srv/mediawiki/php-1.41.0-wmf.26/extensions/VisualEditor/includes/DirectParsoidClient.php(157): MediaWiki\Rest\Handler\Helper\HtmlOutputRendererHelper->getHtml()
#2 /srv/mediawiki/php-1.41.0-wmf.26/extensions/VisualEditor/includes/ApiParsoidTrait.php(110): MediaWiki\Extension\VisualEditor\DirectParsoidClient->getPageHtml(MediaWiki\Revision\RevisionStoreRecord, LanguageEn)
#3 /srv/mediawiki/php-1.41.0-wmf.26/extensions/VisualEditor/includes/ApiVisualEditor.php(231): MediaWiki\Extension\VisualEditor\ApiVisualEditor->requestRestbasePageHtml(MediaWiki\Revision\RevisionStoreRecord)
#4 /srv/mediawiki/php-1.41.0-wmf.26/includes/api/ApiMain.php(1929): MediaWiki\Extension\VisualEditor\ApiVisualEditor->execute()
#5 /srv/mediawiki/php-1.41.0-wmf.26/includes/api/ApiMain.php(906): ApiMain->executeAction()
#6 /srv/mediawiki/php-1.41.0-wmf.26/includes/api/ApiMain.php(877): ApiMain->executeActionWithErrorHandling()
#7 /srv/mediawiki/php-1.41.0-wmf.26/api.php(95): ApiMain->execute()
#8 /srv/mediawiki/php-1.41.0-wmf.26/api.php(48): wfApiMain()
#9 /srv/mediawiki/w/api.php(3): require(string)
#10 {main}

So far the issue doesn't seem to impact any major wikis in group2, so the impact for users seems small.

In T346252#9165504, @dom_walden wrote:

In T346252#9164203, @matmarex wrote:

That's a very weird error for sure, but maybe the real problem is the fact that we're trying to show a visual diff on a .js page? That should not happen.

Yes. I don't understand why the wikitext/visual switcher appears, for example, on this diff https://www.mediawiki.org/w/index.php?title=MediaWiki:Gadget-BugStatusUpdate.js&diff=prev&oldid=1408376 but not on this diff https://www.mediawiki.org/w/index.php?title=MediaWiki:Gadget-BugStatusUpdate.js&diff=next&oldid=1408378.

Oh, I see what's going on with that page now – some of its revisions have the wikitext content model instead of javascript, because it was originally created in a different namespace and then moved. It's not obvious in the interface, but you can look up the details in the API: https://www.mediawiki.org/wiki/Special:ApiSandbox#action=query&format=json&prop=revisions&titles=MediaWiki:Gadget-BugStatusUpdate.js&formatversion=2&rvprop=ids|timestamp|contentmodel|comment&rvslots=main&rvlimit=max

So we have four cases to consider:

1. Diff between wikitext and wikitext: https://www.mediawiki.org/w/index.php?title=MediaWiki:Gadget-BugStatusUpdate.js&diff=prev&oldid=1408376
   • Visual diff option appears and works, as expected
2. Diff between javascript and javascript: https://www.mediawiki.org/w/index.php?title=MediaWiki:Gadget-BugStatusUpdate.js&diff=next&oldid=1408378
   • Visual diff option does not appear (as expected), but we attempt to load the visual diff anyway (not expected), which fails with this exception
3. Diff between javascript and wikitext: https://www.mediawiki.org/w/index.php?title=MediaWiki:Gadget-BugStatusUpdate.js&diff=next&oldid=1408359
   • Visual diff option appears (not expected) and fails as above
4. Diff between wikitext and javascript: https://www.mediawiki.org/w/index.php?title=MediaWiki:Gadget-BugStatusUpdate.js&diff=next&oldid=1408376
   • (behaves the same as #2)

There was a recent change to the loading of visual diffs: https://gerrit.wikimedia.org/r/c/mediawiki/extensions/VisualEditor/ /953958 which may be the cause of some of these problems.

Change 957725 had a related patch set uploaded (by Bartosz Dziewoński; author: Bartosz Dziewoński):

[mediawiki/extensions/VisualEditor@master] Don't offer visual diffs for non-wikitext pages

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

It looks like this can't be solved properly without some changes to the diffs in MediaWiki core, and it's not obvious to me how to do that, so this patch is a bit of a workaround. It will avoid the errors in the simple cases like #2 in my list in the previous comment (e.g. browsing diffs of JavaScript pages, or browsing diffs on entities on Wikidata). It will still fail in cases #3 and #4 (depending on the scenario, you might get the error, or get no option to view a visual diff when there should be one), but these are very rare. There are similar bugs in MediaWiki itself (e.g. T214217).

https://logstash.wikimedia.org/goto/c8f0526248d710342906efffff5a80e0

1 to @matmarex's debugging theory. I would lean towards reverting my patch because ext.visualEditor.diffPage.init doesn't need to be loaded on any page other than a wikitext->wikitext diff. The other issue I was trying to solve is caused by RevisionSlider trying to do a clever partial reload of the page, when this isn't how MediaWiki is designed. There simply might not be a nice way to hook into VE logic to show or hide the interface without fully reloading the page, but we can look at other workarounds for that problem again in the other task.

Change 957725 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Don't offer visual diffs for non-wikitext pages

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

In T346252#9166456, @awight wrote:

I would lean towards reverting my patch because ext.visualEditor.diffPage.init doesn't need to be loaded on any page other than a wikitext->wikitext diff.

I would've been fine with that as well, but my other patch got merged, so let's roll with that? I'm going to backport it later today – the issue is mostly harmless, but I want to backport to avoid upsetting people with scary error messages (both our users and our engineers ;) ).

Change 957399 had a related patch set uploaded (by Bartosz Dziewoński; author: Bartosz Dziewoński):

[mediawiki/extensions/VisualEditor@wmf/1.41.0-wmf.26] Don't offer visual diffs for non-wikitext pages

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

Change 957399 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@wmf/1.41.0-wmf.26] Don't offer visual diffs for non-wikitext pages

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

Mentioned in SAL (#wikimedia-operations) [2023-09-14T20:32:25Z] <thcipriani@deploy1002> Started scap: Backport for [[gerrit:957399|Don't offer visual diffs for non-wikitext pages (T346252)]], [[gerrit:957400|ThreadItemStore: Add details to row insertion exceptions (T343859)]]

Mentioned in SAL (#wikimedia-operations) [2023-09-14T20:33:57Z] <thcipriani@deploy1002> thcipriani and matmarex: Backport for [[gerrit:957399|Don't offer visual diffs for non-wikitext pages (T346252)]], [[gerrit:957400|ThreadItemStore: Add details to row insertion exceptions (T343859)]] synced to the testservers mwdebug2002.codfw.wmnet, mwdebug1001.eqiad.wmnet, mwdebug2001.codfw.wmnet, mwdebug1002.eqiad.wmnet, and mw-debug kubernetes deployment (accessible via k8s-experimental XWD

Mentioned in SAL (#wikimedia-operations) [2023-09-14T20:45:01Z] <thcipriani@deploy1002> Finished scap: Backport for [[gerrit:957399|Don't offer visual diffs for non-wikitext pages (T346252)]], [[gerrit:957400|ThreadItemStore: Add details to row insertion exceptions (T343859)]] (duration: 12m 35s)

See also T346369: Diff-type selection resets back to "wikitext" when using RevisionSlider, which makes me think that the logic flow should be:

• mw-core calls extension hooks with a signature like, getSupportedDiffTypes( Revision $revFrom, Revision $revTo, string[] &$diffTypes ) and VE adds 'visual' when appropriate. There might also be a DiffType class which provides its label, a handler, etc.
• mw-core presents a diff type selector when more than one type is available.
• core calls out to an extension handler to show custom diff content (diff page rendering needs additional fixes so I won't suggest anything too concrete here).