Log Exceptions #230
Log Exceptions #230
Conversation
| } catch (RemoteException e) { | ||
| throw new RuntimeException(e); | ||
| Log.e(TAG, "Error occurred trying to get last Location " + e.getStackTrace()); |
There was a problem hiding this comment.
There is also Log.e(TAG,message,throwable), wouldn't this be preferrable? Or has that method any downsides I am not aware of?
| throw new RuntimeException(e); | ||
| Log.e(TAG, "Error occurred trying to get LocationAvailability " + e.getStackTrace()); | ||
| } finally { | ||
| return availability; |
There was a problem hiding this comment.
Will return null on error. Assuming this how Google's APIs behave as well, it would be good to mention this in the javadoc.
There was a problem hiding this comment.
Good catch, updated
| throw new RuntimeException(e); | ||
| Log.e(TAG, "Error occurred trying to get last Location " + e.getStackTrace()); | ||
| } finally { | ||
| return location; |
There was a problem hiding this comment.
As @westnordost pointed out below with availability, this will also return null if an error is encountered.
This behavior is consistent will play services.
| throw new RuntimeException(e); | ||
| Log.e(TAG, "Error occurred trying to get last Location", e); | ||
| } finally { | ||
| return location; |
There was a problem hiding this comment.
I would just add a note in the Javadoc for getLastLocation() that null is also a valid return value here and this looks good to go.
There was a problem hiding this comment.
We do have this documented already If a location is not available, which should happen very rarely, null will be returned.
Log Exceptions (lostzen#230)
Overview
This is the first step towards improving and standardizing exception handling in Lost. Currently we catch and re-throw
RuntimeExceptionsthrown when interacting with the service, a hard stance for a library to take. To work towards giving developers the chance to handle recovering from these cases, this PR updates Lost to silently ignoreRemoteExceptions. Future work will provide complete APIs to allow developers to handle recovering from these states as they see fit. It will potentially migrate existing silent failures to use these new APIs.Closes #218, #223