Page MenuHomePhabricator

Notice: Undefined property: stdClass::$rc_timestamp in /srv/mediawiki/php-1.31.0-wmf.23/includes/specials/SpecialNewpages.php
Closed, ResolvedPublic

Description

Saw this notice increase dramatically with the wmf.23 to group1

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 28 2018, 9:59 PM

Evidently this means that all pages in Special:Newpages default to the current time:

Mentioned in SAL (#wikimedia-operations) [2018-02-28T22:11:33Z] <thcipriani@tin> rebuilt and synchronized wikiversions files: Group1 back to 1.31.0-wmf.22 T188555

Stacktrace:

#0 /srv/mediawiki/php-1.31.0-wmf.23/includes/specials/SpecialNewpages.php(327): MWExceptionHandler::handleError(integer, string, string, integer, array)
#1 /srv/mediawiki/php-1.31.0-wmf.23/includes/specials/pagers/NewPagesPager.php(140): SpecialNewpages->formatRow(stdClass)
#2 /srv/mediawiki/php-1.31.0-wmf.23/includes/pager/IndexPager.php(445): NewPagesPager->formatRow(stdClass)
#3 /srv/mediawiki/php-1.31.0-wmf.23/includes/specials/SpecialNewpages.php(159): IndexPager->getBody()
#4 /srv/mediawiki/php-1.31.0-wmf.23/includes/specialpage/SpecialPage.php(522): SpecialNewpages->execute(NULL)
#5 /srv/mediawiki/php-1.31.0-wmf.23/includes/specialpage/SpecialPageFactory.php(579): SpecialPage->run(NULL)
#6 /srv/mediawiki/php-1.31.0-wmf.23/includes/MediaWiki.php(288): SpecialPageFactory::executePath(Title, RequestContext)
#7 /srv/mediawiki/php-1.31.0-wmf.23/includes/MediaWiki.php(861): MediaWiki->performRequest()
#8 /srv/mediawiki/php-1.31.0-wmf.23/includes/MediaWiki.php(524): MediaWiki->main()
#9 /srv/mediawiki/php-1.31.0-wmf.23/index.php(42): MediaWiki->run()
#10 /srv/mediawiki/w/index.php(3): require(string)
#11 {main}

This is also happening in another spot, let me see if I can find a trace for that one.

Here's the 2nd one:

#0 /srv/mediawiki/php-1.31.0-wmf.23/includes/specials/SpecialNewpages.php(472): MWExceptionHandler::handleError(integer, string, string, integer, array, array)
#1 /srv/mediawiki/php-1.31.0-wmf.23/includes/specials/SpecialNewpages.php(455): SpecialNewpages->feedItem(stdClass)
#2 /srv/mediawiki/php-1.31.0-wmf.23/includes/specials/SpecialNewpages.php(140): SpecialNewpages->feed(string)
#3 /srv/mediawiki/php-1.31.0-wmf.23/includes/specialpage/SpecialPage.php(522): SpecialNewpages->execute(NULL)
#4 /srv/mediawiki/php-1.31.0-wmf.23/includes/specialpage/SpecialPageFactory.php(579): SpecialPage->run(NULL)
#5 /srv/mediawiki/php-1.31.0-wmf.23/includes/MediaWiki.php(288): SpecialPageFactory::executePath(Title, RequestContext)
#6 /srv/mediawiki/php-1.31.0-wmf.23/includes/MediaWiki.php(861): MediaWiki->performRequest()
#7 /srv/mediawiki/php-1.31.0-wmf.23/includes/MediaWiki.php(524): MediaWiki->main()
#8 /srv/mediawiki/php-1.31.0-wmf.23/index.php(42): MediaWiki->run()
#9 /srv/mediawiki/w/index.php(3): include(string)
#10 {main}
Krinkle assigned this task to Anomie.EditedFeb 28 2018, 10:22 PM
Krinkle triaged this task as High priority.
Krinkle added a subscriber: Krinkle.

Looking at rMW27c61fb1e94d: Add `actor` table and code to start using it it seems like the most likely commit to have caused this.

Before
$fields = [
		'rc_namespace', 'rc_title', 'rc_cur_id', 'rc_user', 'rc_user_text',
		'rc_timestamp', 'rc_patrolled', 'rc_id', 'rc_deleted',
		'length' => 'page_len', 'rev_id' => 'page_latest',
		'rc_this_oldid',
		'page_namespace', 'page_title'
After
$rcQuery = RecentChange::getQueryInfo();//  = [ 'rc_namespace', 'rc_timestamp', ... ];
$fields = [
	'length' => 'page_len', 'rev_id' => 'page_latest', 'page_namespace', 'page_title'
] + $rcQuery['fields'];

Seems like the + operator strikes again, by being used for an array that is not limited to associative entries. As such, I'd expect the 'page_namespace', 'page_title' entries to overwrite the first two entries from getQueryInfo, including rc_timestamp.

greg raised the priority of this task from High to Unbreak Now!.Feb 28 2018, 11:20 PM
greg added a subscriber: greg.

@Krinkle by triaging to High instead of UBN! should I interpret that as meaning that you don't think this should be a deployment blocker? It looks like one to me (and we rolled back because of it).

I'm setting to UBN, feel free to tell me I'm wrong and why.

Restricted Application added subscribers: Liuxinyu970226, TerraCodes. · View Herald TranscriptFeb 28 2018, 11:20 PM
Anomie added a comment.Mar 1 2018, 1:30 PM

Unfortunate timing that this showed up just after 5pm my time. No one else wanted to fix it before my morning?

Change 415558 had a related patch set uploaded (by Anomie; owner: Anomie):
[mediawiki/core@master] NewPagesPages: Use array_merge rather than + for RC query info fields

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

Change 415558 merged by jenkins-bot:
[mediawiki/core@master] NewPagesPages: Use array_merge rather than + for RC query info fields

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

Change 415592 had a related patch set uploaded (by Thcipriani; owner: Anomie):
[mediawiki/core@wmf/1.31.0-wmf.23] NewPagesPages: Use array_merge rather than + for RC query info fields

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

Change 415592 merged by jenkins-bot:
[mediawiki/core@wmf/1.31.0-wmf.23] NewPagesPages: Use array_merge rather than + for RC query info fields

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

Mentioned in SAL (#wikimedia-operations) [2018-03-01T20:15:45Z] <thcipriani@tin> Synchronized php-1.31.0-wmf.23/includes/specials/pagers/NewPagesPager.php: SWAT: [[gerrit:415592|NewPagesPages: Use array_merge rather than + for RC query info fields]] T188555 (duration: 01m 14s)

Krinkle added a comment.EditedMar 1 2018, 8:56 PM

@Krinkle by triaging to High instead of UBN! should I interpret that as meaning that you don't think this should be a deployment blocker? [..]

Nope, I was just trying to be modest; given the relatively limited functional impact (low chance of permanent corruption, and otherwise limited to the UI). But on second thought, the timestamps being all wrong on NewPages is UBN in itself, not just because it happened to overflow low-severity error logs.

greg added a comment.Mar 1 2018, 9:33 PM

@Krinkle by triaging to High instead of UBN! should I interpret that as meaning that you don't think this should be a deployment blocker? [..]

Nope, I was just trying to be modest based given the relatively limited functional impact (low chance of permanent corruption, and otherwise limited to the UI). But on second thought, the timestamps being all wrong on NewPages is UBN in itself, not just because it happened to overflow low-severity error logs.

ack :)

greg closed this task as Resolved.Mar 1 2018, 9:36 PM

Merged in master and wmf.23, confirmed fixed in production, resolving.

TheDJ added a subscriber: TheDJ.Mar 5 2018, 8:48 AM

Just wondering, as this clearly is a recurring problem, is this something we might be able to php code sniff for ?