Page MenuHomePhabricator

Fatal MWException when trying to rename a page
Closed, ResolvedPublicPRODUCTION ERROR

Description

Error message
[8eacd097-c049-480a-a068-9bc12fef99e4] 2020-10-15 13:03:42: Végzetes kivétel: „MWException”
Impact

Unable to rename the page.

Notes
  • This is about renaming one particular page by one particular user. This user is a newbie and doesn’t want to rename any other page, so I don’t know how widespread the issue is. Also, because it’s not me, I may not be able to answer all questions, but I’ll try my best to get the answers from that user.
  • This particular request is from Thursday (before T263179 hit huwiki), but the same error message still appears. I don’t have a newer request ID at hand, but I can ask for it if needed.

Details

Request ID
8eacd097-c049-480a-a068-9bc12fef99e4
Request URL
https://hu.wikipedia.org/w/index.php?title=Speci%C3%A1lis:Lap_%C3%A1tnevez%C3%A9se&action=submit
Stack Trace
[8eacd097-c049-480a-a068-9bc12fef99e4] /w/index.php?title=Speci%C3%A1lis:Lap_%C3%A1tnevez%C3%A9se&action=submit   MWException from line 832 of /srv/mediawiki/php-1.36.0-wmf.11/includes/MovePage.php: Failed to delete page-move revision: This action has been automatically identified as harmful, and therefore disallowed.
If you believe your action was constructive, please inform an administrator of what you were trying to do.
A brief description of the abuse rule which your action matched is: Szócikk teljes tartalmának eltávolítása
#0 /srv/mediawiki/php-1.36.0-wmf.11/includes/MovePage.php(575): MovePage->moveToInternal(User, Title, string, boolean, array)
#1 /srv/mediawiki/php-1.36.0-wmf.11/includes/MovePage.php(421): MovePage->moveUnsafe(User, string, boolean, array)
#2 /srv/mediawiki/php-1.36.0-wmf.11/includes/specials/SpecialMovepage.php(658): MovePage->moveIfAllowed(User, string, boolean)
#3 /srv/mediawiki/php-1.36.0-wmf.11/includes/specials/SpecialMovepage.php(137): MovePageForm->doSubmit()
#4 /srv/mediawiki/php-1.36.0-wmf.11/includes/specialpage/SpecialPage.php(600): MovePageForm->execute(NULL)
#5 /srv/mediawiki/php-1.36.0-wmf.11/includes/specialpage/SpecialPageFactory.php(709): SpecialPage->run(NULL)
#6 /srv/mediawiki/php-1.36.0-wmf.11/includes/MediaWiki.php(307): MediaWiki\SpecialPage\SpecialPageFactory->executePath(Title, RequestContext)
#7 /srv/mediawiki/php-1.36.0-wmf.11/includes/MediaWiki.php(940): MediaWiki->performRequest()
#8 /srv/mediawiki/php-1.36.0-wmf.11/includes/MediaWiki.php(543): MediaWiki->main()
#9 /srv/mediawiki/php-1.36.0-wmf.11/index.php(53): MediaWiki->run()
#10 /srv/mediawiki/php-1.36.0-wmf.11/index.php(46): wfIndexMain()
#11 /srv/mediawiki/w/index.php(3): require(string)
#12 {main}

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 17 2020, 12:04 AM
Tgr added a subscriber: Tgr.

Seems like an AbuseFilter problem:

8eacd097-c049-480a-a068-9bc12fef99e4] /w/index.php?title=Speci%C3%A1lis:Lap_%C3%A1tnevez%C3%A9se&action=submit   MWException from line 832 of /srv/mediawiki/php-1.36.0-wmf.11/includes/MovePage.php: Failed to delete page-move revision: This action has been automatically identified as harmful, and therefore disallowed.
If you believe your action was constructive, please inform an administrator of what you were trying to do.
A brief description of the abuse rule which your action matched is: Szócikk teljes tartalmának eltávolítása
#0 /srv/mediawiki/php-1.36.0-wmf.11/includes/MovePage.php(575): MovePage->moveToInternal(User, Title, string, boolean, array)
#1 /srv/mediawiki/php-1.36.0-wmf.11/includes/MovePage.php(421): MovePage->moveUnsafe(User, string, boolean, array)
#2 /srv/mediawiki/php-1.36.0-wmf.11/includes/specials/SpecialMovepage.php(658): MovePage->moveIfAllowed(User, string, boolean)
#3 /srv/mediawiki/php-1.36.0-wmf.11/includes/specials/SpecialMovepage.php(137): MovePageForm->doSubmit()
#4 /srv/mediawiki/php-1.36.0-wmf.11/includes/specialpage/SpecialPage.php(600): MovePageForm->execute(NULL)
#5 /srv/mediawiki/php-1.36.0-wmf.11/includes/specialpage/SpecialPageFactory.php(709): SpecialPage->run(NULL)
#6 /srv/mediawiki/php-1.36.0-wmf.11/includes/MediaWiki.php(307): MediaWiki\SpecialPage\SpecialPageFactory->executePath(Title, RequestContext)
#7 /srv/mediawiki/php-1.36.0-wmf.11/includes/MediaWiki.php(940): MediaWiki->performRequest()
#8 /srv/mediawiki/php-1.36.0-wmf.11/includes/MediaWiki.php(543): MediaWiki->main()
#9 /srv/mediawiki/php-1.36.0-wmf.11/index.php(53): MediaWiki->run()
#10 /srv/mediawiki/php-1.36.0-wmf.11/index.php(46): wfIndexMain()
#11 /srv/mediawiki/w/index.php(3): require(string)
#12 {main}

Of course, it should be shown as a normal error, not as an exception.

JJMC89 updated the task description. (Show Details)Oct 17 2020, 12:55 AM
JJMC89 edited Stack Trace. (Show Details)
JJMC89 edited Stack Trace. (Show Details)

And it should not match the filter in the first place—in the 10–11th lines, the filter tries to exclude redirections/renames, which is the attempted action here. (I’m not sure if it’s a bug in the filter or in the extension, but the filter looks good for me at first sight.)

Ammarpad added a subscriber: Ammarpad.EditedOct 17 2020, 5:34 AM

Seems like an AbuseFilter problem:
Of course, it should be shown as a normal error, not as an exception.

It does not seem to be AbuseFilter fault. AbuseFilter uses ArticleDeleteHook to filter deletion action (in this case). It founds the deletion should be disallowed (as configured) and the hook requires boolean return value to decide on whether to proceed with the deletion or not. So it's obvious, the only option for AbuseFilter is to return false to abort the deletion. It also sets the correct error message by modifying the supplied error variable in the hook. That's all it could do.

So the problem lies in MovePage, with its logic that if target page deletion fails, then exception will follow

MovePage.php
$status = $newpage->doDeleteArticleReal(...)
if ( !$status->isGood() ) {
     throw new MWException( 
     'Failed to delete page-move revision: ' . $status->getWikiText( false, false, 'en' )  );
}
Daimona added a subscriber: Daimona.

Seems like an AbuseFilter problem:
Of course, it should be shown as a normal error, not as an exception.

It does not seem to be AbuseFilter fault. AbuseFilter uses ArticleDeleteHook to filter deletion action (in this case). It founds the deletion should be disallowed (as configured) and the hook requires boolean return value to decide on whether to proceed with the deletion or not. So it's obvious, the only option for AbuseFilter is to return false to abort the deletion. It also sets the correct error message by modifying the supplied error variable in the hook. That's all it could do.

So the problem lies in MovePage, with its logic that if target page deletion fails, then exception will follow

MovePage.php
$status = $newpage->doDeleteArticleReal(...)
if ( !$status->isGood() ) {
     throw new MWException( 
     'Failed to delete page-move revision: ' . $status->getWikiText( false, false, 'en' )  );
}

I agree with every single word here. MW core is throwing an unhandled exception if the deletion fails, which is certainly not an impossible scenario (and can have lots of different causes, not only AbuseFilter).

Of note, this is not the first time I see a bug report about this issue, but no matter how hard I tried, I cannot find a duplicate.

SpecialMovePage is also calling doDeleteArticleReal and shows the error message (thats happen when the sysop is using the move-and-delete option and that fails)
In this case it is a move-over-redirect which is aborted by AbuseFilter and not reported nicly.

Maybe using a ErrorPageError instead of MWException? It is not possible to return the Status to the caller

throw new ErrorPageError( 'internalerror', $status->getMessage() );

It is not possible to return the Status to the caller

I think it shouldn't be terribly hard. MovePage::moveToInternal is private and it has a single caller, so any change would be self-contained. We might change the method to return a Status object (wrapping the revision in case of success), and check/merge the Status inside moveUnsafe.

Clarakosi added a subscriber: Clarakosi.

@Ammarpad Let us know if you need support or code review on this. Untagging Platform Engineering for now

Change 635407 had a related patch set uploaded (by Ammarpad; owner: Ammarpad):
[mediawiki/core@master] MovePage: Handle target page deletion failure gracefully.

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

Change 635407 merged by jenkins-bot:
[mediawiki/core@master] MovePage: Handle target page deletion failure gracefully.

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

Krinkle renamed this task from MWError when trying to rename a page to Fatal MWException when trying to rename a page.Oct 23 2020, 7:48 PM
Ammarpad closed this task as Resolved.Oct 26 2020, 7:41 AM