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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+8 −26
Open

fix: Improve authentication handling in fetchWithAuth. #1373

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 8 additions & 26 deletions src/lib/fetch.ts
View file
Original file line number Diff line number Diff line change
@@ -1,28 +1,3 @@
// @ts-ignore
import nodeFetch, { Headers as NodeFetchHeaders } from '@supabase/node-fetch'

type Fetch = typeof fetch

export const resolveFetch = (customFetch?: Fetch): Fetch => {
let _fetch: Fetch
if (customFetch) {
_fetch = customFetch
} else if (typeof fetch === 'undefined') {
_fetch = nodeFetch as unknown as Fetch
} else {
_fetch = fetch
}
return (...args: Parameters<Fetch>) => _fetch(...args)
}

export const resolveHeadersConstructor = () => {
if (typeof Headers === 'undefined') {
return NodeFetchHeaders
}

return Headers
}

export const fetchWithAuth = (
supabaseKey: string,
getAccessToken: () => Promise<string | null>,
@@ -32,7 +7,14 @@ export const fetchWithAuth = (
const HeadersConstructor = resolveHeadersConstructor()

return async (input, init) => {
const accessToken = (await getAccessToken()) ?? supabaseKey
let accessToken: string | null
try {
accessToken = (await getAccessToken()) ?? supabaseKey
Copy link

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!

} catch (error) {
console.error("Error retrieving access token:", error)
accessToken = supabaseKey
}

let headers = new HeadersConstructor(init?.headers)

if (!headers.has('apikey')) {