NAVAND-1546 is retryable (#7597)

changelog/unreleased/features/7597.md

@@ -0,0 +1,3 @@
+- Introduced `RouterFailure#isRetryable` which indicates if that makes sense to retry with this type of failure.
+For convenience, you can use an extension property `isRetryable` for the list of `RouterFailure` in `NavigationRouterCallback.onFailure`.
+In case of reroute use `RerouteState.Failed.isRetryable`.

instrumentation-tests/src/androidTest/java/com/mapbox/navigation/instrumentation_tests/core/RouteRequestTests.kt

@@ -0,0 +1,224 @@
+package com.mapbox.navigation.instrumentation_tests.core
+
+import android.location.Location
+import com.mapbox.api.directions.v5.models.RouteOptions
+import com.mapbox.geojson.Point
+import com.mapbox.navigation.base.extensions.applyDefaultNavigationOptions
+import com.mapbox.navigation.base.route.isRetryable
+import com.mapbox.navigation.instrumentation_tests.R
+import com.mapbox.navigation.instrumentation_tests.utils.history.MapboxHistoryTestRule
+import com.mapbox.navigation.instrumentation_tests.utils.readRawFileText
+import com.mapbox.navigation.instrumentation_tests.utils.withMapboxNavigation
+import com.mapbox.navigation.instrumentation_tests.utils.withoutInternet
+import com.mapbox.navigation.testing.ui.BaseCoreNoCleanUpTest
+import com.mapbox.navigation.testing.ui.http.MockRequestHandler
+import com.mapbox.navigation.testing.ui.utils.coroutines.RouteRequestResult
+import com.mapbox.navigation.testing.ui.utils.coroutines.requestRoutes
+import com.mapbox.navigation.testing.ui.utils.coroutines.sdkTest
+import okhttp3.mockwebserver.MockResponse
+import org.junit.Assert.assertFalse
+import org.junit.Assert.assertTrue
+import org.junit.Rule
+import org.junit.Test
+
+/**
+ * See https://docs.mapbox.com/api/navigation/directions/#directions-api-errors for info
+ * about different Directions API failures
+ */
+class RouteRequestTests : BaseCoreNoCleanUpTest() {
+
+    @get:Rule
+    val mapboxHistoryTestRule = MapboxHistoryTestRule()
+
+    private val origin = Point.fromLngLat(
+        13.361378213031003,
+        52.49813341962201
+    )
+
+    override fun setupMockLocation(): Location {
+        return mockLocationUpdatesRule.generateLocationUpdate {
+            longitude = origin.longitude()
+            latitude = origin.latitude()
+        }
+    }
+
+    @Test
+    fun requestRouteWithoutInternetAndTiles() = sdkTest {
+        withMapboxNavigation(
+            historyRecorderRule = mapboxHistoryTestRule
+        ) { navigation ->
+            withoutInternet {
+                val routes = navigation.requestRoutes(createTestRouteOptions())
+                assertTrue(routes is RouteRequestResult.Failure)
+                val isRetryable = (routes as RouteRequestResult.Failure).reasons.isRetryable
+                assertTrue(isRetryable)
+            }
+        }
+    }
+
+    @Test
+    fun error500Unknown() = sdkTest {
+        mockWebServerRule.requestHandlers.add(
+            MockRequestHandler {
+                MockResponse().setBody("unexpected server error").setResponseCode(500)
+            }
+        )
+        withMapboxNavigation(
+            historyRecorderRule = mapboxHistoryTestRule
+        ) { navigation ->
+            val routes = navigation.requestRoutes(createTestRouteOptions())
+            assertTrue(routes is RouteRequestResult.Failure)
+            val isRetryable = (routes as RouteRequestResult.Failure).reasons.isRetryable
+            assertFalse(isRetryable)
+        }
+    }
+
+    @Test
+    fun invalidInput() = sdkTest {
+        mockWebServerRule.requestHandlers.add(
+            MockRequestHandler {
+                MockResponse()
+                    .setBody(readRawFileText(context, R.raw.invalid_alternative_response_body))
+                    .setResponseCode(422)
+            }
+        )
+        withMapboxNavigation(
+            historyRecorderRule = mapboxHistoryTestRule
+        ) { navigation ->
+            val routes = navigation.requestRoutes(createTestRouteOptions())
+            assertTrue(routes is RouteRequestResult.Failure)
+            val isRetryable = (routes as RouteRequestResult.Failure).reasons.isRetryable
+            assertFalse(isRetryable)
+        }
+    }
+
+    @Test
+    fun wrongSegment() = sdkTest {
+        mockWebServerRule.requestHandlers.add(
+            MockRequestHandler {
+                MockResponse()
+                    .setBody(readRawFileText(context, R.raw.wrong_segment_response_body))
+                    .setResponseCode(200)
+            }
+        )
+        withMapboxNavigation(
+            historyRecorderRule = mapboxHistoryTestRule
+        ) { navigation ->
+            val routes = navigation.requestRoutes(createTestRouteOptions())
+            assertTrue(routes is RouteRequestResult.Failure)
+            val isRetryable = (routes as RouteRequestResult.Failure).reasons.isRetryable
+            assertFalse(isRetryable)
+        }
+    }
+
+    @Test
+    fun noRouteFound() = sdkTest {
+        mockWebServerRule.requestHandlers.add(
+            MockRequestHandler {
+                MockResponse()
+                    .setBody("{\"code\":\"NoRoute\",\"message\":\"No route found\",\"routes\":[]}")
+                    .setResponseCode(200)
+            }
+        )
+        withMapboxNavigation(
+            historyRecorderRule = mapboxHistoryTestRule
+        ) { navigation ->
+            val routes = navigation.requestRoutes(createTestRouteOptions())
+            assertTrue(routes is RouteRequestResult.Failure)
+            val isRetryable = (routes as RouteRequestResult.Failure).reasons.isRetryable
+            assertFalse(isRetryable)
+        }
+    }
+
+    @Test
+    fun invalidAccessToken() = sdkTest {
+        mockWebServerRule.requestHandlers.add(
+            MockRequestHandler {
+                MockResponse()
+                    .setBody("{\"message\":\"Not Authorized - Invalid Token\"}")
+                    .setResponseCode(401)
+            }
+        )
+        withMapboxNavigation(
+            historyRecorderRule = mapboxHistoryTestRule
+        ) { navigation ->
+            val routes = navigation.requestRoutes(createTestRouteOptions())
+            assertTrue(routes is RouteRequestResult.Failure)
+            val isRetryable = (routes as RouteRequestResult.Failure).reasons.isRetryable
+            assertFalse(isRetryable)
+        }
+    }
+
+    @Test
+    fun noAccessToken() = sdkTest {
+        mockWebServerRule.requestHandlers.add(
+            MockRequestHandler {
+                MockResponse()
+                    .setBody(readRawFileText(context, R.raw.no_access_token_response_body))
+                    .setResponseCode(401)
+            }
+        )
+        withMapboxNavigation(
+            historyRecorderRule = mapboxHistoryTestRule
+        ) { navigation ->
+            val routes = navigation.requestRoutes(createTestRouteOptions())
+            assertTrue(routes is RouteRequestResult.Failure)
+            val isRetryable = (routes as RouteRequestResult.Failure).reasons.isRetryable
+            assertFalse(isRetryable)
+        }
+    }
+
+    @Test
+    fun forbidden() = sdkTest {
+        mockWebServerRule.requestHandlers.add(
+            MockRequestHandler {
+                MockResponse()
+                    .setBody("{\"message\":\"Forbidden\"}")
+                    .setResponseCode(403)
+            }
+        )
+        withMapboxNavigation(
+            historyRecorderRule = mapboxHistoryTestRule
+        ) { navigation ->
+            val routes = navigation.requestRoutes(createTestRouteOptions())
+            assertTrue(routes is RouteRequestResult.Failure)
+            val isRetryable = (routes as RouteRequestResult.Failure).reasons.isRetryable
+            assertFalse(isRetryable)
+        }
+    }
+
+    @Test
+    fun profileNotFound() = sdkTest {
+        mockWebServerRule.requestHandlers.add(
+            MockRequestHandler {
+                MockResponse()
+                    .setBody("{\"message\":\"Profile not found\"}")
+                    .setResponseCode(401)
+            }
+        )
+        withMapboxNavigation(
+            historyRecorderRule = mapboxHistoryTestRule
+        ) { navigation ->
+            val routes = navigation.requestRoutes(createTestRouteOptions())
+            assertTrue(routes is RouteRequestResult.Failure)
+            val isRetryable = (routes as RouteRequestResult.Failure).reasons.isRetryable
+            assertFalse(isRetryable)
+        }
+    }
+
+    private fun createTestRouteOptions(): RouteOptions {
+        return RouteOptions.builder()
+            .baseUrl(mockWebServerRule.baseUrl)
+            .applyDefaultNavigationOptions()
+            .coordinatesList(
+                listOf(
+                    origin,
+                    Point.fromLngLat(
+                        13.361478213031003,
+                        52.49823341962201
+                    )
+                )
+            )
+            .build()
+    }
+}

instrumentation-tests/src/androidTest/java/com/mapbox/navigation/instrumentation_tests/utils/location/LocationUpdate.kt

@@ -1,5 +1,6 @@
 package com.mapbox.navigation.instrumentation_tests.utils.location
 
+import android.location.Location
 import com.mapbox.navigation.core.MapboxNavigation
 import com.mapbox.navigation.core.internal.extensions.flowLocationMatcherResult
 import com.mapbox.navigation.testing.ui.BaseCoreNoCleanUpTest
@@ -37,6 +38,20 @@ suspend fun BaseCoreNoCleanUpTest.stayOnPosition(
     }
 }
 
+suspend fun BaseCoreNoCleanUpTest.stayOnPosition(
+    location: Location,
+    frequencyHz: Int = 1,
+    block: suspend () -> Unit
+) {
+    stayOnPosition(
+        latitude = location.latitude,
+        longitude = location.longitude,
+        bearing = location.bearing,
+        frequencyHz = frequencyHz,
+        block = block,
+    )
+}
+
 suspend fun BaseCoreNoCleanUpTest.stayOnPositionAndWaitForUpdate(
     mapboxNavigation: MapboxNavigation,
     latitude: Double,

instrumentation-tests/src/main/res/raw/invalid_alternative_response_body.json

@@ -0,0 +1,4 @@
+{
+  "message": "annotations value must be one of duration, distance, speed, congestion, congestion_numeric, closure, state_of_charge, maxspeed",
+  "code": "InvalidInput"
+}

instrumentation-tests/src/main/res/raw/no_access_token_response_body.json

@@ -0,0 +1,4 @@
+{
+  "message": "Not Authorized - Invalid Token",
+  "error_detail": "No valid token prefix found in access_token parameter"
+}

instrumentation-tests/src/main/res/raw/wrong_segment_response_body

@@ -0,0 +1,5 @@
+{
+    "code": "NoSegment",
+    "message": "Could not find a matching segment for input coordinates",
+    "routes": []
+}

libnavigation-base/src/main/java/com/mapbox/navigation/base/internal/route/RetryableThrowable.kt

@@ -0,0 +1,10 @@
+package com.mapbox.navigation.base.internal.route
+
+import com.mapbox.navigation.base.route.RouterFailure
+
+/**
+ * It exists in order not to break API of data class [RouterFailure] by adding boolean field,
+ * instead existing fields are used to transfer more data.
+ */
+@Deprecated("replace by a boolean filed in RouterFailure")
+class RetryableThrowable : Throwable(message = "It makes sense to retry in case of that error")

libnavigation-base/src/main/java/com/mapbox/navigation/base/route/Router.kt

@@ -2,6 +2,7 @@ package com.mapbox.navigation.base.route
 
 import com.mapbox.api.directions.v5.models.DirectionsRoute
 import com.mapbox.api.directions.v5.models.RouteOptions
+import com.mapbox.navigation.base.internal.route.RetryableThrowable
 import java.net.URL
 
 /**
@@ -78,4 +79,10 @@ data class RouterFailure @JvmOverloads constructor(
     val message: String,
     val code: Int? = null,
     val throwable: Throwable? = null
-)
+) {
+    /**
+     * Indicates if it makes sense to retry for this type of failure.
+     * If false, it doesn't make sense to retry route request
+     */
+    val isRetryable get() = throwable is RetryableThrowable
+}

libnavigation-base/src/main/java/com/mapbox/navigation/base/route/RouterEx.kt

@@ -0,0 +1,8 @@
+package com.mapbox.navigation.base.route
+
+/**
+ * Indicates if it makes sense to retry for this type of failures.
+ * If false, it doesn't make sense to retry route request
+ */
+val List<RouterFailure>?.isRetryable: Boolean
+    get() = this?.any { it.isRetryable } ?: false

libnavigation-core/src/main/java/com/mapbox/navigation/core/reroute/RerouteController.kt

@@ -4,6 +4,7 @@ import androidx.annotation.UiThread
 import com.mapbox.api.directions.v5.models.DirectionsRoute
 import com.mapbox.navigation.base.route.RouterFailure
 import com.mapbox.navigation.base.route.RouterOrigin
+import com.mapbox.navigation.base.route.isRetryable
 import com.mapbox.navigation.core.MapboxNavigation
 import com.mapbox.navigation.core.routeoptions.RouteOptionsUpdater
 import com.mapbox.navigation.core.trip.session.OffRouteObserver
@@ -110,7 +111,14 @@ sealed class RerouteState {
         val message: String,
         val throwable: Throwable? = null,
         val reasons: List<RouterFailure>? = null
-    ) : RerouteState()
+    ) : RerouteState() {
+
+        /**
+         * Indicates if it makes sense to retry for this type of failures.
+         * If false, it doesn't make sense to retry route request
+         */
+        val isRetryable get() = reasons.isRetryable
+    }
 
     /**
      * Route fetching is in progress.

libnavigation-router/src/main/java/com/mapbox/navigation/route/internal/RouterWrapper.kt

@@ -12,6 +12,7 @@ import com.mapbox.navigation.base.ExperimentalMapboxNavigationAPI
 import com.mapbox.navigation.base.internal.NavigationRouterV2
 import com.mapbox.navigation.base.internal.RouteRefreshRequestData
 import com.mapbox.navigation.base.internal.route.InternalRouter
+import com.mapbox.navigation.base.internal.route.RetryableThrowable
 import com.mapbox.navigation.base.internal.route.refreshRoute
 import com.mapbox.navigation.base.internal.route.updateExpirationTime
 import com.mapbox.navigation.base.internal.utils.Constants
@@ -86,12 +87,20 @@ class RouterWrapper(
                                 )
                                 callback.onCanceled(routeOptions, origin.mapToSdkRouteOrigin())
                             } else {
+                                val isErrorRetryable = it.type in listOf(
+                                    RouterErrorType.NETWORK_ERROR
+                                )
                                 val failureReasons = listOf(
                                     RouterFailure(
                                         url = urlWithoutToken,
                                         routerOrigin = origin.mapToSdkRouteOrigin(),
                                         message = it.message,
-                                        code = it.code
+                                        code = it.code,
+                                        throwable = if (isErrorRetryable) {
+                                            RetryableThrowable()
+                                        } else {
+                                            null
+                                        }
                                     )
                                 )