Skip to content

fix: Improve authentication handling in fetchWithAuth. #1373

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

Closed
wants to merge 1 commit into from

Conversation

anuragchvn-blip
Copy link

🚀 What This PR Does

  • Improves fetchWithAuth by adding better error handling for getAccessToken().
  • Logs meaningful errors when retrieving an access token fails.
  • Ensures API calls in Supabase SDK always have proper authentication headers.

🔧 Why This Is Important

  • Prevents silent failures in authentication.
  • Improves debugging experience for developers using Supabase SDK.

✅ How to Test

  1. Run Supabase SDK in a Node.js and browser environment.
  2. Intentionally break getAccessToken() to verify error logging.

Would love feedback from maintainers! 🚀

const accessToken = (await getAccessToken()) ?? supabaseKey
let accessToken: string | null
try {
accessToken = (await getAccessToken()) ?? supabaseKey

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's great to see the error handling here! Just a thought: the catch block will only execute if getAccessToken() throws an error. If getAccessToken() returns null (and doesn't throw an error), the catch block won't be reached. We might want to add a check to ensure accessToken has a valid value after the try block, or clarify the expected behavior of getAccessToken() to ensure it always either returns a token or throws an error.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the feedback, @m1nsuppp! You're absolutely right—if getAccessToken() returns null without throwing an error, the catch block won't be triggered. I’ll update the implementation to explicitly check whether accessToken is valid after the try block. We could either throw an error manually if it's null or ensure getAccessToken() always throws when it fails.

Would love your thoughts on the best approach here!

@mandarini
Copy link
Contributor

Hi @anuragchvn-blip ! Thank you for taking the time to contribute to Supabase and for thinking about improving error handling! I appreciate your attention to detail.

However, I need to close this PR because the current implementation is actually working as intended. Let me explain why the existing code handles all the expected scenarios correctly.

The current line:

const accessToken = (await getAccessToken()) ?? supabaseKey

already provides robust handling for all scenarios:

  • When getAccessToken() returns a valid token: Uses the user's JWT token for authenticated requests
  • When getAccessToken() returns null: Falls back to supabaseKey (anon key) - this is expected behavior, not an error
  • When getAccessToken() throws an error: The error appropriately propagates up to the caller

Adding error logging when getAccessToken() returns null would actually create noise in the logs, because returning null is the normal, expected behavior when a user isn't authenticated yet. This supports Supabase's dual authentication model where:

  • Authenticated users can access private resources using their JWT token
  • Anonymous users can access public resources using the anon key

Example: In a blog app, when an anonymous visitor loads the homepage to see public posts, getAccessToken() correctly returns null, and we fall back to the anon key. Your proposed change would log this as an "error" when it's actually the intended flow.

As @m1nsuppp correctly pointed out, the catch block would only execute if getAccessToken() throws an exception, but that's already handled appropriately by letting the error bubble up to the caller.

The current implementation follows the principle of "simple and predictable" - it handles both success and expected failure cases without unnecessary complexity.

Thanks again for taking the time to contribute to Supabase, and please don't hesitate to submit future PRs! Your engagement with the codebase is really appreciated!

@mandarini mandarini closed this Jul 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants