JsonGetter::exec() should return "null" on error

[vivainio]

Currently, JsonGetter misbehaves when http connection fails. It returns jsonmodel, which craps out when called from the superclass (HttpConnection) on retry.

[vivainio]

Some logging:

JSONGetter HTTP response problem: http://www.sodexo.fi/ruokalistat/output/daily_json/731/1/1/2013/en?mobileRedirect=false : null, EXCEPTION: java.lang.ClassCastException: org.tantalum.net.json.JSONModel cannot be cast to byte[]
java.lang.ClassCastException: org.tantalum.net.json.JSONModel cannot be cast to byte[]
at org.tantalum.net.json.JSONGetter.exec(JSONGetter.java:58)
at org.tantalum.net.HttpGetter.exec(HttpGetter.java:765)
at org.tantalum.net.json.JSONGetter.exec(JSONGetter.java:58)
at org.tantalum.net.HttpGetter.exec(HttpGetter.java:765)
at org.tantalum.net.json.JSONGetter.exec(JSONGetter.java:58)
at org.tantalum.Task.executeTask(Task.java:858)
at org.tantalum.Worker.run(Worker.java:471)

[vivainio]

Apparently, you get the same exception even with working HTTP connection, i.e. JsonGetter may be broken in general (?).

[paulirotta]

Nice catch. The HTTP retry introduces some oddness because it calls exec() directly after a 5sec pause. This makes the normal cancel() logic is slightly skewed.

Updating HttpGetter and all superclasses as follows is the short fix.

    if (!success) {
        return null;
    }

    return out;

The alt fix would be to change retry. But for example using a Timer to retry after 5sec is one though. But synchronous retry was done this way to avoid retry break complications with (new HttpGetter("url"...).get())

In general the return value of a failed Task is poorly defined. The next task in chain won't be executed so this mostly does not matter, but cancel() in the context also needs some systematic study to ensure the corner cases are covered.

[vivainio]

I'm also not sure whether JsonGetter being subclass on HttpGetter is optimal design. It could be better if JsonGetter was using HttpGetter instead of inheriting from it.

E.g. when you instantiate JsonGetter, it instantiates HttpGetter and chains itself to it, and only gets the data from "input" parameter in exec()

Edit: ok, that would be contorted design. It should use HttpGetter, but should explicitly launch HttpGetter::exec() and wait for it to complete in its own exec(). "subtasks within tasks" makes a useful FAQ item ;-)

[paulirotta]

Yes, JSONGetter creating and getting an HttpGetter has advantages.

One difference vs chain() is with prioritization. HttpGetter shouldn't be in the FASTLANE as it takes so long and we want to keep that one threadpool thread free to be repsonsive even during heavy traffice. Need to think a bit and have a go-through. Leaving as-is for now

[paulirotta]

Should be fixed in latest merge to master