Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

App crash on homeserver registration #5218

Closed
yatadev opened this issue Feb 12, 2022 · 6 comments · Fixed by #5258
Closed

App crash on homeserver registration #5218

yatadev opened this issue Feb 12, 2022 · 6 comments · Fixed by #5258
Assignees
Labels
A-Registration O-Uncommon Most users are unlikely to come across this or unexpected workflow S-Major Severely degrades major functionality or product features, with no satisfactory workaround T-Defect Something isn't working: bugs, crashes, hangs and other reported problems X-Needs-Info Z-Crash

Comments

@yatadev
Copy link

yatadev commented Feb 12, 2022

Steps to reproduce

  1. Where are you starting? What can you see?
  • Try to register on my homeserver with registration tokens enabled
  • A modal pops up, asking if i want to open the web client for registration
  • The app crashes if i click yes

Outcome

What did you expect?

Browser should open the homeserver url

What happened instead?

App crash

Your phone model

Samsung galaxy s8+

Operating system version

Android 9

Application version and app store

Element v1.3.18, App Store

Homeserver

Synapse v1.52.0

Will you send logs?

No

@yatadev yatadev added the T-Defect Something isn't working: bugs, crashes, hangs and other reported problems label Feb 12, 2022
@yatadev yatadev changed the title Browser does not open App crash on homeserver registation Feb 12, 2022
@yatadev yatadev changed the title App crash on homeserver registation App crash on homeserver registration Feb 12, 2022
@ouchadam ouchadam added A-Registration O-Uncommon Most users are unlikely to come across this or unexpected workflow S-Major Severely degrades major functionality or product features, with no satisfactory workaround X-Needs-Investigation Z-Crash labels Feb 14, 2022
@ouchadam
Copy link
Contributor

thanks for raising, would it be possible to provide the synapse configuration changes you've made? (so we can setup a local environment with the same configuration)

Also submitting a rageshake/logs would also help us narrow down the issue!

@bmarty
Copy link
Member

bmarty commented Feb 17, 2022

I can repro the crash by hacking the code (adding ! at this line) and then trying to create an account on matrix.org.

Caused by: java.lang.ClassCastException: im.vector.app.features.onboarding.OnboardingActivity cannot be cast to im.vector.app.features.signout.soft.SoftLogoutActivity
        at im.vector.app.features.signout.soft.SoftLogoutViewModel$Companion.initialState(SoftLogoutViewModel.kt:59)

Adding it to our milestone

@bmarty bmarty added this to the WTF ready list milestone Feb 17, 2022
@mnaturel mnaturel self-assigned this Feb 17, 2022
@mnaturel
Copy link
Contributor

Hi @ouchadam @bmarty , in order to find the best fix for that crash, I am wondering the purpose of theSoftLogoutViewModel. I see it is used in FtueAuthWebFragment and LoginWebFragment and LoginWebFragment2.

I am asking the question because the session is injected into ViewModel meaning user should be authenticated, right? But for what I understand, it is used for sessionRecovery purpose which may indicating user is no more authenticated. And in the onboarding case I guess user will never be authenticated.

@ouchadam
Copy link
Contributor

@mnaturel from my understanding the soft logout is triggered by receiving a M_UNKNOWN_TOKEN response or the session no longer having access to its token (rather than a 401)

the session instance is provided via

@Provides
fun providesCurrentSession(activeSessionHolder: ActiveSessionHolder): Session {
    return activeSessionHolder.getActiveSession()
}

which means the provided session is unrecognised rather than unauthenticated and can still access unauthenticated endpoints, datastores and eventually re-authenticate itself

@mnaturel
Copy link
Contributor

Okay thanks for explaining. The problem is that in ActiveSessionHolder the implementation of getActiveSession() is the following :

fun getActiveSession(): Session {
    return activeSession.get()
            ?: throw IllegalStateException("You should authenticate before using this")
}

It means in the case of onboarding, injecting Session into SoftLogoutViewModel leads to a crash (not the same crash of this issue) since there is no active session when first instantiating the FtueAuthWebFragment. I don't know if we could have the same problem with the LoginWebFragment.
The solution I have for FtueAuthWebFragment is to remove the usage of this SoftLogoutViewModel, but I am not quite sure I can.

@ouchadam
Copy link
Contributor

ah, the SoftLogoutViewModel will definitely cause a crash in that case! a quick fix could be to avoid injecting the session directly and instead rely on the ActiveSessionHolder.getSafeSession, however it would mean we have some overly decoupled state between the web fragment + softlogout viewmodel which could be a source of future bugs

bmarty added a commit that referenced this issue Feb 22, 2022
…istration

#5218: Fix crash at registration when redirecting to Web View
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Registration O-Uncommon Most users are unlikely to come across this or unexpected workflow S-Major Severely degrades major functionality or product features, with no satisfactory workaround T-Defect Something isn't working: bugs, crashes, hangs and other reported problems X-Needs-Info Z-Crash
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants