Skip to content
This repository has been archived by the owner on May 13, 2023. It is now read-only.

Callback of onAuthStateChange() is not called when recoverSession() is used #17

Closed
kaboc opened this issue Jun 4, 2021 · 8 comments
Closed
Labels
bug Something isn't working

Comments

@kaboc
Copy link

kaboc commented Jun 4, 2021

Bug report

Describe the bug

Calling recoverSession() results in changing the auth state if the recovery is successful, so I thought the callback function of onAuthStateChange() would be called, but it actually wasn't.

To Reproduce

final client = SupabaseClient(...);
client.auth.onAuthStateChange((event, session) => print(event));  // Nothing is printed.

...

final session = await fetchSession();
final response = await client.auth.recoverSession(session);
if (response.error != null) {
  // Error handling
}

...

Future<String> fetchSession() async {
  // Fetching the json string from the local DB
  final session = await ...;
  return session;
}

Expected behavior

"AuthChangeEvent.signedIn" is printed in the console when the user's sign-in state is recovered by recoverSession().

I’m not really sure if this is the right behaviour as it is not documented in the API reference of the package. Therefore it is just what I personally expected to happen.

System information

  • OS: Windows
  • Version of supabase-js: 0.4.2
  • Version of Node.js: 15.11.0
@kaboc kaboc added the bug Something isn't working label Jun 4, 2021
@phamhieu
Copy link
Member

@kaboc, there is an issue with session recovery timer. I fixed it on the latest version supabase: ^0.0.1-dev.20

let me know if you still facing the issue.

@kaboc
Copy link
Author

kaboc commented Jun 16, 2021

@phamhieu
I updated the package to 0.0.0-dev.20, ran the app, and confirmed that the callback of onAuthStateChange was called.

However, it wasn't called when recoverSession() was executed after I hot-restarted the app. Since then it has never been called, regardless of whether a hot restart or a full restart.

Is the session recovery timer you mentioned related to expiration of a token/session? I think my last sign-in to the app was yesterday or the day before, so it is likely that the session had already expired at the point of my first attempt today. I hope this gives you a clue.

@phamhieu
Copy link
Member

phamhieu commented Jun 16, 2021

recoverSession will only trigger an event if the current session is invalid and it refresh to a new one

In your case, can check your access token expiry to confirm? Can you also test on a fresh install to ensure that the persistent session value is right.

The timer bug is because of invalid expiryAt persistent session value.

P.S if you want to force a refresh, then you should call auth.api.refreshAccessToken instead

@kaboc
Copy link
Author

kaboc commented Jun 16, 2021

@phamhieu
Thank you for the explanation. What I understood is, the callback is only called when the session is refreshed, and just recovering a session before expiry does not trigger it.

I think it is reasonable, but onAuthStateChenge() has no document and the method name does not mention the timing. As a user, I guess from the name that its callback function is called whenever the auth state changes, including the transition from the signed-out state right after an app launch to the recovered signed-in session after recoverSession().

What is more confusing is that it is triggered even on recovery if the access token has already expired.

  • on sign in: triggered
  • on recovery before expiry: not triggered
  • on recovery after expiry: triggered

I think this could be improved for better usability.

I actually expected that I could write only once inside the callback the code for navigating a user after sign-in and recovery, but I'll write it separately as a workaround if it's not possible.

@kaboc
Copy link
Author

kaboc commented Jun 16, 2021

@phamhieu

P.S if you want to force a refresh, then you should call auth.api.refreshAccessToken instead

It is also what I'm not sure about. It looks like the token expires if I don't use an app for a while after launching it. I notice the expiry only when a query to the database fails.

  • Why does it expire although autoRefreshToken is true by default?
  • Should expiry be checked before every fetch?
  • Or do I have to see the failure first and then try refreshAccessToken()?
  • Is there a method for the checking?

I would be grateful if you'd direct me to a related document/thread/issue.

@phamhieu
Copy link
Member

phamhieu commented Jun 16, 2021

but onAuthStateChenge() has no document and the method name does not mention the timing

Totally agree with you. We need to improve the document.

I actually expected that I could write only once inside the callback the code for navigating a user after sign-in and recovery, but I'll write it separately as a workaround if it's not possible.

i just update flutter demo app for Supabase. You can take a look as reference https://github.com/phamhieu/supabase-dart-demo.

  • It uses SplashScreen to check persistent session and recover on app open.
  • It uses a singleton to manage supabase-client and persist session .
  • We should have only 1 instance of supabase-client per app.

It looks like the token expires if I don't use an app for a while after launching it.

The default value for token expiry is 3600 seconds (1 hour). If you don't use the app for 1 hour, it will expire. You can update this value if you want (under Authentication settings).

There are many ways to solve this. Exp: you can listen for resume event with WidgetsBindingObserver and trigger recoverSession
Screenshot 2021-06-16 at 2 47 52 PM

Why does it expire although autoRefreshToken is true by default?

there's a bug with gotrue-dart library which causes wrong expiry calculation check.

Should expiry be checked before every fetch?

no, you don't need. gotrue-dart has a timer to auto refresh the token. here

I hope this helps, let me know if you need any other clarification.

@kaboc
Copy link
Author

kaboc commented Jun 16, 2021

Thanks for all the detailed info, @phamhieu! It'll be helpful for other people too.

You can take a look as reference https://github.com/phamhieu/supabase-dart-demo.

I didn't know of the demo app. I'll look into it.

The default value for token expiry is 3600 seconds (1 hour). If you don't use the app for 1 hour, it will expire.
there's a bug with gotrue-dart library which causes wrong expiry calculation check.

It must be the bug you fixed yesterday, right? I left my app (with supabase 0.0.1-dev.20) untouched for longer than 1 hour, and there was no token expiry error this time. It is great that we don't have to care about it!

you can listen for resume event with WidgetsBindingObserver and trigger recoverSession

If the expiry-related issue was fixed, I wonder if you still need to listen for the resume event. The token does not expire while you keep using the app, so recovery is only needed when the app is launched, but AFAIK the state does not change to resume immediately after an app launch. I'll try it anyway and other different ways as well, and choose the best one for me.

Now that you clarified that the behaviour I thought was a bug was actually an expected behaviour, you can close this issue once documentation is improved. Thank you!

@phamhieu
Copy link
Member

phamhieu commented Jun 16, 2021

If the expiry-related issue was fixed, I wonder if you still need to listen for the resume event. The token does not expire while you keep using the app, so recovery is only needed when the app is launched

the token will not expire if app runs on foreground. Listening for the resume event will help recover session when app resumes from background state. I have an example here

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants