supabase-community / supabase-kt

A Kotlin Multiplatform Client for Supabase.
https://supabase.com/docs/reference/kotlin/introduction
MIT License
331 stars 34 forks source link

[Bug]: client.gotrue.currentSessionOrNull() always return null #268

Closed florian-mlr closed 9 months ago

florian-mlr commented 10 months ago

General Info

Version(s)

1.2.0

Target(s)

Android

What happened? (include your code)

In the flow of my app, I want to detect if the user is logged in or not right away after launching the application. Then I can redirect it to login or main screen.

I'm calling client.gotrue.currentSessionOrNull() at the start of the activity to check for the current session but it turns out that it always returns null.

If I want this function to return the current session to me I have to add a delay before calling it as if the client didn't have time to initialize.

For example :

suspend fun getCurrentUser(): NetworkUser? {
       delay(2000) // Let's say 2 seconds wait til the session manager loaded the session
       val user = client.gotrue.currentSessionOrNull()?.user
}

My client is injected in my dataSource class with dagger Hilt and the setup is :

install(GoTrue) {
            autoLoadFromStorage = true
            autoSaveToStorage = true
        }

While inspecting the code I notice that currentSessionOrNull() is checking for the status :

https://github.com/supabase-community/supabase-kt/blob/435f6fde95246a2f3db202a94c83e888f00158e3/GoTrue/src/commonMain/kotlin/io/github/jan/supabase/gotrue/GoTrue.kt#L338

In my case the status is still LoadingFromStorage when the call of currentSessionOrNull() occurs.

Currently to cover this case, I check if it Authenticated or Not Authenticated by using this :

val userStatus = client.gotrue.sessionStatus
                .first { sessionStatus ->
                    sessionStatus is SessionStatus.NotAuthenticated || sessionStatus is SessionStatus.Authenticated
                }

Steps To Reproduce (optional)

No response

Relevant log output (optional)

No response

jan-tennert commented 10 months ago

The session is stored in GoTrue#sessionStatus, so currentSessionOrNull is just a shortcut for (sessionStatus.value as? SessionStatus.Authenticated).session For the demos and my app I just show a loading screen while the session is loading, e.g. when using compose: https://github.com/supabase-community/supabase-kt/blob/435f6fde95246a2f3db202a94c83e888f00158e3/demos/chat-demo-mpp/common/src/commonMain/kotlin/io/github/jan/supabase/common/App.kt#L23-L44

jan-tennert commented 10 months ago

You could also use something like sessionStatus.collect and change the screen depending on the status when not using compose

florian-mlr commented 10 months ago

Yes, I'm using it with Compose, I think I was confused. I just started with Supabase and the sdk and referred to the documentation : https://supabase.com/docs/reference/kotlin/auth-getsession

The session is stored in GoTrue#sessionStatus, so currentSessionOrNull is just a shortcut for (sessionStatus.value as? SessionStatus.Authenticated).session For the demos and my app I just show a loading screen while the session is loading, e.g. when using compose:

https://github.com/supabase-community/supabase-kt/blob/435f6fde95246a2f3db202a94c83e888f00158e3/demos/chat-demo-mpp/common/src/commonMain/kotlin/io/github/jan/supabase/common/App.kt#L23-L44

As the sessionStatus is initialized with NotAuthenticated, it means the LoginScreen will be displayed maybe for a few milliseconds depending on session load. I wouldn' recommend it, what do you think?

You could also use something like sessionStatus.collect and change the screen depending on the status when not using compose

i'm using this , it's like collect but stop to collect when the status is NotAuthenticated or Authenticated.

client.gotrue.sessionStatus
                .first { sessionStatus ->
                    sessionStatus is SessionStatus.NotAuthenticated || sessionStatus is SessionStatus.Authenticated
                }

However I think it's not the good approach because the sessionStatus is initialized with the state NotAuthenticated , I think there should be an other state to describe the initial state

jan-tennert commented 10 months ago

You are right, having NotAuthenticated as default is not good. I'm publishing 1.3.0 later/tomorrow and going to set LoadingFromStorage as the default, so that this should be a problem anymore, you could try 1.3.0-rc-2 in ~30 minutes and let me know what you think!

florian-mlr commented 10 months ago

Thank you for your prompt response, Yes i consider set LoadingFromStorage as the default better however I think another status such as NotLoadedwould better describe the initial state knowing that the load from storage can be disabled.

jan-tennert commented 10 months ago

When not having auto load enabled, the status gets set to NotAuthenticated. If there was another status, clients would get stuck on that status when they never try to load the session, or maybe clients intentionally only store sessions in memory. Not sure if that would be good either 🤔

jan-tennert commented 10 months ago

@florian-mlr I'd release the version, but if you have any suggestions regarding this problem, feel free to let me know

jan-tennert commented 9 months ago

Closing due to inactivity, feel free to reopen!