appcues / appcues-android-sdk

The Appcues Android SDK
https://www.appcues.com/mobile
MIT License
10 stars 3 forks source link

dispatchTouchEvent not on main thread #563

Closed nelletto closed 9 months ago

nelletto commented 9 months ago

Hi, we recently enabled a flow in production and we noticed a large number of crashes caused by Appcues trying to show the flow again when idle, more precisely by MotionEvent.obtain(0L, 0L, MotionEvent.ACTION_CANCEL, 0f, 0f , 0) at ModalStateMachineOwner.kt:102 executed on a non-ui thread.

The most frequent case is when using Maps

com.google.maps.api.android.lib6.common.apiexception.c: Not on the main thread

but there are also other cases, such as

Fatal Exception: android.util.AndroidRuntimeException: Animators may only be run on Looper threads
        at android.animation.ValueAnimator.cancel(ValueAnimator.java:1104)
        at android.widget.Editor$MagnifierMotionAnimator.dismiss(Editor.java:5272)
        at android.widget.Editor$MagnifierMotionAnimator.access$7300(Editor.java:5195)
        at android.widget.Editor.dismissMagnifierForDrag(Editor.java:9070)
        at android.widget.Editor.access$10700(Editor.java:183)
        at android.widget.Editor$SelectionModifierCursorController.onTouchEvent(Editor.java:7790)
        at android.widget.Editor.onTouchEvent(Editor.java:1690)
        at android.widget.TextView.onTouchEvent(TextView.java:11987)
        at android.view.View.dispatchTouchEvent(View.java:14368)
        at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:3833)
        at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:3551)
        at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:3833)
        at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:3551)
        at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:3833)
        at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:3551)
        at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:3833)
        at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:3551)
        at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:3833)
        at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:3551)
        at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:3833)
        at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:3551)
        at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:3833)
        at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:3551)
        at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:3833)
        at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:3551)
        at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:3833)
        at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:3551)
        at com.android.internal.policy.DecorView.superDispatchTouchEvent(DecorView.java:770)
        at com.android.internal.policy.PhoneWindow.superDispatchTouchEvent(PhoneWindow.java:1952)
        at android.app.Activity.dispatchTouchEvent(Activity.java:4038)
        at androidx.appcompat.view.WindowCallbackWrapper.dispatchTouchEvent(WindowCallbackWrapper.java:70)
        at io.sentry.android.core.internal.gestures.WindowCallbackAdapter.dispatchTouchEvent(WindowCallbackAdapter.java:39)
        at io.sentry.android.core.internal.gestures.SentryWindowCallback.dispatchTouchEvent(SentryWindowCallback.java:64)
        at com.android.internal.policy.DecorView.dispatchTouchEvent(DecorView.java:728)
        at com.appcues.ui.ModalStateMachineOwner$attemptRetry$1.invoke(ModalStateMachineOwner.kt:101)
        at com.appcues.ui.ModalStateMachineOwner$attemptRetry$1.invoke(ModalStateMachineOwner.kt:96)
        at com.appcues.ui.ModalStateMachineOwner$onUiIdle$$inlined$schedule$1.run(ModalStateMachineOwner.kt:151)
        at java.util.TimerThread.mainLoop(Timer.java:562)
        at java.util.TimerThread.run(Timer.java:512)

Unfortunately we had to turn off the flows given the high number of events

andretortolano commented 9 months ago

Hey @nelletto thanks for reaching out with this issue, we are still trying to reproduce it to make sure we can fix it, were you able to reproduce it locally or is that just from your crash reports? and the other thing I'm curious is if that is happening on any particular android API / device model

nelletto commented 9 months ago

yes, I was able to reproduce the crash related to Maps locally using the flow preview QR. We have two activities, a main activity and the second containing the map (and the Appcues flow), when the second opens we make a network call and upon response we carry out an animated zoom to a specific point on the map. To reproduce the crash I opened the application using the flow preview QR that takes the application to the main activity, I waited for the Appcues error toast notification and opened the second activity. Here in most of my attempts I get the crash, regardless of the device used. I think it's probably related to the map animation.

Sorry but at the moment I am not able to provide an example project to reproduce the crash

andretortolano commented 9 months ago

hey @nelletto I'm still not able to reproduce but I think we can try to provide you with a fix to test and see if that helps.

since you can reproduce it easily can you try running it on the 3.1.12-rc01?

nelletto commented 9 months ago

hey @andretortolano thanks for the quick fix, I made a few attempts and there seems to be no more crashes. I just have a little doubt, I saw the pull request, I was wondering if it was safe to execute any block of code passed to onUiIdle on the ui thread, are we sure there is no risk of performing heavy or network operations? However, I confirm that the fix works and for me you can close the issue :)

andretortolano commented 9 months ago

this is supposed to run on UI thread, for some reason your scenario is not but whats happening in there is just the process of "finding the view" that was targeted for the tooltip. no extra network requests or anything

ninniuz commented 9 months ago

@andretortolano do you have any estimate on the availability of a new release that includes this fix? At the moment this prevents us from sending flows to our users.

andretortolano commented 9 months ago

hey @ninniuz, the 3.1.12-rc01 contains only this fix so its ok to go with it for now

ninniuz commented 9 months ago

@andretortolano thanks for the update. Any ETA for a non RC version of the SDK?

andretortolano commented 9 months ago

we talked internally and decided to provide a 3.1.12 version containing this bug fix, should be available later today but I will let you know

andretortolano commented 9 months ago

@ninniuz version 3.1.12 is available