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

Impossible to check null session without getSession warning #907

Open
2 tasks done
jdgamble555 opened this issue May 13, 2024 · 10 comments
Open
2 tasks done

Impossible to check null session without getSession warning #907

jdgamble555 opened this issue May 13, 2024 · 10 comments
Labels
bug Something isn't working

Comments

@jdgamble555
Copy link

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

This is an auth.js issue related to #873, so I'm not going to link the other repos.

It is IMPOSSIBLE to check for a session without contacting the Supabase server or displaying the annoying warning.

To be more specific, if my session is NULL, there is NO reason for me to need to contact the Supabase server. This means, for my pages that don't require a login, it will ALWAYS contact the Supabase server needlessly.

The problem lies in the suppressGetSessionWarning can only be called when the _saveSession gets called. What if there is no session? Then we have to use getUser() for no reason, or deal with a needless warning OVER and OVER.

To Reproduce

Check a user's session with getSession() while not logged in, you get the warning.

Expected behavior

It is expected that we can use getSession() without a warning. The warning only shows up when there is a session and we can use getSession() without calling getUser().

Other Problems

This has been mentioned in all three github issues, but still not solved.

Please do not close this issue until both the NextJS and SvelteKit (preferably all docs) have been updated to NOT only use getUser() on the server for NULL.

Adding an extra needless fetch is a big deal.

J

@jdgamble555 jdgamble555 added the bug Something isn't working label May 13, 2024
@j4w8n
Copy link
Contributor

j4w8n commented May 13, 2024

The SvelteKit steps are updated - they call getSession() first, and if null then they return such and do not invoke getUser(). However, Next docs do not appear to show this.

Check a user's session with getSession() while not logged in, you get the warning.

Which framework are you having this issue with?

@jdgamble555
Copy link
Author

I'm using SvelteKit, but it doesn't matter which Framework you use, as you cannot call getSession first due to the _saveSession problem mentioned above.

If there is no session, you will get the warning always.

J

@j4w8n
Copy link
Contributor

j4w8n commented May 13, 2024

I'm using SvelteKit, but it doesn't matter which Framework you use, as you cannot call getSession first due to the _saveSession problem mentioned above.

If there is no session, you will get the warning always.

J

Ok, thanks. I assumed that's what you used, based on past interactions.

I ask because I'm using SvelteKit, but I don't have that specific issue when there's no one logged in. Do you have a repro?

@jdgamble555
Copy link
Author

Did you look at the core code and do you see the problem? Are you using getSession before getUser?

I can make a repo if necessary, but all you have to do is follow the SvelteKit guide.

The NextJS guide doesn't use getSession, which means you have to call the extra fetch with getUser.

@j4w8n
Copy link
Contributor

j4w8n commented May 13, 2024

Hmm... I have a test repo that follows the SvelteKit guide, here, but I never see logs unless there is a logged-in user.

@jdgamble555
Copy link
Author

I will check my repo when I get home from work.

However, if there is a logged-in user, why are you still seeing the logs unless it is fixed? If you're seeing it once, it will run multiple times due to how hooks.server.ts works in Svelte.

J

@j4w8n
Copy link
Contributor

j4w8n commented May 13, 2024

Yep, it logs multiple times for me. But only when there's a user logged in.

If I read your description correctly, you're also having issues when a user isn't logged in, correct?

P.S. When auth-js v2.64.3 drops, it will limit the logs; which isn't a great consolation, but makes it a bit less annoying.

@jdgamble555
Copy link
Author

Yep, it logs multiple times for me. But only when there's a user logged in.

I just checked my app on my home project, and it seems I jumped the gun. The warning is only shown when logged in. After re-looking at this code, I believe I see how this is possible on this line.

That means SvelteKit has the correct idea to use getSession() first, while NextJS is fetching needlessly. The NextJS Tutorial should be updated to call getSession() first, and only call getUser() when there is a session to verify. Again, extra fetches can be costly, even for Supabase.

P.S. When auth-js v2.64.3 drops, it will limit the logs; which isn't a great consolation, but makes it a bit less annoying.

  1. When will auth-js v2.64.3 drop?
  2. What do you mean limit the log? Shouldn't it ONLY be displayed when a user is calling it incorrectly (not verifying a session)?

J

@j4w8n
Copy link
Contributor

j4w8n commented May 14, 2024

I'm not sure about the version timeline. Likely this week.

The limit should keep things calmer. But we're never gonna get rid of it with the current state of SvelteKit and the Supabase ssr layout.ts cookie storage for SvelteKit.

The warning needs to die, but that likely won't happen until Supabase comes out with asymmetric jwts. I hope it happens at that point; or as silentworks suggested: at least don't log it in production.

@fnimick
Copy link

fnimick commented Jul 28, 2024

If you are calling getUser on requests and want to silence the warning as your session is guaranteed to be safe, see my approach in https://github.com/fnimick/sveltekit-supabase-auth-starter - I replace the user proxy object that triggers the warning with the user instance returned from getUser.

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

3 participants