getSession with persistSession true(default) fails silently with no localStorage

[GaryAustin1]

Bug report

Describe the bug

Currently getSession will clear out the memory session and return null if there is no localStorage and persistSession is left to default of true.

This so far has caused 1/2 dozen user issues on Discord where users are moving from v1 to v2 and losing lots of time debugging missing session data after sign in. Some of these users are moving from the old .user() method to check for a user to the getSession method to check for a user. Some are on serverside code, another was Electron. They don't need the persist, and setting the flag to false solves the issue, but that is a change that setting the flag is required.

To Reproduce

signInWithPassord in an environment without local storage and do getSession(). Result will be null for session AND the in memory session is cleared.

Relevant gotrue-js code here.
https://github.com/supabase/gotrue-js/blob/a441eef4d7291ec20b756cde99ccee87329594d0/src/GoTrueClient.ts#L512

Expected behavior

Either an error should be generated pointing to persistSession being on and no local storage, or go back to v1 like method where in memory still works, just nothing will be persisted.

Screenshots

If applicable, add screenshots to help explain your problem.

System information

supabase-js 2.x.x

Additional context

related: supabase/supabase-js#571

[j4w8n]

Hopefully this is getting more visibility soon.

There are similar issues when using getUser() server-side, without passing in a jwt.

As well as setSession(); but that one is really because of how _saveSession() works, when called.

[iolyd]

I think something's broken with the storage in general and getSession()'s returned session when used server-side is always null even when providing a server-compatible storage. I'm fairly certain doing something like this worked before one of the latest updates:

// Fill-in replacement for lack of local storage on server context.
function sessionStorageProvider(): SupportedStorage {
  const s = new Map();
  return {
    getItem: (key: string) => {
      console.log('Get item', s)
      return s.get(key);
    },
    setItem: (key: string, value: string) => {
      s.set(key, value);
      console.log('Set item', s)
    },
    removeItem: (key: string) => {
      s.delete(key);
    },
  };
}

function createServerClient() {
  return createClient<App.DatabaseSchema>(PUBLIC_SUPABASE_URL, PUBLIC_SUPABASE_ANON_KEY, {
    auth: {
      persistSession: true,
      autoRefreshToken: false,
      detectSessionInUrl: false,
      storage: sessionStorageProvider(),
    },
  });
}

But when using it recently, the storage is not passed around/kept properly:

event.locals.db = createServerClient();
const sessionRes = await event.locals.db.auth.setSession(event.locals.partialSession);
console.log('Set session', sessionRes.data.session);
// Set item, Map(1)
// Set session, {...session}
console.log('Get session', (await event.locals.db.auth.getSession()).data.session);
// Get item, Map(0)
// Get session, null

[iolyd]

After diving more into the latest commits and comparing with older versions to figure out why things break, I think the culprit for my case is here: 21e496c#r91728816

In fact, I'm surprised it isn't breaking the auth-helpers as it changes quite a bit how setSession behaves and makes it useless when trying to set auth on a not-logged-in server-side short-lived client instance.

When overriding my project's subdepenencies to pin "@supabase/gotrue-js" to "2.3.1", things go back to working as expected.

[j4w8n]

What auth helper are you using? Might need to start a new issue.

The changes in the PR did nothing to affect how a session is saved to storage - which takes place in _saveSession, and was not directly changed. However, the update would've been breaking for any code that doesn't pass in a properly formatted access_token.

However, supabase client code hasn't been updated to include gotrue-js v2.4.0 or higher. For instance, supabase-js is still using 2.3.1.

[j4w8n]

As a sidenote, unless you're just wanting to use a custom storage provider, you can set persistSession to false in your server client options and setSession and getSession will work fine.

[iolyd]

I'm not using any auth helper, although my setup is somewhat similar to the sveltekit helper.

Unless I'm mistaken, if you look at my comment in the commit's code, you'll see that the _saveSession call was removed in favor of letting _callRefreshToken handle it. But the latter is not reachable in setSession unless the passed token is expired. Passing fresh tokens to a client that doesn't have any previously set session and hasn't been signed-in just does nothing and returns the extracted session.

I'm using pnpm and recently (and frequently) ran an pnpm up --latest which pinned sub dependencies to latest version. After that supabse-js ended up using gotrue-js 2.4.1. This broke pretty much everything regarding my implementation of server-side auth where I retrieve a minimal session (access and refresh tokens) from a cookie and instanciate a supabase client that lives only for the time of a request, setting its auth with setSession(#value from cookie#)

[j4w8n]

Oh wow, you're right! I should not have removed this line, but moved it up into the else clause. I'll get a PR stat and hopefully it gets added overnight. Thank you!

await this._saveSession(session)

[iolyd]

Ahaha, I banged my head so hard today trying to figure out why nothing worked all of a sudden. Happy to help!

[GaryAustin1]

Just had another user lose 2 hours over this with node.js...

[j4w8n]

Well, that's fun. I'm starting to think it'll be next year before this is addressed. Launch week is coming up; and I'll assume staff, etc are off the last two weeks of the year?

[GaryAustin1]

I've not paid attention to other issues with this. I just wish createClient would error if you don't have persistSession false and no local storage with a message. Most of the cases of new users with server side code are wasting time on this are trying to figure out why getSession is returning nothing. They are not thinking about persistSession, may not even know about it, but don't care. An error would flag them immediately to know to turn it off and that would then force them to think about what that means.

[Torwent]

Took me 4h hours until I got help from @GaryAustin1 to figure out the reason my session would constantly return null was due to this!

[edgarsilva]

Took me 4h hours until I got help from @GaryAustin1 to figure out the reason my session would constantly return null was due to this!

Same, but sadly without @GaryAustin1 help 😄

[edgarsilva]

I think something's broken with the storage in general and getSession()'s returned session when used server-side is always null even when providing a server-compatible storage. I'm fairly certain doing something like this worked before one of the latest updates:

// Fill-in replacement for lack of local storage on server context.
function sessionStorageProvider(): SupportedStorage {
  const s = new Map();
  return {
    getItem: (key: string) => {
      console.log('Get item', s)
      return s.get(key);
    },
    setItem: (key: string, value: string) => {
      s.set(key, value);
      console.log('Set item', s)
    },
    removeItem: (key: string) => {
      s.delete(key);
    },
  };
}

function createServerClient() {
  return createClient<App.DatabaseSchema>(PUBLIC_SUPABASE_URL, PUBLIC_SUPABASE_ANON_KEY, {
    auth: {
      persistSession: true,
      autoRefreshToken: false,
      detectSessionInUrl: false,
      storage: sessionStorageProvider(),
    },
  });
}

But when using it recently, the storage is not passed around/kept properly:

event.locals.db = createServerClient();
const sessionRes = await event.locals.db.auth.setSession(event.locals.partialSession);
console.log('Set session', sessionRes.data.session);
// Set item, Map(1)
// Set session, {...session}
console.log('Get session', (await event.locals.db.auth.getSession()).data.session);
// Get item, Map(0)
// Get session, null

I just did the exact same think with a redis store, got exact same issues and was about to do a deep dive to find the root cause, @iolyd beat me to it.

@j4w8n your fix seems like it should be already in latest supabase release right? I'm still getting null when I call supabase.auth.getSession, maybe I'm missing something here 🤔 ....

[iolyd]

@edgarsilva Yeah, @j4w8n 's fix was merged and things are working on my side ever since (unless there's a freshly introduced bug that I haven't encountered yet). Sorry for not being of much help :/

[edgarsilva]

@iolyd don't worry about it, I'm redoing my setup to test it, I think it might be flawed and the server is the one cleaning it up, still adding this to the docs would really help and save a few hours to other people, mostly the createClient options for server side.

[edgarsilva]

On a side note I don't think passing a Bearer in the config like this =>

#474 (comment)

Works on server side, that's also another piece that I think is missing in the docs.

[j4w8n]

@edgarsilva regarding your ref to the 474 comment. I only skimmed that next-auth page, but I believe the client setup they show is only for making supabase requests. It isn't sufficient to make things like getSession() work server-side or client-side.

[j4w8n]

Also, if you've upgraded to the latest supabase-js and it's still jacked, I'd recommend opening a separate issue. We've probably hijacked Gary's post enough 🤣

[edgarsilva]

Also, if you've upgraded to the latest supabase-js and it's still jacked, I'd recommend opening a separate issue. We've probably hijacked Gary's post enough rofl

@iolyd I just confirmed @j4w8n fix works fine, it was indeed my setup (I was calling a signOut somewhere else 🤦🏽‍♂️)...

@j4w8n thanks for the quick follow up 👍🏽 ... all seems good now, Good to know regarding the headers stuff, I had dropped it already, but was planning on debugging later, I'll just leave it be, that's totally true about the hijacking 😅 ...

[hf]

Hey everyone, it appears this has been fixed since version ^2.4.2.

I'll close the issue now do reopen it if I'm mistaken.

[GaryAustin1]

Hey everyone, it appears this has been fixed since version ^2.4.2.

I'll close the issue now do reopen it if I'm mistaken.

@hf
I don't believe the original issue has been addressed (there was another issue added to the my original issue that may have been.)

Users are still encountering the original problem and wasting hours of time. At a minimum this should flag an error telling you turn persistSession false if on a server or other platform without localStorage.

I don't do serverside operations so can't easily reproduce the problem, but @j4w8n agrees it has not been fixed and spends time pointing this out to users in discord all the time also...

https://discord.com/channels/839993398554656828/1057489732952674324/1062272534768271520

[GaryAustin1]

Yet another user wasting time on this with web workers...
supabase/supabase#12355 (reply in thread)

[GaryAustin1]

Adding yet another user in limbo on this for a long time...
supabase/supabase#11559

[j4w8n]

I tried to get local dev working for the supabase repo, but couldn't. Maybe because I'm on Windows. I keep installing software (visual studio, python) and other errors just keep coming during npm install. So, I will not be doing a PR to try and add clarification to the docs.

@GaryAustin1 have you followed-up with the Supabase team on this? I hate to @ mention them myself.

[GaryAustin1]

@j4w8n I'm mainly using this thread as it has some team participation. I also did flag this as a high priority one to another team member in Discord. But I don't really have direct contact to the team that would be appropriate to "burn" on a single issue.

[hf]

Hey so we're trying to understand the issue better and came up with these ideas:

• Let's use in-memory storage via the storage interface instead of the existing inMemorySession property which probably has an edge case somewhere.
• We're not sure why you would set persistSession to true and not include a storage -- this can be handled better as an error.

Do you have a reproducible example we can take a look at?

[GaryAustin1]

@hf the second point is the biggest issue. User's don't set it to true. That is the default. They don't even really think about it as they get no error on a server (or other environments without local storage) during createClient that there is not local storage. To me the biggest issue is the amount of time wasted as they try and figure out why they don't have a session in calls after they do getSession (which appears to null out what would have been an in memory session if local storage does not exist).

Edit:
I'll add the use of persistSession is only documented in options behind tabs in the js initialize docs. Someone using Supabase for the first time, or first time on an environment without local storage is not thinking about persist or local storage or not. I've worked with dozens of users who have lost alot of time because there is no error and the symptom is their RLS does not work. It takes awhile to figure out the getSession (and possibly other calls) are silently nulling out in memory session and then without looking for help or thru issues or source code, will not even think about local storage and persistSession flag.

An error would have been ideal on createClient if no local storage and persistSession true. But that might now be a breaking change.

[j4w8n]

@hf

I don't know where the best place is in docs, but if we had a section that covered a few use-cases I think it would help. Gary mentioned the JS Initializing docs, which has a tab for React Native options. I also noticed that the getting started guides for certain things show the auth options you'd want to have. I hope these are helping.

But there are also other scenarios, like some of the ones Gary has linked to, where people are doing this in web workers and testing in node.js.

If we could boil things down to a few scenarios, like Client Auth options in the browser, in non-browser environments with storage, in non-browser environments without storage it MIGHT help, but who knows.

Or maybe you put a blurb with the docs for getSession(), setSession(), and getUser() (especially when not passing in a JWT), etc.

[GaryAustin1]

Yet another user, just starting out, spent 5 hours debugging before asking for help...
https://discord.com/channels/839993398554656828/1081961537822007470

[JoaquimLey]

I am the user who spent 5+ hours on this 😅.

Reading the discussion I would like to leave my $0.02 with two changes:

1. Make sure we surface in the docs (and properly comment on the SDK's source/types)
2. Make it required to explicitly set the persistSession property.

This way we ensure we cater to both use cases.
Making it explicit how to instantiate the supabase client in a server or browser environment would avoid further confusion.

This is taking into account that (IMHO), any serious project that leverages supabase will eventually need to run code in worker/server env.

---

PS: I personally dislike the ambiguity of where your code is running of certain FE/full stack frameworks, so my suggestion might be a bit too overkill, but at least it would avoid confusion.

[j4w8n]

Another person, spending hours on this. Ideally, people would hit up Discord before spending this much time troubleshooting, but still.

https://discord.com/channels/839993398554656828/1109843885934518332

[j4w8n]

I just found out that there's a server-side example in the docs now. Not sure how long it's been there.

https://supabase.com/docs/reference/javascript/auth-api

[GaryAustin1]

So far the example is not helping... but at least you can point to it...

[kangmingtay]

hmm i think a low hanging fruit would be to emit some warning logs if persistSession is set to true and there's no storage option being set / localstorage doesn't exist

in v3, we can go with @hf's suggestion to return an error if persistSession is set to true and no storage option exists

[GaryAustin1]

The fun does not end... Now Nuxt user(s) get the warning and they evidently can't turn it off as then whatever Nuxt does to store the session (not localStorage) does not work and they lose the session on refresh...

https://discord.com/channels/839993398554656828/1006358244786196510/threads/1113542747643715584

[j4w8n]

The fun does not end... Now Nuxt user(s) get the warning and they evidently can't turn it off as then whatever Nuxt does to store the session (not localStorage) does not work and they lose the session on refresh...

https://discord.com/channels/839993398554656828/1006358244786196510/threads/1113542747643715584

I commented on that Discord post. But here is my takeaway - not knowing anything about Nuxt, other than what I read on their repo.

So the browser client obv has access to local storage, but their server clients keep persistSession as the default of true, so it makes sense why this error is thrown. And it's likely being thrown only when the server clients are created, not the supabaseAuthClient for the browser.

So in this case, the warning is valid in my opinion. The library should be setting these values to false for their server clients:

persistSession
detectSessionInUrl
autoRefreshToken

[kangmingtay]

Hey @GaryAustin1 and @j4w8n, thanks for noticing this! I'm not too sure how the nuxt serverside client is using persistSession but i'm reaching out to the nuxt folks to see if we can make a forward fix on the nuxt supabase library. I still think it's better to output the warning for clarity for anyone else who's trying to use supabase-js on the server-side for auth.

[GaryAustin1]

@kangmingtay Yes, please. It is painful to see users who have spent hours on server side before they ask and we say persistSession:false....

[j4w8n]

Ran across someone who's using a supabase client in an environment that doesn't support local storage, plus they aren't using auth. Because of this, they get the console warning. They bring up a good point of having to set an auth option, to avoid the warning, when they aren't even using auth.

I'm not sure how you'd work around this with existing code while keeping the warning. Might need to find an alternate approach.

#699 (comment)

[kangmingtay]

@j4w8n yeah i think @hf mentioned that we can initialise the auth client lazily and not emit the warning until it actually gets used

[ethanfox]

I am having this same issue but I don't know how I can fix it. I get this error:

No storage option exists to persist the session, which may result in unexpected behavior when using auth.
        If you want to set persistSession to true, please provide a storage option or you may set persistSession to false to disable this warning.
AuthApiError: invalid JWT: unable to parse or verify signature, signature is invalid
    at /Users/user/Code/modern-saas/node_modules/.pnpm/@supabase+gotrue-js@2.39.2/node_modules/@supabase/gotrue-js/dist/main/lib/fetch.js:44:15
    at Generator.next (<anonymous>)
    at fulfilled (/Users/user/Code/modern-saas/node_modules/.pnpm/@supabase+gotrue-js@2.39.2/node_modules/@supabase/gotrue-js/dist/main/lib/fetch.js:5:58)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5) {
  __isAuthError: true,
  status: 401
}

Whenever I try to add auth{... I get an error like it's not supposed to be there.
This is how I set up my server client

export const handle: Handle = async ({ event, resolve }) => {
  event.locals.supabase = createSupabaseServerClient({
    supabaseUrl: ENV.PUBLIC_SUPABASE_URL,
    supabaseKey: ENV.PUBLIC_SUPABASE_ANON_KEY,
    
    event,
  });

[gergely-simonics]

Oh Man, this thread saved my week. Thank you @GaryAustin1

[hf]

This should be fixed with #774. Please continue to comment or re-open if it's still an issue.