Page MenuHomePhabricator

Exception "Call to a member function getTimestamp() on a non-object (boolean)"
Closed, ResolvedPublic2 Estimated Story PointsPRODUCTION ERROR

Description

An error is frequently occurring in logstash. It might be exposing a bug that we are not aware of, and is making it harder to see other bugs in our error logs so we should aim to investigate it and squash it!

Uncommon
Live errors

Stack trace:

/srv/mediawiki/php-1.29.0-wmf.16/extensions/MobileFrontend/includes/skins/SkinMinerva.php(91): SkinMinerva->prepareUserButton(MinervaTemplate)
#1 /srv/mediawiki/php-1.29.0-wmf.16/includes/skins/SkinTemplate.php(249): SkinMinerva->prepareQuickTemplate()
#2 /srv/mediawiki/php-1.29.0-wmf.16/includes/OutputPage.php(2400): SkinTemplate->outputPage()
#3 /srv/mediawiki/php-1.29.0-wmf.16/includes/MediaWiki.php(867): OutputPage->output(boolean)
#4 /srv/mediawiki/php-1.29.0-wmf.16/includes/MediaWiki.php(879): Closure$MediaWiki::main()
#5 /srv/mediawiki/php-1.29.0-wmf.16/includes/MediaWiki.php(521): MediaWiki->main()
#6 /srv/mediawiki/php-1.29.0-wmf.16/index.php(43): MediaWiki->run()
#7 /srv/mediawiki/w/index.php(3): include(string)
#8 {main}

Along with T151838 this contributes for the majority of our noise in our error logs so it would be good to make it go away.

Developer notes

The issue appears to be due to the line so only happens when the user is logged in

$echoSeenTime < $notifLastUnreadTime->getTimestamp( TS_ISO_8601 )

getLastUnreadNotificationTime can return false so this should be a trivial fix.

Acceptance criteria

  • False is returned when there are no notifications. A bell should show in this case.

Event Timeline

Jdlrobson triaged this task as Medium priority.Apr 6 2017, 4:53 PM
Jdlrobson set the point value for this task to 2.

From @Jdlrobson: getLastUnreadNotificationTime returns false when the user has no notifications. If this is the case, then we should render the bell icon.

Change 353481 had a related patch set uploaded (by Bmansurov; owner: Bmansurov):
[mediawiki/extensions/MobileFrontend@master] Check data type before calling member function

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

^ I left a couple of comments about the organization/readability of the tests. The change itself is 👌

I also left some comments on Make use of request context passed to SkinMinerva. I think the same outcome can be achieved by not making ContextSource to the constructor so should be easy to resolve.

bmansurov added a subscriber: bmansurov.

Change 353481 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Check data type before calling member function

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

This should be fixed but cannot be signed off till it's in production which should be 25th.

In the last 24 hours, following the deployment, all exceptions relating to this are gone HURRAH!

Compare this with the last 48 hours.

Will double check again next Tuesday before resolving.

No errors in 48hrs. Quite confident this is fixed at this point. We'll keep an eye on the logs as part of chores so can reopen if it does show it's face again.

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