appcues / appcues-react-native-module

The Appcues React Native Module
MIT License
11 stars 0 forks source link

Android IllegalArgumentException in 3.1.7 #132

Open ashfurrow opened 10 months ago

ashfurrow commented 10 months ago

Hello šŸ‘‹ Following up on #128 (where I mentioned an Android crash I was seeing) and #129 (where the crashing code was wrapped in a try/catch), I'm still seeing the following crash:

No view found for id 0x77 (unknown) for fragment AppcuesWrapperFragment{fa93181} (1880fa72-8c2b-46da-aec0-cc4112b699d9 id=0x77 tag=119)

Exception java.lang.IllegalArgumentException:
  at androidx.fragment.app.FragmentStateManager.createView (FragmentStateManager.java:513)
  at androidx.fragment.app.FragmentStateManager.moveToExpectedState (FragmentStateManager.java:282)
  at androidx.fragment.app.FragmentManager.executeOpsTogether (FragmentManager.java:2189)
  at androidx.fragment.app.FragmentManager.removeRedundantOperationsAndExecute (FragmentManager.java:2100)
  at androidx.fragment.app.FragmentManager.execPendingActions (FragmentManager.java:2002)
  at androidx.fragment.app.FragmentManager$5.run (FragmentManager.java:524)
  at android.os.Handler.handleCallback (Handler.java:942)
  at android.os.Handler.dispatchMessage (Handler.java:99)
  at android.os.Looper.loopOnce (Looper.java:201)
  at android.os.Looper.loop (Looper.java:288)
  at android.app.ActivityThread.main (ActivityThread.java:7918)
  at java.lang.reflect.Method.invoke
  at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run (RuntimeInit.java:548)
  at com.android.internal.os.ZygoteInit.main (ZygoteInit.java:936)

For context, an issue prevented me from releasing the update with 3.1.7 in it until last week and I've only seen this crash happen once so far on that version. But both the Play Console and Bugsnag are reporting this as an unhandled exception/error. It could be that they are grouping this together with the previous crash, but judging by the stack trace, it's not a problem that would be handled by the try/catch from #129.

Any advice would be appreciated, thanks again!

iujames commented 10 months ago

Hi @ashfurrow, thanks for your input here - we've actually had one other similar report and are actively looking into this one. So far, it has been challenging to reproduce. The other report indicated that it seemed likely tied to a scenario where the app was cold launched from a push notification (not already running). Perhaps your app has a similar possible use case?

The AppcuesWrapperFragment here, as you may already know, is tied to the usage of AppcuesFrameView instances in the React Native app - Embed frames. Our other bug report confirmed, no crash is seen if no AppcuesFrameView is included in the layout. Of course, we still want to make those work. The AppcuesFrameView is using the Android Native UI Components approach from React Native.

The Fragment gets set up and added to the SupportFragmentManager in a Transaction here. I'm wondering if there is some sort of timing issue with the way RN is doing layout and how the native Views are getting created. At this line of code, the current AppcuesWrapperView has an id (resource id) value that is being used to provide the spot for the fragment to replace in the UI when the transaction is committed. Since there is no immediate issue, there is no exception thrown or caught on L65 here.

This fragment transaction commit runs asynchronously, as described in the [developer docs](https://developer.android.com/reference/androidx/fragment/app/FragmentTransaction#commit()). One thing I'm wondering about is trying [commitNow](https://developer.android.com/reference/androidx/fragment/app/FragmentTransaction#commitNow()) instead, to see if it would change any behavior around async timing that we're seeing with RN applications.

So far, I've not been able to reproduce this issue exactly. I've tried some different sample app configs, and techniques to cold launch the app from a deep link to try to simulate what might be happening. If you have any leads on steps to repro that might help pinpoint, that would certainly help us to narrow in on a solution.

iujames commented 10 months ago

After some further debugging, I believe we may have a potential fix for this issue in #133, available for any feedback.

iujames commented 10 months ago

@ashfurrow - the version 3.1.8 update just released contains the change in #133 to try out - hopefully this will resolve it!

ashfurrow commented 10 months ago

Hello, and thank you for the quick response!

The other report indicated that it seemed likely tied to a scenario where the app was cold launched from a push notification (not already running). Perhaps your app has a similar possible use case?

That is the case. From what I can tell, the user received a push notification, opened it, logged in, and then our app navigated to the screen with the notification content. That screen didn't have any Appcues components rendered on it, but the initialRouteName screen (from react-navigation) does. That screen is never actually navigated to, and I can't tell from the breadcrumbs if it is even rendered, but it does have a AppcuesFrameView in its component tree.

I'll update and get that deployed, thank you!

iujames commented 10 months ago

@ashfurrow a heads up that an additional proposed update is in #134, to address an observed issue. While no longer crashing, there could be scenarios where the embed content would not load as expected on the first render of the screen - requiring navigating away and back to see the embed content. We believe this update should resolve that part, and ensure fully expected behavior for embeds in React Native Android applications. Once testing completes successfully, we'll release an updated version with that change as well.

ashfurrow commented 10 months ago

Understood, thanks for the transparency šŸ‘ That shouldn't affect our current use case anyway, but it's good to know.

iujames commented 10 months ago

Our 3.1.9 update has been released, containing the updated fix to handle the first render of embed content on the screen. We believe all issues here are resolved at this point, so going ahead and closing this issue. Please let us know if you have any additional feedback or notice any thing else not working as expected.

ashfurrow commented 9 months ago

I think this might be related; we just got a crash in 3.1.8 of the SDK:

java.lang.IllegalStateException: Cannot locate windowRecomposer; View androidx.compose.ui.platform.ComposeView{56d86ef V.E...... ......I. 0,0-0,0} is not attached to a window
        at androidx.compose.ui.platform.WindowRecomposer_androidKt.getWindowRecomposer(WindowRecomposer.android.kt:295)
        at androidx.compose.ui.platform.AbstractComposeView.resolveParentCompositionContext(ComposeView.android.kt:244)
        at androidx.compose.ui.platform.AbstractComposeView.ensureCompositionCreated(ComposeView.android.kt:251)
        at androidx.compose.ui.platform.AbstractComposeView.onMeasure(ComposeView.android.kt:288)
        at android.view.View.measure(View.java:28531)
        at android.view.ViewGroup.measureChildWithMargins(ViewGroup.java:7386)
        at android.widget.FrameLayout.onMeasure(FrameLayout.java:194)
        at android.view.View.measure(View.java:28531)
        at com.appcuesreactnative.AppcuesWrapperView.onMeasure(AppcuesFrameViewManager.kt:99)
        at android.view.View.measure(View.java:28531)
        at com.facebook.react.uimanager.NativeViewHierarchyManager.updateLayout(NativeViewHierarchyManager.java:189)
        at com.swmansion.reanimated.layoutReanimation.ReanimatedNativeHierarchyManager.updateLayout(ReanimatedNativeHierarchyManager.java:256)
        at com.facebook.react.uimanager.UIViewOperationQueue$UpdateLayoutOperation.execute(UIViewOperationQueue.java:169)
        at com.facebook.react.uimanager.UIViewOperationQueue$1.run(UIViewOperationQueue.java:915)
        at com.facebook.react.uimanager.UIViewOperationQueue.flushPendingBatches(UIViewOperationQueue.java:1026)
        at com.facebook.react.uimanager.UIViewOperationQueue.-$$Nest$mflushPendingBatches(Unknown)
        at com.facebook.react.uimanager.UIViewOperationQueue$DispatchUIFrameCallback.doFrameGuarded(UIViewOperationQueue.java:1086)
        at com.facebook.react.uimanager.GuardedFrameCallback.doFrame(GuardedFrameCallback.java:29)
        at com.facebook.react.modules.core.ReactChoreographer$ReactChoreographerDispatcher.doFrame(ReactChoreographer.java:175)
        at com.facebook.react.modules.core.ChoreographerCompat$FrameCallback$1.doFrame(ChoreographerCompat.java:85)
        at android.view.Choreographer$CallbackRecord.run(Choreographer.java:1648)
        at android.view.Choreographer$CallbackRecord.run(Choreographer.java:1659)
        at android.view.Choreographer.doCallbacks(Choreographer.java:1129)
        at android.view.Choreographer.doFrame(Choreographer.java:1045)
        at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:1622)
        at android.os.Handler.handleCallback(Handler.java:958)
        at android.os.Handler.dispatchMessage(Handler.java:99)
        at android.os.Looper.loopOnce(Looper.java:230)
        at android.os.Looper.loop(Looper.java:319)
        at android.app.ActivityThread.main(ActivityThread.java:8893)
        at java.lang.reflect.Method.invoke(Method.java:-2)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:608)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1103)

This was on a Galaxy S22 running Android 34. 600ms before the exception, the user had navigated to a new screen. Let me know if I should open a new issue or if I can provider further details.

iujames commented 9 months ago

Hi @ashfurrow, thanks for letting us know! I am curious if you have seen this in 3.1.9 or later versions as well? (if you had released any of those). This fix in 3.1.9 hopefully fixed the underlying issue that could lead to this "not attached to a window" situation (fragment not getting attached).

However, while analyzing this particular error, it seems like it might still be possible that this line could be executed (in the onMeasure noted in the callstack above), even if this fragment addition failed. And perhaps we can add another safeguard in the onMeasure override to only execute that code if fragmentCreated == true (L67 reached)

We will look into adding this extra check, but again hopefully this situation would not occur with the 3.1.9 update included as well. Thanks!

ashfurrow commented 9 months ago

Gotcha. We haven't upgraded past 3.1.8 yet but I'll get a new release with the latest version and let you know šŸ‘

ashfurrow commented 9 months ago

We have seen just the one Android exception since upgrading, to 3.1.11.

java.lang.IllegalArgumentException: No view found for id 0x63 (unknown) for fragment AppcuesWrapperFragment{5ab6f86} (7dce5c18-a0c7-4ba0-a813-2d014a9cb43c id=0x63 tag=99)
        at androidx.fragment.app.FragmentStateManager.createView(FragmentStateManager.java:513)
        at androidx.fragment.app.FragmentStateManager.moveToExpectedState(FragmentStateManager.java:282)
        at androidx.fragment.app.FragmentStore.moveToExpectedState(FragmentStore.java:112)
        at androidx.fragment.app.FragmentManager.moveToState(FragmentManager.java:1647)
        at androidx.fragment.app.FragmentManager.executeOpsTogether(FragmentManager.java:2196)
        at androidx.fragment.app.FragmentManager.removeRedundantOperationsAndExecute(FragmentManager.java:2106)
        at androidx.fragment.app.FragmentManager.execSingleAction(FragmentManager.java:1971)
        at androidx.fragment.app.BackStackRecord.commitNowAllowingStateLoss(BackStackRecord.java:311)
        at com.swmansion.rnscreens.ScreenStack.onUpdate(ScreenStack.kt:223)
        at com.swmansion.rnscreens.ScreenContainer.performUpdates(ScreenContainer.kt:310)
        at com.swmansion.rnscreens.ScreensShadowNode.onBeforeLayout$lambda$0(ScreensShadowNode.kt:15)
        at com.swmansion.rnscreens.ScreensShadowNode.$r8$lambda$QEuKUJdDcTLrP4_Kyq3N9u5-WdI(Unknown)
        at com.swmansion.rnscreens.ScreensShadowNode$$ExternalSyntheticLambda0.execute(Unknown:2)
        at com.facebook.react.uimanager.UIViewOperationQueue$UIBlockOperation.execute(UIViewOperationQueue.java:579)
        at com.facebook.react.uimanager.UIViewOperationQueue$1.run(UIViewOperationQueue.java:915)
        at com.facebook.react.uimanager.UIViewOperationQueue.flushPendingBatches(UIViewOperationQueue.java:1026)
        at com.facebook.react.uimanager.UIViewOperationQueue.-$$Nest$mflushPendingBatches(Unknown)
        at com.facebook.react.uimanager.UIViewOperationQueue$DispatchUIFrameCallback.doFrameGuarded(UIViewOperationQueue.java:1086)
        at com.facebook.react.uimanager.GuardedFrameCallback.doFrame(GuardedFrameCallback.java:29)
        at com.facebook.react.modules.core.ReactChoreographer$ReactChoreographerDispatcher.doFrame(ReactChoreographer.java:175)
        at com.facebook.react.modules.core.ChoreographerCompat$FrameCallback$1.doFrame(ChoreographerCompat.java:85)
        at android.view.Choreographer$CallbackRecord.run(Choreographer.java:1648)
        at android.view.Choreographer$CallbackRecord.run(Choreographer.java:1659)
        at android.view.Choreographer.doCallbacks(Choreographer.java:1129)
        at android.view.Choreographer.doFrame(Choreographer.java:1045)
        at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:1622)
        at android.os.Handler.handleCallback(Handler.java:958)
        at android.os.Handler.dispatchMessage(Handler.java:99)
        at android.os.Looper.loopOnce(Looper.java:230)
        at android.os.Looper.loop(Looper.java:319)
        at android.app.ActivityThread.main(ActivityThread.java:8893)
        at java.lang.reflect.Method.invoke(Method.java:-2)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:608)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1103)
iujames commented 9 months ago

@ashfurrow intersting, the No view found for id is the exact error that we believe should have been fixed with this change in 3.1.8. If you are still seeing this in a build running 3.1.11, then there must be something else going on. cc @andretortolano

ashfurrow commented 9 months ago

Interesting! We've seen this (on 3.1.11) a total of 16 times over the last five days. It looks like it happens right after navigating back to the main screen of the app. Android 12 and 14, but I don't see any pattern with device manufacturer or model.

ashfurrow commented 9 months ago

Hey there šŸ‘‹ Any update on this? We're seeing this crash about 10 times per day currently.

iujames commented 9 months ago

Hi @ashfurrow , apologies, we are continuing to work on this one but we have yet to find a solution to pass along. From the stack, it looks like something in how we are managing the fragment for an embed frame is conflicting in behavior with how the react-native-screens backstack management is working, but this is a bit of a tricky one. We will keep investigating. If you have any further ideas or details please let us know! I'm re-opening this issue to keep it clearly active.

iujames commented 9 months ago

@ashfurrow I'm curious if the issue we are seeing here is related at all to this one https://github.com/react-navigation/react-navigation/issues/11384 . Our particular case here doesn't look like it is the exact same as the Bottom Tabs issue noted there, but it ends up with the same underlying exception due to the Android fragment state management.

Do you have a way to consistently repro this one or a minimal reproducible sample? I'd be interested if the workaround mentioned in the other issue using detachInactiveScreens={false} on the stack navigator had any impact (or is even possible in your case).

ashfurrow commented 8 months ago

Hi and sorry for the delayed response ā€“ something came up last week.

I took a look at that issue and we haven't seen that issue yet. I haven't been able to find a way to reproduce this, unfortunately. Based on breadcrumbs, this problem seems to happen either when the user taps back to go to the root screen in a stack navigator, or directly after MainActivity#onPause().

Our app navigation structure is a little unusual because we need to hide the bottom tabs when navigating to certain screens. Here's a simplified view:

const App = createNativeStackNavigator()

const TabNavigator = () => {

    <Tab.Navigator /* ... */>
      {/* Five of these: */}
      <Tab.Screen name={/* */} component={/* */} />
  </Tab.Navigator>
}

// ...

<App.Group>
  <App.Screen
    name={IDENTIFIERS.stacks.home}
    component={TabNavigator}
  />
  {/* Several other native stack navigators (required due to design constraints) */}
</App.Group>

Happy to provide any other details on this. In the meantime, we've disabled initializing the Appcues SDK for our Android users, which seems to have resolved the crash.