[Bug]: Exception in ViewTagResolver after app reloading

[orca-nazar]

Mapbox Implementation

Mapbox

Mapbox Version

10.16.2

Platform

Android

@rnmapbox/maps version

#main

Standalone component to reproduce

// I could not reproduce it in the Example app 
export default () => {};

Observed behavior and steps to reproduce

The issue occurs after hot reloading or reloading via terminal in debug mode. To avoid it only helps full(force) closing the app

Expected behavior

No issues

Notes / preliminary analysis

Looks like making the manager optional and adding guards when extracting it, solves the problem

private val manager : UIManager?
        get() =
            if (BuildConfig.IS_NEW_ARCHITECTURE_ENABLED) {
                UIManagerHelper.getUIManager(context, UIManagerType.FABRIC)
            } else {
                UIManagerHelper.getUIManager(context, UIManagerType.DEFAULT)
            }

Additional links and references

[mfazekas]

Looks like making the manager optional and adding guards when extracting it, solves the problem

Not solves just hides the issue. Without view tag resolver we will not be able to call methods on the components like MapView#getCenter.

[orca-nazar]

Looks like making the manager optional and adding guards when extracting it, solves the problem

Not solves just hides the issue. Without view tag resolver we will not be able to call methods on the components like MapView#getCenter.

Yes, I see. Maybe there is a way to avoid red screens, when exception occurs. I guess it should be handled by try/catch block, right? @mfazekas

[orca-nazar]

@mfazekas If we move out viewWaiters from catch and handle it in try block. Do you think it might work for all cases?

    fun <V>withViewResolved(viewTag: Int, reject: Promise? = null, fn: (V) -> Unit) {
        context.runOnUiQueueThread() {
            try {
                val view = manager?.resolveView(viewTag)

                if (view == null && !createdViews.contains(viewTag)) {
                    viewWaiters.getOrPut(viewTag) { mutableListOf<ViewTagWaiter<View>>() }.add(ViewTagWaiter<View>({ view -> fn(view as V) }, reject))
                } else if (view == null && createdViews.contains(viewTag)) {
                    reject?.reject(Exception("No view"))
                } else {
                    fn(view as V)
                }
            } catch (err: IllegalViewOperationException) {
                if (!createdViews.contains(viewTag)) {
                    viewWaiters.getOrPut(viewTag) { mutableListOf<ViewTagWaiter<View>>() }.add(ViewTagWaiter<View>({ view -> fn(view as V) }, reject))
                } else {
                    reject?.reject(err)
                }
            }
        }
    }

Updated: I see, that catch should be handled in an original way as IllegalViewOperationException still occurs

[mfazekas]

So the crash is coming from

                UIManagerHelper.getUIManager(context, UIManagerType.DEFAULT)!!

And we need that UI manager for finding the native views for viewTag. (JS side passes methods with viewTags and we need to find the native view using the viewTag.)

Does it say anything int the logcat? Which RN version do you use?

[orca-nazar]

RN: 0.71.8
On initial loading UI manager is available, however IllegalViewOperationException is thrown (it's covered by catch block). After reloading either via metro bundler or RN context menu, the manager becomes null and createdViews are not empty. So it goes to this condition: reject?.reject(Exception("No view")). Actually, it should be another message here

[WoLewicki]

IIRC for some reason after refresh, the catalyst instance is not present and this check fails: https://github.com/facebook/react-native/blob/31005b7fd9a9579920b55f921c477a90fb3f6d10/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContext.java#L240 when calling UIManagerHelper.getUIManager(context, UIManagerType.FABRIC)!! (at least on new arch). It would be nice to find out why it happens and if we are doing something wrong.

[mfazekas]

@WoLewicki do you see it on new arch? In 0.73?

[WoLewicki]

Yes

[mfazekas]

Yep I see it on our example app. But for me react-navigation also doesn't works after a refresh. Does refresh works if @rnmapbox/maps not installed?

@orca-nazar do you also have the new arch enabled?

2023-12-08 17:35:28.365 19202-19202 unknown:Vi...rtyUpdater com.rnmapboxglexample                W  Could not find generated setter for class com.facebook.react.uimanager.RootViewManager
2023-12-08 17:35:28.370 19202-19202 unknown:co...agerHelper com.rnmapboxglexample                E  Unhandled SoftException
                                                                                                    com.facebook.react.bridge.ReactNoCrashSoftException: Cannot get UIManager because the context doesn't contain an active CatalystInstance.
                                                                                                    	at com.facebook.react.uimanager.UIManagerHelper.getUIManager(UIManagerHelper.java:77)
                                                                                                    	at com.facebook.react.uimanager.UIManagerHelper.getEventDispatcher(UIManagerHelper.java:128)
                                                                                                    	at com.facebook.react.uimanager.UIManagerHelper.getEventDispatcherForReactTag(UIManagerHelper.java:106)
                                                                                                    	at com.th3rdwave.safeareacontext.SafeAreaProviderManagerKt.handleOnInsetsChange(SafeAreaProviderManager.kt:39)
                                                                                                    	at com.th3rdwave.safeareacontext.SafeAreaProviderManagerKt.access$handleOnInsetsChange(SafeAreaProviderManager.kt:1)
                                                                                                    	at com.th3rdwave.safeareacontext.SafeAreaProviderManager$addEventEmitters$1.invoke(SafeAreaProviderManager.kt:28)
                                                                                                    	at com.th3rdwave.safeareacontext.SafeAreaProviderManager$addEventEmitters$1.invoke(SafeAreaProviderManager.kt:28)
                                                                                                    	at com.th3rdwave.safeareacontext.SafeAreaProvider.maybeUpdateInsets(SafeAreaProvider.kt:21)
                                                                                        

[orca-nazar]

Nope, the error happens here UIManagerHelper.getUIManager(context, UIManagerType.DEFAULT)

[mfazekas]

FWIW For the Fabric issues on reload went away for me since I've update reps in our example app. (I've upgraded react-native-safe-area-context and react-native-screens and removed react-native-svg https://github.com/rnmapbox/maps/pull/3257/files) But I'm not 100% if that this is the solution of the issue for the fabric case.

[WoLewicki]

Hmm I am afraid it is something we would want to fix upstream in the RN core, since we use methods from the UIManagerHelper and they fail. cc @cortinico are you aware of such issue, the catalyst instance being not present after resfresh of the app? Or maybe we are using it in the wrong way?

[cortinico]

cc @cortinico are you aware of such issue, the catalyst instance being not present after resfresh of the app? Or maybe we are using it in the wrong way?

Sorry but it's really hard to say as I lack context on what is the issue

[WoLewicki]

@mfazekas could we make a simple example where the bug is easily reproducible?

[dpyeates]

I'm seeing this as well. First load works OK, but if you put the app into the background and then bring it back active I get an Exception in Native call from JS in ViewTagResolver.kt:51.

For info, I am NOT using new arch and I see this on both Mapbox 10 and 11.

"react-native": "^0.73.1",
"@rnmapbox/maps": "^10.1.3",
"react-native-safe-area-context": "^4.8.2",
"react-native-screens": "^3.29.0",

[dpyeates]

Any update on this? I know its a quiet time of year with Christmas and New Year but I'm getting multiple daily emails from users saying my application runs fine the first time after installation but every subsequent start the app crashes straight away.

Big thanks to all and Happy New Year everyone.

[mfazekas]

@dpyeates no I had not looked into this issue nor do I plan to. I cannot reproduce the issue in development (with or without fabric), so it would be very hard for me to do so.

What I suggest you to do. Assumption: the reload in debug issue relates to production, and you can repro the issue in your debug build.
1.) Simplify your MapView usage to see if you can still repro the issue (ideally you just replace your App.js with a simple )
2.) Try to reproduce the issue on a blank rn project (created with npx [email protected] init AwesomeProject --version 0.71.8
If you cannot, then add packages you're using to the blank project.

[VolkerLieber]

It seems to be reproducible for me by calling

import ExpoUpdates from "expo-updates/src/ExpoUpdates";
ExpoUpdates.reload();

I'll look into it if I find some time, because it screws up our hotfix procedure, but maybe this helps you reproduce the issue on your side.

Error:

FATAL EXCEPTION: expo-updates-error-recovery

java.lang.NullPointerException
	at com.rnmapbox.rnmbx.utils.ViewTagResolver.getManager(ViewTagResolver.kt:54)
	at com.rnmapbox.rnmbx.utils.ViewTagResolver.withViewResolved$lambda$4(ViewTagResolver.kt:61)
	at com.rnmapbox.rnmbx.utils.ViewTagResolver.$r8$lambda$ZFXWzquiK28lSR5tbH0BihabahM(Unknown Source:0)
	at com.rnmapbox.rnmbx.utils.ViewTagResolver$$ExternalSyntheticLambda0.run(Unknown Source:8)
	at android.os.Handler.handleCallback(Handler.java:958)
	at android.os.Handler.dispatchMessage(Handler.java:99)
	at com.facebook.react.bridge.queue.MessageQueueThreadHandler.dispatchMessage(MessageQueueThreadHandler.java:27)
	at android.os.Looper.loopOnce(Looper.java:224)
	at android.os.Looper.loop(Looper.java:318)
	at android.app.ActivityThread.main(ActivityThread.java:8770)
	at java.lang.reflect.Method.invoke(Native Method)
	at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:561)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1013)

[mfazekas]

@VolkerLieber so you're using rnmapbox then applying expo update, then try to use rnmapbox again?

import {
  reloadAsync,
} from 'expo-updates'

It's kind of a tricky situation. And I can image some code is @rnmapbox/maps is not prepared for this, because normally this doesn't happen.

(FWIW Even current expo has issue with applying update in the middle of an app run: expo/expo#32073)

[VolkerLieber]

@mfazekas The update is requested on startup, the reload is triggered after download. At this point, the map screen should probably not be loaded, because we wait until the whole update procedure before we continue.

Good find with expo notifications 😄