fix: use skipAutoInitialize to prevent SSR token refresh race condition#131
fix: use skipAutoInitialize to prevent SSR token refresh race condition#131
Conversation
dc11250 to
9c556eb
Compare
|
@mandarini thanks for opening this, I'm running into this issue since I had to upgrade from Vite 5 => 7 in a SvelteKit project. Important Svelte and SvelteKit are moving really fast right now, since they have experimental support for async Svelte and SvelteKit remote functions. So SvelteKit users that want to keep dependencies up to day have to bump Vite to 7. Is there a workaround I could do to the recommended approach from the docs to sidestep the issue? The best I could come up with (with Claude's help) is: export const handle: Handle = async ({ event, resolve }) => {
// Creates a Supabase client specific to this server request.
// The Supabase client gets the Auth token from the request cookies.
event.locals.supabase = createServerClient(
PUBLIC_SUPABASE_URL,
PUBLIC_SUPABASE_ANON_KEY,
{
cookies: {
getAll: () => event.cookies.getAll(),
// SvelteKit requires cookie `path` to be explicity set
setAll: (cookiesToSet) => {
cookiesToSet.forEach(({ name, value, options }) => {
event.cookies.set(name, value, { ...options, path: '/' });
});
},
},
},
);
// Workaround for supabase/ssr#131: trigger token refresh early
// before resolve() to prevent race condition with Vite 7
await event.locals.supabase.auth.getSession();
// Unlike `supabase.auth.getSession()`, which returns the session _without_
// validating the JWT, this function also calls `getUser()` to validate the
// JWT before returning the session.
event.locals.safeGetSession = async () => {
const {
data: { session },
} = await event.locals.supabase.auth.getSession();
// no session
if (!session) {
return { session: null, user: null };
}
const {
data: { user },
error,
} = await event.locals.supabase.auth.getUser();
// no jwt token
if (error) {
return { session: null, user: null };
}
return { session, user };
};
return resolve(event, {
filterSerializedResponseHeaders(name) {
// forward supabase headers
return name === 'content-range' || name === 'x-supabase-api-version';
},
});
}; |
|
pls help |
|
Can someone add more context for this issue? I've got a branch running vite 7 and async svelte, but I've not run across this issue. |
Having this issue with cookies when using Social Login with callback endpoint like /auth/callback. Working if you're not using any callback endpoint and redirecting to /. |
are you using callback endpoint or root redirect? |
Using a callback endpoint. All auth things are server side |
9c556eb to
120b48c
Compare
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis pull request adds explicit SSR session initialization controls and related docs and tests. createServerClient now sets skipAutoInitialize, wraps auth.initialize to track a private _initialized flag, and exposes auth.isInitialized(). applyServerStorage catches setAll errors, logs a specific message when cookies are set after the response, and rethrows other errors. Documentation explains when to call initialize() (early in request handlers) and when to skip it (OAuth callbacks). Tests cover initialization behavior, concurrency safety, and the new cookie error handling. Sequence DiagramsequenceDiagram
participant Handler as Request Handler
participant Client as createServerClient
participant Auth as auth (initialize / isInitialized)
participant SessionAPI as getSession()
participant Cookies as applyServerStorage.setAll
participant Response as HTTP Response
Handler->>Client: createServerClient(skipAutoInitialize:true)
Handler->>Auth: auth.initialize()
Auth->>SessionAPI: getSession()
SessionAPI-->>Auth: session
Auth->>Auth: set _initialized = true
Auth-->>Handler: initialize resolved
Handler->>Cookies: setAll(tokens)
Cookies->>Response: cookies.set(...)
alt response already sent -> setAll throws "after the response"
Cookies-->>Handler: throw response-sent error
Note right of Handler: logs helpful message about token refresh timing
else other error
Cookies-->>Handler: rethrow original error
end
Handler->>Response: send response
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
120b48c to
182343e
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@README.md`:
- Around line 26-86: The TS example in the README is missing a comma after the
cookies object in the createServerClient call and the fenced error block lacks a
language tag; update the snippet near createServerClient so the options object
reads `cookies: { getAll, setAll },` and add a language identifier (e.g.,
```text) to the error fence `Error: Cannot use \`cookies.set(...)\` after the
response has been generated`; after making these edits run Prettier/formatting
on README.md to clear the CI warning.
In `@src/types.ts`:
- Around line 69-76: Prettier is complaining about quote style for the
SessionInitializationMode type; update the string literals in the exported type
SessionInitializationMode in src/types.ts to use the project's preferred quotes
(e.g. change 'auto' | 'manual' to "auto" | "manual") or simply run the project's
Prettier formatter to reformat the file so the union uses the consistent quote
style while keeping the boolean false unchanged.
There was a problem hiding this comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@README.md`:
- Around line 44-46: Update the Markdown fenced code block that shows the error
so it includes a language tag (e.g., change the fence to ```text) and remove the
leading space before the error line so the block is well-formed; after that run
Prettier/formatting on README to satisfy CI. Locate the error example block
containing "Error: Cannot use `cookies.set(...)` after the response has been
generated" and apply these changes.
5f32054 to
7b01148
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/createServerClient.spec.ts`:
- Around line 385-473: Tests are touching private internals
(supabase.auth.settings and assuming initialize() calls getSession) which makes
them brittle; update the tests that use createServerClient so they assert
observable behavior instead: for the "skipAutoInitialize" case, do not access
supabase.auth.settings — instead verify no network/global.fetch is invoked
during client creation and that auth.isInitialized() remains false until you
call (supabase.auth as any).initialize(); for the concurrency test, stop
hijacking getSession internals and either mock/createClient to capture the auth
options passed at creation time or spy/mock the global.fetch used by the auth
initializer to count actual initialization calls, then assert isInitialized()
toggles to true and that the fetch/initializer was invoked only once when
multiple initialize() calls run in parallel.
cdb84f7 to
95c6e05
Compare
95c6e05 to
6fbbbea
Compare
c380af0 to
a4f415b
Compare
Problem
In SSR contexts, the Supabase auth client constructor automatically initializes and may trigger token refresh asynchronously. If this completes after the HTTP response has been generated, cookie setting fails with:
Solution
Set
skipAutoInitialize: truewhen creating the server client. This prevents GoTrue's constructor from auto-initializing, which is the correct behavior for SSR — GoTrue's own docs note that all public methods have lazy initialization so the client remains fully functional without it.No user-facing
initialize()call is needed.getSession()handles session loading and token refresh lazily, as it always has.