Page MenuHomePhabricator

EditPage: Argument 2 passed to MediaWiki\Revision\RevisionStore::getRevisionByTimestamp() must be of the type string, null given
Closed, ResolvedPublicPRODUCTION ERROR



MediaWiki version: 1.36.0-wmf.33

Argument 2 passed to MediaWiki\Revision\RevisionStore::getRevisionByTimestamp() must be of the type string, null given, called in /srv/mediawiki/php-1.36.0-wmf.33/includes/EditPage.php on line 2619


Looks to me like some edits are failing.


Page title can be found in logstash under this request ID.

Noticed 6 of these in the last 4 hours in logstash, and 45 in logspam-watch since logs on mwlog1001 last rolled over:

45      __▇▄___ 1044    1312    TypeError...........    .33 i/R/RevisionStore:1209  Argument 2 passed to MediaWiki\Revision\R
[pattern: .*getRevisionByTimestamp.*]

Already in 1.36.0-wmf.33, so not blocking the wmf.34 train on it.


Request ID
Request URL[title]&action=submit
Stack Trace
from /srv/mediawiki/php-1.36.0-wmf.33/includes/Revision/RevisionStore.php(1209)
#0 /srv/mediawiki/php-1.36.0-wmf.33/includes/EditPage.php(2619): MediaWiki\Revision\RevisionStore->getRevisionByTimestamp(Title, NULL, integer)
#1 /srv/mediawiki/php-1.36.0-wmf.33/includes/EditPage.php(2542): EditPage->getExpectedParentRevision()
#2 /srv/mediawiki/php-1.36.0-wmf.33/includes/EditPage.php(2214): EditPage->mergeChangesIntoContent(WikitextContent)
#3 /srv/mediawiki/php-1.36.0-wmf.33/includes/EditPage.php(1687): EditPage->internalAttemptSave(NULL, boolean)
#4 /srv/mediawiki/php-1.36.0-wmf.33/includes/EditPage.php(662): EditPage->attemptSave(NULL)
#5 /srv/mediawiki/php-1.36.0-wmf.33/includes/actions/EditAction.php(71): EditPage->edit()
#6 /srv/mediawiki/php-1.36.0-wmf.33/includes/actions/SubmitAction.php(38): EditAction->show()
#7 /srv/mediawiki/php-1.36.0-wmf.33/includes/MediaWiki.php(531): SubmitAction->show()
#8 /srv/mediawiki/php-1.36.0-wmf.33/includes/MediaWiki.php(315): MediaWiki->performAction(Article, Title)
#9 /srv/mediawiki/php-1.36.0-wmf.33/includes/MediaWiki.php(925): MediaWiki->performRequest()
#10 /srv/mediawiki/php-1.36.0-wmf.33/includes/MediaWiki.php(547): MediaWiki->main()
#11 /srv/mediawiki/php-1.36.0-wmf.33/index.php(53): MediaWiki->run()
#12 /srv/mediawiki/php-1.36.0-wmf.33/index.php(46): wfIndexMain()
#13 /srv/mediawiki/w/index.php(3): require(string)
#14 {main}

Event Timeline

Krinkle raised the priority of this task from Medium to High.Mar 27 2021, 3:07 AM
Krinkle subscribed.

Type errors are almost always deterministic, highly visible, and very likely regressions caused by recent development in this area

(Since both this code, and our habit of typing are relatively new; and we tend to fix this kind of issue quickly when it happens.)

The null is EditPage::edittime which is set to empy string, but on validation it can be set to null. When EditPage::editRevId is also null, than it fails like this here.

DannyS712 moved this task from Unsorted to Next on the User-DannyS712 board.

We should fix this before switching to the new editpage backend, so that it doesn't inherit any existing errors - I'll try to investigate this. Is there a stable link or set of steps to reproduce this?

I was able to reproduce this by setting the form field editRevId to "0" and omitting the form field wpEdittime. The error occurs when EditPage::mergeChangesIntoContent() is called when edittime is null and editRevId is falsey. If editRevId is null, the edit conflict is suppressed (EditPage.php:2134), that's why you have to set it to 0.

More broadly, edittime is clearly nullable and its type is incorrectly documented. So it's incorrect to pass it through to getRevisionByTimestamp(). getExpectedParentRevision() is documented as potentially returning null, an unlikely scenario with the current code, the effect of which is apparently harmless. Before type checks were added, a null timestamp would have caused it to return null due to the Revision::loadFromTimestamp query returning no rows.

Change 685179 had a related patch set uploaded (by Tim Starling; author: Tim Starling):

[mediawiki/core@master] EditPage::getExpectedParentRevision(): guard against null edittime

Change 685179 merged by jenkins-bot:

[mediawiki/core@master] EditPage::getExpectedParentRevision(): guard against null edittime