okta / okta-oidc-android

OIDC SDK for Android
https://github.com/okta/okta-oidc-android
Other
59 stars 45 forks source link

com.okta.oidc.OktaResultFragment.addLogoutFragment (OktaResultFragment.java:64) #263

Closed gopalp1709 closed 2 years ago

gopalp1709 commented 3 years ago

Hi ,

Crash analytics capturing below exception for production user .

androidx.fragment.app.FragmentManager.checkStateLoss (FragmentManager.java:1844) androidx.fragment.app.BackStackRecord.commit (BackStackRecord.java:294) com.okta.oidc.OktaResultFragment.addLogoutFragment (OktaResultFragment.java:64) com.okta.oidc.clients.web.SyncWebAuthClientImpl.lambda$startSignOut$4$SyncWebAuthClientImpl (SyncWebAuthClientImpl.java:321) com.okta.oidc.clients.web.-$$Lambda$SyncWebAuthClientImpl$9xGAYUir3zQW6WeNxzc89L77keM.run (Unknown Source:6) android.os.Handler.handleCallback (Handler.java:888)

Please help us resolving this exception and let me know if more information is required.

NikitaAvraimov-okta commented 3 years ago

Thanks for reaching out, we`ll try to reproduce it and get back to you

gyasistory commented 3 years ago

HI, We are receiving a similar error. below is a copy of our Stacktrace.

 Fatal Exception: java.lang.IllegalStateException: FragmentManager has been destroyed
       at androidx.fragment.app.FragmentManager.enqueueAction(FragmentManager.java:1878)
       at androidx.fragment.app.BackStackRecord.commitInternal(BackStackRecord.java:329)
       at androidx.fragment.app.BackStackRecord.commit(BackStackRecord.java:294)
       at com.okta.oidc.OktaResultFragment.addLogoutFragment(OktaResultFragment.java:64)
       at com.okta.oidc.clients.web.SyncWebAuthClientImpl.lambda$startSignOut$4(SyncWebAuthClientImpl.java:321)
       at com.okta.oidc.clients.web.SyncWebAuthClientImpl.$r8$lambda$9xGAYUir3zQW6WeNxzc89L77keM(SyncWebAuthClientImpl.java)
       at com.okta.oidc.clients.web.SyncWebAuthClientImpl$$InternalSyntheticLambda$0$b3dd8d9a0e099b6668a99a499759c7f24c87fe9a5601ffd65c13e8eccfb072f5$0.run$bridge(SyncWebAuthClientImpl.java:22)
       at android.os.Handler.handleCallback(Handler.java:938)
       at android.os.Handler.dispatchMessage(Handler.java:99)
       at android.os.Looper.loop(Looper.java:236)
       at android.app.ActivityThread.main(ActivityThread.java:7876)
       at java.lang.reflect.Method.invoke(Method.java)
       at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:656)
       at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:967)
idomo1 commented 3 years ago

Experiencing this issue as well. To provide some more insight, this happens when there is an attempt to commit a fragment transaction when the activity has been stopped or destroyed. Pretty simple to reproduce. Relevant code from OktaResultFragment.java:

public static void addLogoutFragment(WebRequest request,
                                         CustomTabOptions customTabOptions,
                                         FragmentActivity activity,
                                         String[] browsers) {
        OktaResultFragment fragment = new OktaResultFragment();
        fragment.logoutIntent = createAuthIntent(activity, request.toUri(), customTabOptions,
                browsers);
        activity.getSupportFragmentManager()
                .beginTransaction()
                .add(fragment, AUTHENTICATION_REQUEST)
                .commit(); // Causes crash if activity is stopped or destroyed
    }

Some more info https://www.androiddesignpatterns.com/2013/08/fragment-transaction-commit-state-loss.html

JayNewstrom commented 3 years ago

Thanks for the added details. I've create an internal issue to track this issue: OKTA-427467

gyasistory commented 2 years ago

@JayNewstrom has there been any update on this issue my crash-free rate has gone from 99% to 85% with almost 100 crashes per day due to this issue.

JayNewstrom commented 2 years ago

The task is in our backlog. We will update the GitHub issue here if we have more questions.

redaAazNJ commented 2 years ago

Hi @JayNewstrom, Just to inform you that we are Experiencing this issue while logging out (OktaResultFragment.addLogoutFragment) and while logging in as well (OktaResultFragment.addLoginFragment):

Fatal Exception: java.lang.IllegalStateException: Can not perform this action after onSaveInstanceState
       at androidx.fragment.app.FragmentManager.checkStateLoss(FragmentManager.java:1844)
       at androidx.fragment.app.FragmentManager.enqueueAction(FragmentManager.java:1884)
       at androidx.fragment.app.BackStackRecord.commitInternal(BackStackRecord.java:329)
       at androidx.fragment.app.BackStackRecord.commit(BackStackRecord.java:294)
       at com.okta.oidc.OktaResultFragment.addLoginFragment(OktaResultFragment.java:51)
       at com.okta.oidc.clients.web.SyncWebAuthClientImpl.lambda$startSignIn$2(SyncWebAuthClientImpl.java:187)
       at com.okta.oidc.clients.web.SyncWebAuthClientImpl.$r8$lambda$20tTQ6-XKgd_y2eWjLhG60LXQNo(SyncWebAuthClientImpl.java)
       at com.okta.oidc.clients.web.SyncWebAuthClientImpl$$InternalSyntheticLambda$0$b3dd8d9a0e099b6668a99a499759c7f24c87fe9a5601ffd65c13e8eccfb072f5$0.run$bridge(SyncWebAuthClientImpl.java)
       at android.os.Handler.handleCallback(Handler.java:751)
       at android.os.Handler.dispatchMessage(Handler.java:95)
       at android.os.Looper.loop(Looper.java:154)
       at android.app.ActivityThread.main(ActivityThread.java:6780)
       at java.lang.reflect.Method.invoke(Method.java)
       at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:1500)
       at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1390)
ashish-d-hh commented 2 years ago

Hi @JayNewstrom any update on this.

We are also getting crash on login

2021-12-17 20:04:42.361 14707-14707/D/AndroidRuntime: Shutting down VM
2021-12-17 20:04:42.364 14707-14707/ E/AndroidRuntime: FATAL EXCEPTION: main
    Process: , PID: 14707
    java.lang.IllegalStateException: FragmentManager has been destroyed
        at androidx.fragment.app.FragmentManager.enqueueAction(FragmentManager.java:1878)
        at androidx.fragment.app.BackStackRecord.commitInternal(BackStackRecord.java:329)
        at androidx.fragment.app.BackStackRecord.commit(BackStackRecord.java:294)
        at com.okta.oidc.OktaResultFragment.addLoginFragment(OktaResultFragment.java:51)
        at com.okta.oidc.clients.web.SyncWebAuthClientImpl.lambda$startSignIn$2$com-okta-oidc-clients-web-SyncWebAuthClientImpl(SyncWebAuthClientImpl.java:187)
        at com.okta.oidc.clients.web.SyncWebAuthClientImpl$$ExternalSyntheticLambda4.run(Unknown Source:6)
        at android.os.Handler.handleCallback(Handler.java:938)
        at android.os.Handler.dispatchMessage(Handler.java:99)
        at android.os.Looper.loop(Looper.java:223)
        at android.app.ActivityThread.main(ActivityThread.java:7656)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:592)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:947)

and also on logout

2021-12-17 20:07:31.883 15144-15144/ D/AndroidRuntime: Shutting down VM
    --------- beginning of crash
2021-12-17 20:07:31.883 15144-15144/ E/AndroidRuntime: FATAL EXCEPTION: main
    Process: , PID: 15144
    java.lang.IllegalStateException: FragmentManager has been destroyed
        at androidx.fragment.app.FragmentManager.enqueueAction(FragmentManager.java:1878)
        at androidx.fragment.app.BackStackRecord.commitInternal(BackStackRecord.java:329)
        at androidx.fragment.app.BackStackRecord.commit(BackStackRecord.java:294)
        at com.okta.oidc.OktaResultFragment.addLogoutFragment(OktaResultFragment.java:64)
        at com.okta.oidc.clients.web.SyncWebAuthClientImpl.lambda$startSignOut$4$com-okta-oidc-clients-web-SyncWebAuthClientImpl(SyncWebAuthClientImpl.java:321)
        at com.okta.oidc.clients.web.SyncWebAuthClientImpl$$ExternalSyntheticLambda5.run(Unknown Source:6)
        at android.os.Handler.handleCallback(Handler.java:938)
        at android.os.Handler.dispatchMessage(Handler.java:99)
        at android.os.Looper.loop(Looper.java:223)
        at android.app.ActivityThread.main(ActivityThread.java:7656)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:592)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:947)
JayNewstrom commented 2 years ago

We've not had a chance to look into this yet. Could you validate you're not calling into signIn or any of the signOut methods while your activity is stopped? This issue appears to be an issue in your apps code. Can you reproduce this in our sample app?

ashish-d-hh commented 2 years ago

@JayNewstrom --- Our is in running state, not is stopped state. But we are using chrome for login/logout --- In our app ,sometimes it happens and sometimes it doesn't crash

davidvavra commented 2 years ago

We are also seeing this crash in production Crashlytics. This is how we do logout:

1) client.registerCallback(...) 2) client.signOutOfOkta(activity) 3) When the callback is triggered (same handling for all cases): 4) client.sessionClient?.clear() 5) activity.finish()

Do you see some problem in this flow?

JayNewstrom commented 2 years ago

That flow looks fine @davidvavra.

davidvavra commented 2 years ago

Our crash is always happening during logout. Here is the stacktrace, similar to the one in the beginning:

Fatal Exception: java.lang.IllegalStateException
FragmentManager has been destroyed
androidx.fragment.app.FragmentManager.enqueueAction (FragmentManager.java:1542)

androidx.fragment.app.BackStackRecord.commit (BackStackRecord.java:306)
com.okta.oidc.OktaResultFragment.addLogoutFragment (OktaResultFragment.java:64)
com.okta.oidc.clients.web.SyncWebAuthClientImpl.lambda$startSignOut$4 (SyncWebAuthClientImpl.java:321)
com.okta.oidc.clients.web.SyncWebAuthClientImpl.$r8$lambda$9xGAYUir3zQW6WeNxzc89L77keM (SyncWebAuthClientImpl.java)
com.okta.oidc.clients.web.SyncWebAuthClientImpl$$InternalSyntheticLambda$0$d787f1d4a5cbac304e1d1e8dca806be8b9084ceb4a03a44ad24a9ec426c49b62$0.run (SyncWebAuthClientImpl.java:6)
davidvavra commented 2 years ago

I added a check for destroyed FragmentManager in our app code. We released it to production yesterday, but it didn't help. I'm still seeing the same crashes. I'm afraid the check must be done in the library. What do you think @JayNewstrom ?

Here is simplified code how we interact with Okta SDK during logout:

fun signOut(activity: FragmentActivity) {
        if (!activity.supportFragmentManager.isDestroyed) {
            client.registerCallback(object : ResultCallback<AuthorizationStatus, AuthorizationException> {
                override fun onSuccess(result: AuthorizationStatus) {
                    client.sessionClient?.clear()
                    activity.finish()
                }

                override fun onCancel() {
                    client.sessionClient?.clear()
                    activity.finish()
                }

                override fun onError(msg: String?, exception: AuthorizationException?) {
                    client.sessionClient?.clear()
                    activity.finish()
                }
            }, activity)
            client.signOutOfOkta(activity)
        }
}
JayNewstrom commented 2 years ago

Hmm, this isn't what I expected. @davidvavra would you be willing to build and run your PR version in production to validate that'll fix it?

You can create the AAR via ./gradlew :library:assembleRelease And you can copy the AAR into your project from library/build/outputs/aar/oidc-androidx-release.aar

My fear is that the fix you proposed in your PR is going to equate to the same issue.

davidvavra commented 2 years ago

OK I can do that in two weeks (our release cycle)

davidvavra commented 2 years ago

We have released the fix to production and I can confirm that the crash is gone. Should I reopen the PR?

JayNewstrom commented 2 years ago

Hi @davidvavra thanks for testing this. Sure, could you reopen your PR?

JayNewstrom commented 2 years ago

Hi all, I've opened a PR that should fix this for good here: https://github.com/okta/okta-oidc-android/pull/316 Please note that this solution will result in a cancel (much better than a crash), but isn't ideal still. In our new kotlin SDK, this is handled much more gracefully, and will be launched once the activity is resumed, or another activity is started after a configuration change. https://github.com/okta/okta-mobile-kotlin

davidvavra commented 2 years ago

Thanks a lot! My fix was more like a workaround and although it fixed most of the crashes, I still see a different crash sometimes:

Fatal Exception: java.lang.IllegalStateException
Can not perform this action after onSaveInstanceState

So I hope this will fix it properly.