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

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 21 2017, 7:27 PM
Jdlrobson triaged this task as Medium priority.Apr 6 2017, 4:53 PM
Jdlrobson moved this task from 2016-17 Q4 to Upcoming on the Readers-Web-Backlog board.
Jdlrobson updated the task description. (Show Details)Apr 24 2017, 7:25 PM
Jdlrobson updated the task description. (Show Details)May 4 2017, 12:02 AM
Jdlrobson updated the task description. (Show Details)May 4 2017, 5:25 PM
Jdlrobson set the point value for this task to 2.
phuedx added a subscriber: phuedx.May 4 2017, 5:25 PM

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

Reviewing…

^ 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 removed bmansurov as the assignee of this task.May 12 2017, 7:48 PM
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

phuedx assigned this task to Jdlrobson.May 13 2017, 1:43 AM

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.

Jdlrobson closed this task as Resolved.May 26 2017, 5:47 PM

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