Page MenuHomePhabricator

ParsoidBatchAPI: Images with invalid titles aren't handled correctly
Closed, ResolvedPublic

Description

I enabled batching in production today and was monitoring error logs in kibana on wtp1002 which I used as a canary before restarting parsoid on all nodes.

In a 15 min period, I saw about 4 errors that all were of the form:

error={"code":"internal_api_error_BadMethodCallException","info":"[88b2103a] Exception Caught: Call to a member function getDBkey() on a non-object (NULL)"}

@Catrope's comments about this:

<RoanKattouw>                                 $title = Title::makeTitleSafe( NS_FILE, $itemParams['filename'] );
<RoanKattouw>                                 $filenames[] = $batch[$itemIndex]['filename'] = $title->getDBkey();
<RoanKattouw> the documentation for makeTitleSafe states that it returns null on error
<RoanKattouw> and that code doesn't check whether $title is null

This can be reproduced locally with:

$ node parse --useBatchAPI --prefix eswiki --page 'Lázaro_Morúa' < /dev/null > /dev/null

Here is the diff of batching and non-batching output locally (after introducing newlines after every > char in the output)

[subbu@earth api] diff /tmp/batch.html /tmp/nobatch.html 
227c227
< <figure class="mw-halign-center" typeof="mw:Error mw:Image" data-mw='{"errors":[{"key":"api-error","message":{}}]}'>
---
> <figure class="mw-halign-center" typeof="mw:Image">
229c229
< <img alt="" resource="./Archivo:Gnome-speakernotes.svg" src="./Special:FilePath/Gnome-speakernotes.svg" height="45" width="45"/>
---
> <img alt="" resource="./Archivo:Gnome-speakernotes.svg" src="//upload.wikimedia.org/wikipedia/commons/thumb/9/94/Gnome-speakernotes.svg/45px-Gnome-speakernotes.svg.png" data-file-width="128" data-file-height="128" data-file-type="drawing" height="45" width="45"/>
249c249
< <span typeof="mw:Error mw:Image" data-mw='{"errors":[{"key":"api-error","message":{}}],"caption":"noicon"}'>
---
> <span typeof="mw:Error mw:Image" data-mw='{"errors":[{"key":"missing-image","message":"This image does not exist."}],"caption":"noicon"}'>

So, it looks like ParsoidBatchAPI needs to detect these failures and return an appropriate error message similar to what the main API returns. Parsoid image handling code than includes this error information with appropriate markup in the HTML.

Details

Related Gerrit Patches:
mediawiki/extensions/ParsoidBatchAPI : wmf/1.26wmf24[1.26wmf24] In imageinfo, check for invalid title
mediawiki/extensions/ParsoidBatchAPI : masterIn imageinfo, check for invalid title

Event Timeline

ssastry created this task.Sep 28 2015, 10:27 PM
ssastry assigned this task to tstarling.
ssastry raised the priority of this task from to High.
ssastry updated the task description. (Show Details)
ssastry added projects: Parsing-Team, Parsoid.
ssastry added subscribers: ssastry, Catrope.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 28 2015, 10:27 PM

Change 241961 had a related patch set uploaded (by Tim Starling):
In imageinfo, check for invalid title

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

tstarling renamed this task from ParsoidBatchAPI: Missing images aren't handled correctly to ParsoidBatchAPI: Images with invalid titles aren't handled correctly.Sep 28 2015, 11:14 PM
tstarling set Security to None.

This can be reproduced with just [[Image:]]. This is the fifth case in my table at T113322. MediaWiki outputs a literal [[Image:]] for this input whereas Parsoid makes a broken image link, with img src="./Special:FilePath/".

Change 241961 merged by jenkins-bot:
In imageinfo, check for invalid title

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

Change 242024 had a related patch set uploaded (by Tim Starling):
[1.26wmf24] In imageinfo, check for invalid title

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

Change 242024 merged by jenkins-bot:
[1.26wmf24] In imageinfo, check for invalid title

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

ssastry closed this task as Resolved.Oct 1 2015, 4:29 PM
ssastry removed a project: Patch-For-Review.
ssastry removed a subscriber: gerritbot.