Page MenuHomePhabricator

[Bug] fatal error: Argument 1 passed to CachingEntityRevisionLookup::getEntityRevision() must be an instance of EntityId, null given
Closed, ResolvedPublic

Description

Seeing a lot of these in the hhvm.log right now, don't have backtraces though.

Apr 13 23:25:38 mw1209:  #012Catchable fatal error: Argument 1 passed to Wikibase\Lib\Store\CachingEntityRevisionLookup::getEntityRevision() must be an instance of Wikibase\DataModel\Entity\EntityId, null given in /srv/mediawiki/php-1.27.0-wmf.21/extensions/Wikidata/extensions/Wikibase/lib/includes/store/CachingEntityRevisionLookup.php on line 136

This started with 1.27.0-wmf.21:

hoo@fluorine:/a/mw-log$ grep -Fc 'Argument 1 passed to Wikibase\Lib\Store\CachingEntityRevisionLookup::getEntityRevision() must be an instance of Wikibase\DataModel\Entity\EntityId, null given in /srv/mediawiki/php-1.27.0-wmf.21' hhvm.log 
385
hoo@fluorine:/a/mw-log$ grep -Fc 'Argument 1 passed to Wikibase\Lib\Store\CachingEntityRevisionLookup::getEntityRevision() must be an instance of Wikibase\DataModel\Entity\EntityId, null given in /srv/mediawiki/php-1.27.0-wmf.20' hhvm.log 
0

Details

Related Gerrit Patches:
mediawiki/extensions/Wikibase : wmf/1.27.0-wmf.21Fix fatal error in OutputPageBeforeHTMLHookHandler
mediawiki/extensions/Wikibase : masterAdd deleted revisions test for OutputPageBeforeHTMLHookHandler
mediawiki/extensions/Wikibase : masterFix minor code style issues in OutputPageBeforeHTMLHookHandler
mediawiki/extensions/Wikibase : masterFix fatal error in OutputPageBeforeHTMLHookHandler

Event Timeline

hoo created this task.Apr 13 2016, 11:31 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptApr 13 2016, 11:31 PM
hoo renamed this task from Argument 1 passed to CachingEntityRevisionLookup::getEntityRevision() must be an instance of EntityId, null given to [Bug] fatal error: Argument 1 passed to CachingEntityRevisionLookup::getEntityRevision() must be an instance of EntityId, null given.Apr 13 2016, 11:31 PM
adrianheine triaged this task as High priority.Apr 14 2016, 8:14 AM

I created T132697 to find how to get backtraces in the future.

Seeing a lot of these in the hhvm.log right now, don't have backtraces though.

hhvm.log will never have PHP backtraces. fatal.log will if MWExceptionHandler::handleError has been notified of the error. One thing that should be done is an audit of core and extensions for places where set_error_handler() is used to change the error handler from MWExceptionHandler::handleError which knows how to log stacktraces.

T89169#2235280 didn't help with getting a back trace here is a newer occurance from logstash:

{
  "_index": "logstash-2016.04.25",
  "_type": "hhvm",
  "_id": "AVROS2NLCsPTNesWoJye",
  "_score": null,
  "_source": {
    "message": "Catchable fatal error: Argument 1 passed to Wikibase\\Lib\\Store\\CachingEntityRevisionLookup::getEntityRevision() must be an instance of Wikibase\\DataModel\\Entity\\EntityId, null given in /srv/mediawiki/php-1.27.0-wmf.21/extensions/Wikidata/extensions/Wikibase/lib/includes/store/CachingEntityRevisionLookup.php on line 136",
    "@version": "1",
    "@timestamp": "2016-04-25T16:40:43.000Z",
    "type": "hhvm",
    "host": "mw1220",
    "priority": 11,
    "timestamp": "Apr 25 16:40:43",
    "logsource": "mw1220",
    "program": "hhvm",
    "severity": 3,
    "facility": 1,
    "facility_label": "user-level",
    "severity_label": "Error",
    "level": "ERROR",
    "tags": [
      "syslog",
      "es",
      "_grokparsefailure",
      "normalized_message_trimmed"
    ],
    "normalized_message": "Catchable fatal error: Argument 1 passed to Wikibase\\Lib\\Store\\CachingEntityRevisionLookup::getEntityRevision() must be an instance of Wikibase\\DataModel\\Entity\\EntityId, null given in /srv/mediawiki/php-1.27.0-wmf.21/extensions/Wikidata/extensions/Wikiba"
  },
  "sort": [
    1461602443000
  ]
}

A "Catchable fatal error" getting to the HHVM logs is still a sign that MWExceptionHandler::handleError() is not the active handler via set_error_handler() at the time that the exception is raised.

One way you could deal with this at least as a temporary patch is to change the signature of Wikibase\Lib\Store\CachingEntityRevisionLookup::getEntityRevision() so that nulls are allowed for $entityId and then validate the non-null status inside the method. At that point you would be in control of logging the call path.

aude added a comment.Apr 25 2016, 7:47 PM

this issue happens when trying to view a deleted revision (if you are admin), and maybe in other cases.

before, we checked if we got an EntityRevision object and if not, skip rendering the term box (return html rendering as empty string):

https://gerrit.wikimedia.org/r/#/c/277771/5/view/src/EntityViewPlaceholderExpander.php

We need to re-introduce such a check in the code, though in a way that keeps EntityRevisionLookup out of EntityViewPlaceholderExpander.

Change 285289 had a related patch set uploaded (by Aude):
Fix fatal error in OutputPageBeforeHTMLHookHandler

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

Change 285405 had a related patch set uploaded (by Thiemo Mättig (WMDE)):
Fix minor code style issues in OutputPageBeforeHTMLHookHandler

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

Change 285289 merged by jenkins-bot:
Fix fatal error in OutputPageBeforeHTMLHookHandler

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

Change 285405 merged by jenkins-bot:
Fix minor code style issues in OutputPageBeforeHTMLHookHandler

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

Change 285417 had a related patch set uploaded (by Thiemo Mättig (WMDE)):
Add deleted revisions test for OutputPageBeforeHTMLHookHandler

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

Change 285417 merged by jenkins-bot:
Add deleted revisions test for OutputPageBeforeHTMLHookHandler

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

Result from trying to debug this with things like file_put_contents('/tmp/debug.txt', 'info'."\n", FILE_APPEND); to find how to get full back traces:

handleError gets called 2 times with:

$level => 4096,
$message => 'Argument 1 passed to Wikibase\\Lib\\Store\\CachingEntityRevisionLookup::getEntityRevision() must be an instance of Wikibase\\DataModel\\Entity\\EntityId, null given',
$file => '/srv/mediawiki/php-1.27.0-wmf.21/extensions/Wikidata/extensions/Wikibase/lib/includes/store/CachingEntityRevisionLookup.php',
$line => 136,

handleFatalError gets called once after that with:

$level => 4096,
$message => 'Argument 1 passed to Wikibase\\Lib\\Store\\CachingEntityRevisionLookup::getEntityRevision() must be an instance of Wikibase\\DataModel\\Entity\\EntityId, null given',
$file => '/srv/mediawiki/php-1.27.0-wmf.21/extensions/Wikidata/extensions/Wikibase/lib/includes/store/CachingEntityRevisionLookup.php',
$line => 136,
$context => NULL,
$trace => NULL,
debug_backtrace() =>   array (
  0 => 
  array (
    'function' => 'handleFatalError',
    'class' => 'MWExceptionHandler',
    'type' => '::',
    'args' => 
    array (
    ),
  ),
),

If I try to insert a debug_backtrace() or func_get_args() at the beginning of handleError things change in ways I didn't understand.

Change 285557 had a related patch set uploaded (by Aude):
Fix fatal error in OutputPageBeforeHTMLHookHandler

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

Fix is merged. Is it deployed? Did the error disappear?

hoo added a comment.EditedApr 28 2016, 9:12 AM

Fix is merged. Is it deployed? Did the error disappear?

Hasn't been deployed yet… probably should be if possible (@aude, @JanZerebecki ).

I was considering doing it after the train is done so I only need to do it for one branch.

aude added a comment.Apr 28 2016, 3:47 PM

@hoo @JanZerebecki https://gerrit.wikimedia.org/r/#/c/285557/

The change is non-trivial (couldn't just cherry pick) so would prefer someone else +2 it. I have manually tested it on the branch.

hoo added a comment.Apr 28 2016, 3:55 PM

@hoo @JanZerebecki https://gerrit.wikimedia.org/r/#/c/285557/
The change is non-trivial (couldn't just cherry pick) so would prefer someone else +2 it. I have manually tested it on the branch.

I had a look, seems good to deploy to me.

Change 285557 merged by jenkins-bot:
Fix fatal error in OutputPageBeforeHTMLHookHandler

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

aude added a comment.Apr 29 2016, 1:20 AM

I am running into Echo test failure (T133774) on the build:

https://gerrit.wikimedia.org/r/#/c/286109/

https://integration.wikimedia.org/ci/job/mwext-testextension-hhvm/10706/consoleFull

Echo still appears to be included in the daily builds (e.g. https://integration.wikimedia.org/ci/job/mwext-testextension-hhvm/10627/consoleFull) and didn't fail there. Was this fixed in master?

aude added a comment.Apr 29 2016, 1:22 AM

I'm not enthusiastic about overriding jenkins right now and think it would be better to try again tomorrow morning (even if it is a Friday).

Maybe @JanZerebecki knows what to do about Echo here?

Mentioned in SAL [2016-04-29T18:14:27Z] <jzerebecki@tin> Synchronized php-1.27.0-wmf.22/extensions/Wikidata/extensions/Wikibase/repo/includes/Hooks/OutputPageBeforeHTMLHookHandler.php: wmf.22 fc20c54f7915b94ec0d15ef17e207c116910623d 2 of 2 T132645 (duration: 00m 34s)

Mentioned in SAL [2016-04-29T18:21:10Z] <jzerebecki@tin> Synchronized php-1.27.0-wmf.22/extensions/Wikidata/extensions/Wikibase/repo/includes/Hooks/OutputPageBeforeHTMLHookHandler.php: wmf.22 fc20c54f7915b94ec0d15ef17e207c116910623d 2 of 2 T132645 (duration: 00m 28s)

JanZerebecki closed this task as Resolved.Apr 29 2016, 6:23 PM
JanZerebecki assigned this task to aude.
JanZerebecki moved this task from Review to Done on the Wikidata-Sprint-2016-04-26 board.
mmodell changed the subtype of this task from "Task" to "Production Error".Aug 28 2019, 11:11 PM