Page MenuHomePhabricator

Cannot render diffs where pages are converted from wikitext to MassMessageListContent
Closed, DuplicatePublic

Description

{"id":"WIjz8wpAEKYAAFUViYUAAABJ","type":"Exception","file":"/srv/mediawiki/php-1.29.0-wmf.9/extensions/MassMessage/includes/content/MassMessageListDiffEngine.php","line":20,"message":"Cannot diff content types other than MassMessageListContent","code":0,"

Replication steps

  • Install MassMessage and make sure it shows up in Special:Version

In desktop:

Developer notes

The problem is with any diff where the content-model of the page is changed. MassMessageListDiffEngine as it says in its own exception ''Cannot diff content types other than MassMessageListContent''. It should be special cased to deal with the situation where revision A is a different content model than MassMessageListContent and revision B is a MassMessageListContent.

T156293#4595478 suggests this exception should be caught in mobile. While that's true, the exception displays on desktop as well as mobile:

Event Timeline

Matanya created this task.Jan 25 2017, 7:32 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJan 25 2017, 7:32 PM
mmodell added a subscriber: mmodell.EditedJan 25 2017, 8:39 PM

Seems like this only happens on that specific revision because it is actually invalid.

mmodell triaged this task as High priority.Jan 25 2017, 9:37 PM

re-added as blocker because it's still showing up in logs, I think I misunderstood the issue here

Also can someone provide the full backtrace please?

Legoktm: I couldn't get a backtrace and I'm not seeing the fatals anymore either.

Since I can no longer reproduce this one I'm removing blocking status.

Krinkle added a subscriber: Krinkle.

Viewing https://www.mediawiki.org/wiki/Special:MobileDiff/2234116 does fatals consistently (today).

Error
[{exception_id}] {exception_url}   Exception from line 27 of /srv/mediawiki/php-1.32.0-wmf.22/extensions/MassMessage/includes/content/MassMessageListDiffEngine.php: Cannot diff content types other than MassMessageListContent

#0 /srv/mediawiki/php-1.32.0-wmf.22/includes/diff/DifferenceEngineSlotDiffRenderer.php(58): MediaWiki\MassMessage\MassMessageListDiffEngine->generateContentDiffBody(WikitextContent, MediaWiki\MassMessage\MassMessageListContent)
#1 /srv/mediawiki/php-1.32.0-wmf.22/includes/diff/DifferenceEngine.php(1054): DifferenceEngineSlotDiffRenderer->getDiff(WikitextContent, MediaWiki\MassMessage\MassMessageListContent)
#2 /srv/mediawiki/php-1.32.0-wmf.22/includes/diff/DifferenceEngine.php(972): DifferenceEngine->getDiffBody()
#3 /srv/mediawiki/php-1.32.0-wmf.22/includes/diff/DifferenceEngine.php(937): DifferenceEngine->getDiff(string, string, string)
#4 /srv/mediawiki/php-1.32.0-wmf.22/includes/diff/DifferenceEngine.php(706): DifferenceEngine->showDiff(string, string, string)
#5 /srv/mediawiki/php-1.32.0-wmf.22/extensions/MobileFrontend/includes/specials/SpecialMobileDiff.php(160): DifferenceEngine->showDiffPage()
#6 /srv/mediawiki/php-1.32.0-wmf.22/extensions/MobileFrontend/includes/specials/SpecialMobileDiff.php(132): SpecialMobileDiff->displayDiffPage()
#7 /srv/mediawiki/php-1.32.0-wmf.22/extensions/MobileFrontend/includes/specials/MobileSpecialPage.php(58): SpecialMobileDiff->executeWhenAvailable(string)
#8 /srv/mediawiki/php-1.32.0-wmf.22/includes/specialpage/SpecialPage.php(569): MobileSpecialPage->execute(string)
#9 /srv/mediawiki/php-1.32.0-wmf.22/includes/specialpage/SpecialPageFactory.php(581): SpecialPage->run(string)
#10 /srv/mediawiki/php-1.32.0-wmf.22/includes/MediaWiki.php(288): MediaWiki\Special\SpecialPageFactory->executePath(Title, RequestContext)
#11 /srv/mediawiki/php-1.32.0-wmf.22/includes/MediaWiki.php(868): MediaWiki->performRequest()
#12 /srv/mediawiki/php-1.32.0-wmf.22/includes/MediaWiki.php(525): MediaWiki->main()
#13 /srv/mediawiki/php-1.32.0-wmf.22/index.php(42): MediaWiki->run()
Krinkle renamed this task from mass message fatals on Cannot diff content types other than MassMessageListContent to Unable to view certain mobile diffs "Exception: Cannot diff content types other than MassMessageListContent".Sep 18 2018, 8:15 PM
Krinkle added a project: Editing-team.
Krinkle edited projects, added MobileFrontend; removed Editing-team.Nov 7 2018, 4:25 AM
Krinkle added a comment.EditedNov 7 2018, 4:30 AM

If I understand correctly, viewing differences for this content-type isn't currently supported on MobileDiff (or in MassMessage).

As such, it isn't so much that something we wrote is broken, rather, it's attempting to do something it shouldn't – hence it currently fails with an application error which gets wrongly attributed to software and cluster stability (an has the risk of aborting random deployments and alerting SRE when users try it too often).

The simplest counter-measure for the short term would be to catch these exceptions in SpecialMobileDiff and print an error message to the user indicating that "viewing differences for this content type isn't currently supported".

The main point being that it gets acknowledged as a client error (performing an action not currently supported, also with the user understanding why this one failed and not other ones, and the error gets localised), instead of a server error (which can triggers alerts).

Jdlrobson added a subscriber: Jdlrobson.

I think we had similar issues with Flow.

CCicalese_WMF added a subscriber: CCicalese_WMF.

From reading through the discussion, this does not seem to have anything to do with Multi-Content Revisions, so I am untagging. If I missed something, please feel free to re-tag and explain.

Jdlrobson added a comment.EditedNov 8 2018, 5:50 PM

@Legoktm @Krinkle is there any easy way to replicate this locally? What is the easiest way to create a local page (and thus diff) for a page with a different content type? I must confess I've never done this before.

Once I can replicate I can push up a fix.

Jdlrobson updated the task description. (Show Details)Nov 12 2018, 10:02 PM
Jdlrobson renamed this task from Unable to view certain mobile diffs "Exception: Cannot diff content types other than MassMessageListContent" to Cannot render diffs where pages are converted from wikitext to MassMessageListContent .Nov 12 2018, 10:06 PM
Jdlrobson updated the task description. (Show Details)

This issue is also on the desktop version of MassMessage so should be fixed inside that extension.

@Legoktm @Krinkle is there any easy way to replicate this locally? What is the easiest way to create a local page (and thus diff) for a page with a different content type? I must confess I've never done this before.
Once I can replicate I can push up a fix.

I don't know the answer to #1, but to change content models for an existing page an admin can use [[Special:ChangeContentModel]] (or with the user-right editcontentmodel).

It's okay I worked out how to replicate (see updated description)

The simplest counter-measure for the short term would be to catch these exceptions in SpecialMobileDiff and print an error message to the user indicating that "viewing differences for this content type isn't currently supported".

Or redirect to a functioning (non-MobileFrontend) diff view.

@Nemo_bis like this broken desktop view?
https://www.mediawiki.org/w/index.php?title=User%3AQuiddity%2Fdemomodel&type=revision&diff=2234116&oldid=2234115

To be clear this is not a mobilefrontend bug as it impacts desktop diffs as well. It is a MassMessage bug and the fix lies inside MassMessage.

Tgr added a comment.Jan 22 2019, 8:03 AM

Funnily enough this depends on which direction you look at it . https://www.mediawiki.org/w/index.php?title=User%3AQuiddity%2Fdemomodel&type=revision&diff=2234115&oldid=2234116 works fine (although not exactly useful as it compares with the JSON representation of the MassMessage list).

MassMessageListContent is a JsonContent subclass (which in turn is a TextContent subclass) and the diff renderer is created from the right-side content, so when the left side is massmessage content and the right side is wikitext or another text content, TextSlotDiffRenderer will be used and that happily compares with another TextContent. When the sides are switched, MassMessageListDiffEngine is asked to do the comparison and that refuses to work with any other content type.

The fact that JsonContent is a TextContent annoys me so much.

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