Page MenuHomePhabricator

ApiQueryExtracts::parse cannot handle failed API queries
Closed, ResolvedPublic2 Story Points

Description

Log error is occurring infrequently but easily fixed:
Notice: Undefined index: parse in /srv/mediawiki/php-1.30.0-wmf.6/extensions/TextExtracts/includes/ApiQueryExtracts.php on line 269

Offending code: https://github.com/wikimedia/mediawiki-extensions-TextExtracts/blob/master/includes/ApiQueryExtracts.php#L280

logstash

Acceptance criteria

  • Avoid exception - return empty string when $data['parse'] is undefined.
  • Log information about the page title and revision that we failed to parse.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJun 27 2017, 8:55 PM
Jdlrobson triaged this task as Normal priority.Jun 28 2017, 4:02 PM
Jdlrobson updated the task description. (Show Details)Jul 25 2017, 5:27 PM

Per chores email (25th) this keeps coming up so I'd like to nominate it for estimation/upcoming sprint.

Jdlrobson updated the task description. (Show Details)Jul 26 2017, 5:29 PM
Jdlrobson updated the task description. (Show Details)Jul 26 2017, 5:32 PM
Jdlrobson set the point value for this task to 2.
Jdlrobson moved this task from Upcoming to 2017-18 Q1 on the Readers-Web-Backlog board.
pmiazga claimed this task.Aug 1 2017, 1:39 PM

I'm not sure if avoiding exception by returning empty text is a correct approach. API:Parsing_wikitext won't return parse data only when there is an error.
In that case we shouldn't mute error and silently return an empty string because then we store the response in cache for ParserCacheExpireTime which is 22 days.

First, we should log those pages to find the root cause instead of sweeping it under the rug and then or fix the broken behavior or handle API errors properly.

Jdlrobson updated the task description. (Show Details)Aug 1 2017, 4:21 PM

Sounds good to me. Added acceptance criteria.

Change 369549 had a related patch set uploaded (by Pmiazga; owner: Pmiazga):
[mediawiki/extensions/TextExtracts@master] When APIParse fails log the warning and return null

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

Change 369549 merged by jenkins-bot:
[mediawiki/extensions/TextExtracts@master] When APIParse fails log the warning and return null

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

bmansurov claimed this task.Aug 2 2017, 4:58 PM
bmansurov added a subscriber: pmiazga.

Will sign off once the patch hits production.

Note errors occurs very infrequently and we're not sure why it happens, so it's likely we won't be able to sign this off by looking at logstash.
I would recommend we sign off by simulating the issue locally and then reopen it if we see it occur again (this time we should have some better information in logs).

Note if we can find a way to replicate this we can sign off using https://logstash-beta.wmflabs.org

bmansurov closed this task as Resolved.Aug 4 2017, 6:16 PM
bmansurov removed bmansurov as the assignee of this task.
bmansurov added a subscriber: bmansurov.

I added unset($data['parse']); to the offending function, towards the end of it. The API didn't fail unlike before.

We added an extra logging to find out why this happens in production as we couldn't reproduce it locally. Now we will stop logging the PHP notice error, we will log our custom error instead with a bit more information whats going on so we can fix it.

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