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

Supabase operations in onAuthStateChange will cause the next call to supabase anywhere else in the code to not return. #762

Open
2 tasks done
izhan opened this issue Aug 1, 2023 · 23 comments
Labels
bug Something isn't working

Comments

@izhan
Copy link

izhan commented Aug 1, 2023

Bug report

  • I confirm this is a bug with Supabase, not with my own application.
  • I confirm I have searched the Docs, GitHub Discussions, and Discord.

Describe the bug

await supabase.auth.getSession() will hang indefinitely every hour or so, until the user reloads the page. We suspect it's related to the token needing to be refreshed.

To Reproduce

  1. Sign into an application with Supabase auth running
  2. Wait an hour (or however long it takes for your token to expire)
  3. Reload the page and call await supabase.auth.getSession()
  4. getSession will hang indefinitely

It's unclear right now whether (4) happens all the time or only some of the time. But we've been managing to reproduce pretty frequently, both locally and in prod. It's been affecting our customers as well.

Reloading the browser after (4) will immediately resolve the issue. Subsequent reloads will be fine, until an hour of inactivity in which this problem occurs again.

We've tried multiple versions of supabase-js and gotrue already, to no effect. This has been occurring for the past few weeks.

supabase/supabase#15930 seems related but that ticket seems to be concerned with immediately after Google OAuth. Our problem occurs well after initial login.

Expected behavior

We expect getSession to never hang indefinitely

Screenshots

If applicable, add screenshots to help explain your problem.

System information

  • OS: [e.g. macOS, Windows] macOS
  • Browser (if applies) [e.g. chrome, safari] Chrome
  • Version of supabase-js: [e.g. 6.0.2] Multiple (2.24, 2.26, 2.31)
  • Version of Node.js: [e.g. 10.10.0] v18.16.0

Additional context

We tried this on multiple versions of supabase-js (2.24, 2.26, 2.31) and gotrue (2.42, 2.43.1, 2.46.1)

@izhan izhan added the bug Something isn't working label Aug 1, 2023
@saltcod saltcod transferred this issue from supabase/supabase Aug 1, 2023
@j4w8n
Copy link
Contributor

j4w8n commented Aug 2, 2023

If you lower your jwt expiry time, does that also shorten the timeframe of how often this occurs?

@hf
Copy link
Contributor

hf commented Aug 3, 2023

@izhan What "environment" is your code running in -- what browser (did you try others?), what framework, what TypeScript targets...?

We generally expect to see issues reproduced with the latest version of gotrue-js before we can continue, as some of the versions you listed are known to have issues.

@izhan
Copy link
Author

izhan commented Aug 5, 2023

If you lower your jwt expiry time, does that also shorten the timeframe of how often this occurs?

Have not tried yet, can do that

@izhan What "environment" is your code running in -- what browser (did you try others?), what framework, what TypeScript targets...?

We generally expect to see issues reproduced with the latest version of gotrue-js before we can continue, as some of the versions you listed are known to have issues.

Repro'd on Chrome, Safari, Firefox. Framework is ReactJS. TS target is ES2020.

We've tested on [email protected] and [email protected] which I believe were latest. The other versions I listed were tested after following threads in #768 and supabase/supabase#15930 – all of which didn't work.

New data point: we've recently downgraded back to [email protected] - the oldest known version that worked for us - and getSession() no longer hangs. We're gonna try this version for a few more days and monitor our logs to see if it hangs for any of our customers.

@aviadmini
Copy link

@izhan did the version downgrade work?

I most likely am facing the same issue in one of the edge cases:
when app tries to load and if Supabase backend is unreachable (e.g. local dev setup is down) then calling await supabase.auth.getSession() hangs. It's pretty bad in my case because I'm using Svelte auth helpers and it blocks +layout.ts which blocks the rest of the app from loading.
Should I open a separate issue?

@kangmingtay kangmingtay transferred this issue from supabase/supabase-js Aug 21, 2023
@izhan
Copy link
Author

izhan commented Aug 23, 2023

@aviadmini the downgrade did work for me. Similar hang here too with the newer version, but I don't see any failed network requests, do you? It seemed to be a bug in the gotrue logic, ie when to actually kickoff the call to backend

@aviadmini
Copy link

When supabase is unreachable (e.g. local dev setup is down) there are console errors for the unsuccessful fetch and retries during a call to svelte auth helpers functions. Right after that the following call to await supabase.auth.getSession() never completes

@danrasmuson
Copy link

We experienced this issue...

CleanShot 2023-09-04 at 13 21 34@2x

Our call supabaseClientSide.auth.getSession().then(({ data: { session } }) => session); would just hang forever under certain circumstances when the client did not have the token.

The way our team was able to resolve this was by upgrading from

"@supabase/supabase-js": "^2.26.0"

to...

"@supabase/supabase-js": "^2.33.1"

@felix-andreas
Copy link

felix-andreas commented Sep 8, 2023

We are also experiencing as similar issue when updating from supabase-js 2.30 -> 2.31. the only change between these versions seems to be supabase/supabase-js@c07ce23 which updates "@supabase/gotrue-js from 2.43.1 to 2.46.1

More specifically it happens, when awaiting a supabase function within the onAuthStateChange. The callback hangs indefinitely, depending on how fast the Promises returns. For us it only happens if the supabase-js client points to a local SUPABASE_URL setup, but not with a remote SUPABASE_URL...

@kjhughes
Copy link

kjhughes commented Sep 27, 2023

I also experienced getSession()'s returned promise never resolving. In my case, with @GaryAustin1 's help on Discord, I found that in my initial, rough onAuthStateChange() handler, hitting the database for all auth events indiscriminately was causing the problem. Specifically, hitting the database when receiving TOKEN_REFRESHED callbacks broke something such that subsequent getSession() calls would all hang.

I will of course refine the problematic onAuthStateChange() handler, but it would be great getSession() could be fixed to return an error or throw an exception rather than return a promise that never resolves for this and any related problems.

supabase-js version 2.36.0
gotrue-js version 2.54.0

@kangmingtay
Copy link
Member

hey everyone, i'm trying to reproduce this so that i can figure out why it happens but i need some help here -

@andreasfelix you mentioned that:

when awaiting a supabase function within the onAuthStateChange. The callback hangs indefinitely, depending on how fast the Promises returns. For us it only happens if the supabase-js client points to a local SUPABASE_URL setup, but not with a remote SUPABASE_URL...

which function are you calling in onAuthStateChange that leads to this?

@kjhughes just wondering, why do you need to call getSession() inside of the callback for onAuthStateChange()? The onAuthStateChange() method will pass the event and the session as parameters to the callback function so there shouldn't be a need to call getSession() again

@kjhughes
Copy link

kjhughes commented Oct 19, 2023

@kangmingtay: Thanks for investigating.

You are right: Calling getSession() within the onAuthStateChange() callback was an unnecessary mistake.

However, removing that call did not itself resolve the issue where another getSession() call would never return. I also had to remove from the onAuthStateChange() callback the code whereby it accessed the database. Only then would the non-resolving getSession() promise resolve.

More details are available in the linked Discord chat. Note that I believe I also observed that the scenario I described above (where onAuthStateChange() callback accessing the database hosed subsequent calls to getSession()) also hosed some other different subsequent supabase calls -- signInWithPassword(), for example.

@felix-andreas
Copy link

felix-andreas commented Oct 21, 2023

which function are you calling in onAuthStateChange that leads to this?

if I recall correctly any async operation that involves the supabase client, e.g.

supabase.auth.onAuthStateChange(async (event, session) => {
  switch (event) {
    case "TOKEN_REFRESHED":
    case "SIGNED_IN":
        await supabase.rpc(...)
    ...
  }
})

Our current workaround is to set a reactive variable (svelte) and run the effect whenever it changes:

supabase.auth.onAuthStateChange((event, session) => {
  switch (event) {
    case "TOKEN_REFRESHED":
    case "SIGNED_IN":
      dirty.set(true)
    ...
  }
})

dirty.subscribe(async ($dirty) => {
  if ($dirty) {
    dirty.set(false)
    await supabase.rpc(...)
  }
})

We didn't bother to look into why this works. I assume this somehow changes the order the microtasks are executed in (I am not super familiar with JS ...).

@kangmingtay
Copy link
Member

kangmingtay commented Oct 25, 2023

TLDR: We are looking into a fix for this but for now the workaround is to not make async calls to postgREST / storage / edge functions using the supabase client lib inside the callback function registered in onAuthStateChange :/


Hey everyone, it seems like this issue was introduced when this PR was merged. For context, this PR was made to address issues related to random logouts where _useSession is being called concurrently which led to refresh tokens being unintentionally reused. The solution was to introduce a locking mechanism to serialise the calls made to _useSession.

When an async supabase call is made in the callback function in onAuthStateChange, it results in a deadlock. This is because the supabase-js client library creates a custom fetch method which makes a call to _useSession under the hood.

This is how the deadlock occurs. For example, if you set up a callback function in onAuthStateChange to retrieve the latest todos on a TOKEN_REFRESHED event:

supabase.auth.onAuthStateChange( async (event, session) => {
  if (event === 'TOKEN_REFRESHED') {
    const todos = await supabase.from('todos').select('*')
  }
})
image image

If you make any async calls to postgREST / storage / functions using the supabase client library, it uses the custom fetch method mentioned above which requires a lock to be available.
image

@izhan
Copy link
Author

izhan commented Oct 25, 2023

Thank you @kangmingtay for the update and @kjhughes for the helpful hints!

Confirmed on our end too that the bug we've been experiencing was caused by this. We shipped a workaround last week to avoid the async call on TOKEN_REFRESH and no longer see these issues in prod:
CleanShot 2023-10-25 at 14 52 18@2x

@ryonwhyte
Copy link

ryonwhyte commented Oct 31, 2023

Hi, I had the issue in React Native with @supabase/supabase-js": "^2.38.4"
Here is what the code that triggered the bug looked like

 supabase.auth.onAuthStateChange(async (event, session) => {
      console.log(`Supabase auth event is ${event}`);
      //dispatch(setUserSession(session));
      //dispatch(setUser(session?.user ?? null));

      /**After an authStateChange we fetch user details. Maybe this should be conditional */
      if (session?.user?.id) {
        await new Promise((resolve) => setTimeout(resolve, 5000)); // Introduce a 1-second delay

        const details = await getProfileDetails(session.user.id);
        console.log("User details:", details);

        if (details.status === true) {
          dispatch(setProfile(details.data));
        }

        //Here we get the users orders
        const allUserOrders = await getUserOrders(session.user.id);
        //console.log("All user orders is: ", allUserOrders.data);
        if (allUserOrders.status === true) {
          dispatch(setUserOrders(allUserOrders.data));
        }
      }
    });

After getting some advice from @GaryAustin1 on discord. I removed all the functions inside onAuthStateChange functions and put them in another function, and made a single call.

The code looked like this now and works fine. Note, all the logic was moved to external function "handleUserUpdates()"

supabase.auth.onAuthStateChange(async (event, session) => {
      console.log(`Supabase auth event is ${event}`);
      dispatch(setUserSession(session));
      dispatch(setUser(session?.user ?? null));

      // Call the function to handle user updates
      handleUserUpdates(dispatch, session);
    })

It seems after auth state change, any immediate supabase database calls hang after that. In my case, the auth.updateUser() function would not return a response.

Hopefully this helps someone.

@GaryAustin1 GaryAustin1 changed the title getSession() hangs indefinitely every hour or so Supabase operations in onAuthStateChange will cause the next call to supabase anywhere else in the code to not return. Nov 17, 2023
@GaryAustin1
Copy link

GaryAustin1 commented Nov 17, 2023

I just edited the title of this issue to reflect the root problem so users might spot it quicker.

This bug is causing users major loss of time as it is not documented you can't make calls in onAuthStateChange and the symptom they see is in a completely different part of code.

This should be documented in the docs and fixed or addressed as soon as possible. It is a multi-hour loss of time for users trying to figure out and/or reaching out for help.

@brycegoh
Copy link

Big ups to OP we have this problem too m8

@hf
Copy link
Contributor

hf commented Dec 19, 2023

This should be documented in the docs and fixed or addressed as soon as possible. It is a multi-hour loss of time for users trying to figure out and/or reaching out for help.

@GaryAustin1 Can you point to where you would want this to be present in docs. Trust me, I've tried to address the issue on a technical level -- unfortunately it's not possible because of JavaScript, browsers, NextJS and all the other runtimes.

Alternatively, we can maybe turn off the await handling or make it configurable. Please let me know what works best for you.

@GaryAustin1
Copy link

@hf It should be documented here https://supabase.com/docs/reference/javascript/auth-onauthstatechange at least to not make supabase calls in that function handler and what happens if you do.

I added a note in troubleshooting https://github.com/orgs/supabase/discussions/19058 awhile back that I can clean up as it says "until this is fixed".

Not sure the user suggestion above to move calls to a function is the recommended way to avoid this or not:

supabase.auth.onAuthStateChange(async (event, session) => {
      console.log(`Supabase auth event is ${event}`);
      dispatch(setUserSession(session));
      dispatch(setUser(session?.user ?? null));

      // Call the function to handle user updates
      handleUserUpdates(dispatch, session);
    })

@hf
Copy link
Contributor

hf commented Dec 20, 2023

Docs changes here: supabase/supabase#19902

@justinkchen
Copy link

I removed my onAuthStateChange code and still have getSession hang on supabase-js 2.39.2

@ScreamZ
Copy link

ScreamZ commented Apr 12, 2024

Docs changes here: supabase/supabase#19902

I can confirm this is working see my comment nuxt-modules/supabase#273 (comment)

@CarrettaRiccardo
Copy link

I found that you have to call every function for auth (signIn, signOut, ...) in client-side code (using a client supabase) in order for it to work properly.
Having things like server actions in the server using a server supabase instance will not trigger an onAuthStateChange client side, even tho in that case there are some weird behaviours, like when changing browser tab

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

No branches or pull requests