search

mapbox / mapbox-navigation-android

Mapbox Navigation SDK for Android
https://docs.mapbox.com/android/navigation/overview/
Other
621 stars 319 forks source link

[Android Auto] Fatal Exception: java.lang.IllegalStateException: When a deep link is found the geoDeeplinkPlacesProvider needs to be set #7098

Open narko opened 1 year ago

narko commented 1 year ago

Android API: 33

Mapbox Navigation SDK version: 2.11.0 androidauto-v0.22.0

Steps to trigger behavior

N/A

Expected behavior

No crash

Actual behavior

The app is crashing due to an IllegalStateException potentially because the MapboxScreenManager is not set up on time.

Logs

Fatal Exception: java.lang.IllegalStateException: When a deep link is found the geoDeeplinkPlacesProvider needs to be set
       at com.mapbox.androidauto.screenmanager.factories.GeoDeeplinkPlacesCarScreenFactory.create(GeoDeeplinkPlacesCarScreenFactory.java:36)
       at com.mapbox.androidauto.screenmanager.MapboxScreenManager.onPush(MapboxScreenManager.java:45)
       at com.mapbox.androidauto.screenmanager.MapboxScreenManager.onScreenEvent(MapboxScreenManager.java:34)
       at com.mapbox.androidauto.screenmanager.MapboxScreenManager.access$onScreenEvent(MapboxScreenManager.java)
       at com.mapbox.androidauto.screenmanager.MapboxScreenManager$lifecycleObserver$1$onCreate$1$invokeSuspend$$inlined$collect$1.emit(MapboxScreenManager.java:4)
       at kotlinx.coroutines.flow.SharedFlowImpl.collect$suspendImpl(SharedFlowImpl.java:203)
       at kotlinx.coroutines.flow.SharedFlowImpl.collect(SharedFlowImpl.java)
       at kotlinx.coroutines.flow.ReadonlySharedFlow.collect(ReadonlySharedFlow.java:2)
       at com.mapbox.androidauto.screenmanager.MapboxScreenManager$lifecycleObserver$1$onCreate$1.invokeSuspend(MapboxScreenManager.java:41)
       at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(BaseContinuationImpl.java:11)
       at kotlinx.coroutines.internal.DispatchedContinuationKt.resumeCancellableWith(DispatchedContinuation.kt:132)
       at kotlinx.coroutines.intrinsics.CancellableKt.startCoroutineCancellable(Cancellable.kt:16)
       at kotlinx.coroutines.intrinsics.CancellableKt.startCoroutineCancellable$default(Cancellable.kt:5)
       at kotlinx.coroutines.CoroutineStart.invoke(CoroutineStart.java:41)
       at kotlinx.coroutines.AbstractCoroutine.start(AbstractCoroutine.java)
       at kotlinx.coroutines.BuildersKt__Builders_commonKt.launch(BuildersKt__Builders_common.kt:22)
       at kotlinx.coroutines.BuildersKt.launch(Builders.kt)
       at kotlinx.coroutines.BuildersKt__Builders_commonKt.launch$default(BuildersKt__Builders_common.kt:12)
       at kotlinx.coroutines.BuildersKt.launch$default(Builders.kt)
       at com.mapbox.androidauto.screenmanager.MapboxScreenManager$lifecycleObserver$1.onCreate(MapboxScreenManager.java:42)
       at androidx.lifecycle.FullLifecycleObserverAdapter.onStateChanged(FullLifecycleObserverAdapter.java:52)
       at androidx.lifecycle.LifecycleRegistry$ObserverWithState.dispatchEvent(LifecycleRegistry.java:14)
       at androidx.lifecycle.LifecycleRegistry.forwardPass(LifecycleRegistry.java:69)
       at androidx.lifecycle.LifecycleRegistry.sync(LifecycleRegistry.java:72)
       at androidx.lifecycle.LifecycleRegistry.moveToState(LifecycleRegistry.java:53)
       at androidx.lifecycle.LifecycleRegistry.handleLifecycleEvent(LifecycleRegistry.java:9)
       at androidx.car.app.Session$LifecycleObserverImpl.onCreate(Session.java:6)
       at androidx.lifecycle.FullLifecycleObserverAdapter.onStateChanged(FullLifecycleObserverAdapter.java:52)
       at androidx.lifecycle.LifecycleRegistry$ObserverWithState.dispatchEvent(LifecycleRegistry.java:14)
       at androidx.lifecycle.LifecycleRegistry.forwardPass(LifecycleRegistry.java:69)
       at androidx.lifecycle.LifecycleRegistry.sync(LifecycleRegistry.java:72)
       at androidx.lifecycle.LifecycleRegistry.moveToState(LifecycleRegistry.java:53)
       at androidx.lifecycle.LifecycleRegistry.handleLifecycleEvent(LifecycleRegistry.java:9)
       at androidx.car.app.Session.handleLifecycleEvent(Session.java:2)
       at androidx.car.app.CarAppBinder.lambda$onAppCreate$0(CarAppBinder.java:153)
       at androidx.car.app.utils.RemoteUtils.lambda$dispatchCallFromHost$0(RemoteUtils.java)
       at android.os.Handler.handleCallback(Handler.java:942)
       at android.os.Handler.dispatchMessage(Handler.java:99)
       at android.os.Looper.loopOnce(Looper.java:226)
       at android.os.Looper.loop(Looper.java:313)
       at android.app.ActivityThread.main(ActivityThread.java:8757)
       at java.lang.reflect.Method.invoke(Method.java)
       at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:571)
       at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1067)
narko commented 1 year ago

@kmadsen, we started seeing this issue in Android 13 that we cannot reproduce in our code. There must be something that is triggering a MapboxScreen.GEO_DEEPLINK push in some phones. I wonder, if we could relax a bit the condition in GeoDeeplinkPlacesCarScreenFactory and log the error instead of triggering an exception there.

kmadsen commented 1 year ago

Hey thanks for cutting the ticket and sorry I am just coming back from a vacation.

I think I see why this is possible.. The MapboxScreenManager companion object has a lifecycle that is longer than the MapboxCarContext. So the app can destroy the MapboxCarContext and the geoDeeplinkPlacesProvider, but it still thinks it's on the GEO_DEEPLINK screen. Upon re-launching the car head unit, it tries to reload the GEO_DEEPLINK screen but the geoDeeplinkPlacesProvider has been cleared - so it crashes.

Look into GeoDeeplinkNavigateAction. The code path to set the geoDeeplinkPlacesProvider comes from a regular Intent and geo formatted strings. So the exception would be impossible if the screen is shown with the GeoDeeplinkNavigateAction

I wonder, if we could relax a bit the condition in GeoDeeplinkPlacesCarScreenFactory and log the error instead of triggering an exception there.

This is fair. But, let me explain more because fixing the issue first may help us with SDK improvements. The exception is thrown to help developers know they need to use the GeoDeeplinkNavigateAction when implementing the session. 🤔 But the cause seems to be unrelated to GEO_DEEPLINK screen, it would happen to other screens too.

@narko Can you try resetting your screen when the Car lifecycle is destroyed? In the example app that would be here. https://github.com/mapbox/mapbox-navigation-android/blob/07ca7859ccdd56a40fa7c6df62ec32f4137ecac6/android-auto-app/src/main/java/com/mapbox/navigation/examples/androidauto/car/MainCarSession.kt#L102-L104

The default screen we have in the example is FREE_DRIVE, but it is possible that your default screen would be different than the one we have in the example

            override fun onDestroy(owner: LifecycleOwner) {
                mapboxCarMap.clearObservers()
                MapboxScreenManager.replaceTop(MapboxScreen.FREE_DRIVE)
            }
narko commented 1 year ago

Thanks @kmadsen for the detailed explanation.

I wonder how I could try to reproduce the issue for sharing further details about the lifecycle hint.

One thing I realized is that in our session we use mapboxMapInstaller().install(initializer) with a custom MapboxCarMapInitializer for initializing mapboxCarMap. It looks like this code already has an onDestroy implementation doing mapboxCarMap.clearObservers() https://github.com/mapbox/mapbox-maps-android/blob/main/extension-androidauto/src/main/java/com/mapbox/maps/extension/androidauto/MapboxCarMapSessionInstaller.kt#L92

Wouldn't this one be enough? Alternatively I could also override onDestroy in the session directly.