Add null start timestamp check for metric events #857
Conversation
There was a problem hiding this comment.
Noting here that this won't be needed after #697 lands because it's now dealt internally by the new Events library.
| @@ -216,4 +217,11 @@ private static void updateRouteProgressSessionData(MetricsRouteProgress routePro | |||
| private static Location[] convertToArray(List<Location> locationList) { | |||
| return locationList.toArray(new Location[locationList.size()]); | |||
| } | |||
|
|
|||
| private static Date checkNullStartTimestamp(SessionState sessionState) { | |||
| if (sessionState.startTimestamp() != null) { | |||
There was a problem hiding this comment.
Minor detail: Why not the other way around? Why not using a guard clause instead? In the exceptional case (sessionState.startTimestamp() is null) return new Date() and in the normal case return sessionState.startTimestamp(). IMO it makes clearer the intention of the method.
@danesfeder Given certain constraints related to my own backend's data, I had to test several times to fetch very short navigation routes with a starting point almost identical to that of arrival. So, as you said in your first post, this could be the cause! |
danesfeder
force-pushed
the
dan-null-timestamp
branch
from
63d0e49 to
dfccd51
Compare
danesfeder
force-pushed
the
dan-null-timestamp
branch
from
dfccd51 to
72cb4de
Compare
Closes #827
I haven't been able to reproduce this issue. In the ticket, it seems an arrival event is being fired as soon as navigation begins and before we have a start timestamp (some distance has been traveled).
This PR will fix the immediate issue, but I'm interested if there is a deeper issue. The only way I can imagine this occurring is if a
DirectionsRouteis provided that's very short, where the first step is the arrival step. I'm hoping the two developers that reproduced this will provide more context in #827.