Check for valid DirectionsRoute in RouteRefresh

[danesfeder]

Found while testing:

E/AndroidRuntime: FATAL EXCEPTION: mapbox_navigation_thread
    Process: com.mapbox.mapboxsdk.downstream.testapp, PID: 1955
    java.lang.NumberFormatException: Invalid int: "null"
        at java.lang.Integer.invalidInt(Integer.java:138)
        at java.lang.Integer.parseInt(Integer.java:358)
        at java.lang.Integer.parseInt(Integer.java:334)
        at java.lang.Integer.valueOf(Integer.java:525)
        at com.mapbox.services.android.navigation.v5.navigation.RouteRefresh.refresh(RouteRefresh.java:80)
        at com.mapbox.services.android.navigation.v5.navigation.RouteRefresh.refresh(RouteRefresh.java:74)
        at com.mapbox.services.android.navigation.v5.navigation.RouteRefresher.refresh(RouteRefresher.java:33)
        at com.mapbox.services.android.navigation.v5.navigation.RouteProcessorRunnable.process(RouteProcessorRunnable.java:66)
        at com.mapbox.services.android.navigation.v5.navigation.RouteProcessorRunnable.run(RouteProcessorRunnable.java:46)
        at android.os.Handler.handleCallback(Handler.java:739)
        at android.os.Handler.dispatchMessage(Handler.java:95)
        at android.os.Looper.loop(Looper.java:148)
        at android.os.HandlerThread.run(HandlerThread.java:61)

Description

We should protect against this error. It seems it was produced while sitting in a stale navigation state for a while.

• I have added all related labels (bug, feature, new API(s), SEMVER, etc.)
• I have added the appropriate milestone and project boards

Implementation

Validity check that will fire error callback with error message when invalid.

Testing

Please describe the manual tests that you ran to verify your changes

• I have tested locally (including SNAPSHOT upstream dependencies if needed)
• I have tested via a test drive, or a simulation/mock location app
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes

Checklist

• My code follows the style guidelines of this project
• I have performed a self-review of my own code

[codecov-io]

Codecov Report

Merging #1909 into master will increase coverage by 0.28%.
The diff coverage is 85.71%.

@@             Coverage Diff              @@
##             master    #1909      +/-   ##
============================================
+ Coverage     35.04%   35.33%   +0.28%     
- Complexity     1060     1069       +9     
============================================
  Files           261      261              
  Lines          8848     8855       +7     
  Branches        667      668       +1     
============================================
+ Hits           3101     3129      +28     
+ Misses         5475     5452      -23     
- Partials        272      274       +2

[Guardiola31337]

We should protect against this error. It seems it was produced while sitting in a stale navigation state for a while.

Yeah, I believe that's coming from the periodic refresh call in RouteRefresher

mapbox-navigation-android/libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/navigation/RouteRefresher.java

Lines 31 to 34 in a30d970

	void refresh(RouteProgress routeProgress) {
	updateIsChecking(true);
	routeRefresh.refresh(routeProgress, new RouteRefresherCallback(mapboxNavigation, this));
	}

Wondering why is that happening though 🤔

Overall this looks good to me @danesfeder I've left some comments / questions to discuss before merging here. Great test coverage ❤️

[Guardiola31337]

NIT Could we extract this magic number into a constant?

[Guardiola31337]

Not sure if we should report this as an "error" until we know why the Refresh API is returning null after some time stale. Should we maybe stop the periodic refresh for this scenario? 🤔

[danesfeder]

I think it's fine to send the error, our behavior is to fail silently:

  @Override
  public void onError(RefreshError error) {
    Timber.w(error.getMessage());
    routeRefresher.updateIsChecking(false);
  }

I extracted the magic number 👍

[danesfeder]

Wondering why is that happening though

Yeah, good to protect against it for now. But we should keep an eye on this.

[Guardiola31337]

Thanks for addressing the feedback @danesfeder