images failing to sync about inaturalistandroid HOT 15 CLOSED

It looks like the response from observation_photos.json is missing the "photo" key, so o.getJSONObject("photo") returns null. That would appear to happen only when this branch is triggered in the controller: https://github.com/inaturalist/inaturalist/blob/master/app/controllers/observation_photos_controller.rb#L61

It looks like the JSON response contains an error instead of a proper response and that should probably be checked here: https://github.com/inaturalist/iNaturalistAndroid/blob/master/src/org/inaturalist/android/INaturalistService.java#L755

Regardless, a NPE should never occur, so some more error checking should certainly be done.

from inaturalistandroid.

If the rails logs are handy for the date I posted the photo, it should be possible to see what the original error message was. The observation I reported the problem for has this timestamp: Jul. 29, 2014 04:38:59 -0700

from inaturalistandroid.

Also, regarding the missing photo on disk issue, I don't think that could have been the case for me since all I did was take the photo, go into the iNat app to add the observation, and have the failed sync occur. I didn't do anything that would alter the photo on disk between those steps and it was still on my camera in the same place after the failed sync.

from inaturalistandroid.

Cool, good sleuthing. We don't keep server logs that far back, but I'll add an Airbrake notification there and see what we get.

from inaturalistandroid.

Mike - do you think this is something you can get to the bottom of and/or would it be helpful for us to contract a few hours of Yaron's time to help you look into it?

from inaturalistandroid.

I think that getting the error message from the server side would help determine the underlying issue (assuming it's just a single problem).

Beyond that, the error handling should be enhanced in the app itself, which would mean someone doing dev work on iNaturalistAndroid. I'd be happy to do that work, since it's quite easy, but I need to get my development setup for the app working and I haven't had time to do it yet since I'm on vacation :)

from inaturalistandroid.

Ok, we'll put @budowski on this issue for now then. I have the Airbrake notification in place, so if you can trigger the error again I can probably figure out what's causing the ObservationPhoto to be invalid.

from inaturalistandroid.

Sounds good. My suggestion would be to put the check for the "errors" key in the JSON response into the post method, assuming that the method of presenting errors from various controllers consistently uses that key. These invalid JSON responses should never be making it to code that expects a valid response.

Beyond that, I'm leery to recommend having null checks on every call to getJSONObject, but NPEs suck enough that it might be worth looking for a way to avoid these exceptions and get better information (e.g. a version of getJSONObject that throws an informative exception on a missing key).

from inaturalistandroid.

I'm not running into any problems along these lines during my use of the Android app - Mike are you still?

from inaturalistandroid.

I haven't noticed it lately. I'm curious whether anything ever showed up in Airbrake though.

from inaturalistandroid.

I'm looking at Airbreak now - do you know how to access the Notification Ken-ichi's referring to?

from inaturalistandroid.

I think it would say "Failed to create observation photo" (cf. https://github.com/inaturalist/inaturalist/blob/master/app/controllers/observation_photos_controller.rb#L62-L63)

from inaturalistandroid.

I don't see any 'Failed to create observation photo' notifications (under resolved or unresolved errors) on Airbrake searching through the last 6 months

from inaturalistandroid.

Good to hear that. If the work was never done to standardize the error handling in this app, I think it still makes sense as an enhancement, but I'd consider this immediate issue closed for the moment.

The bottom line is that images were failing to sync because something was breaking on the server that is no longer breaking. The enhanced error handling just helps to make better sense of future server errors, should they occur.

from inaturalistandroid.

ok great - I think @pleary is going to take on standardizing error handling in the coming months. Thanks for your help with this and everything else, Mike

from inaturalistandroid.