nextauthjs / next-auth

Authentication for the Web.
https://authjs.dev
ISC License
22.71k stars 3.03k forks source link

[SvelteKit] Session cannot be streamed using Promise #10530

Open deronek opened 1 month ago

deronek commented 1 month ago

Environment

  Binaries:
    Node: 20.9.0 - C:\Program Files\nodejs\node.EXE
    npm: 10.1.0 - C:\Program Files\nodejs\npm.CMD
    pnpm: 8.15.1 - ~\AppData\Local\pnpm\pnpm.EXE
  Browsers:
    Chrome: 123.0.6312.106
    Edge: Chromium (123.0.2420.81)
    Internet Explorer: 11.0.19041.3636
  npmPackages:
    @auth/sveltekit: latest => 0.13.0

Also reproducible on Vercel deployment

Reproduction URL

https://github.com/deronek/sveltekit-auth-example/

Describe the issue

@auth/sveltekit@0.8.0 broke an undocumented behavior which I was using in my application, which was lazily retrieving the Session object in a root layout, then awaiting it where necessary in the application. This way, I could start rendering full page, while displaying spinners/skeletons on data which need user data from the Session object.

9694 introduces setting cookies when retrieving the Session object. Migrating to version @auth/sveltekit@0.8.0 (replacing getSession with auth) will cause a possible race condition, where if the first response was already sent from the server before below code is executed:

https://github.com/nextauthjs/next-auth/blob/6116736ae2103ea7357b08e88c608af1f17fe70b/packages/frameworks-sveltekit/src/lib/actions.ts#L120-L125

we will receive an error:

Cannot use `cookies.set(...)` after the response has been generated

traced to here: https://github.com/sveltejs/kit/blob/f80ba75dfd967fb9d28d705d6891933bab603dc9/packages/kit/src/runtime/server/respond.js#L541-L543

How to reproduce

  1. Try to stream Session using streaming with promises: https://github.com/deronek/sveltekit-auth-example/blob/230836cb600db61db0ac81619a0caac83296b1a1/src/routes/%2Blayout.server.ts#L3-L7
  2. Await this Promise in your Svelte code. https://github.com/deronek/sveltekit-auth-example/blob/230836cb600db61db0ac81619a0caac83296b1a1/src/components/header.svelte#L9-L27
  3. In the deployed application, you should be able to observe Loading... before Sign in button appears.
  4. Login in the application.
  5. After successful login and redirect, observe client-side error Could not load user data and below error on server side.
    Error: Cannot use `cookies.set(...)` after the response has been generated
    at event2.cookies.set (file:///var/task/.svelte-kit/output/server/index.js:3259:15)
    at auth (file:///var/task/.svelte-kit/output/server/chunks/auth.js:6376:19)

Live deployment on Vercel: sveltekit-auth-example-eight-smoky.vercel.app

Expected behavior

Session should be allowed to be streamed using Promise.

While I find the feature of streaming the Session object to be potentially beneficial, I acknowledge that there may be valid reasons why it's not implemented as such. This opens the floor for discussion, and I'm welcome to insights on this matter. I see following outcomes of this discussion:

stalkerg commented 2 weeks ago

I have this issue:

Error: Cannot use cookies.set(...) after the response has been generated

even without streaming.

stalkerg commented 1 week ago

Yes, we have a race condition here, and it works even without streaming and simple +layout.server.js. Problem here event.local.auth() actual try to set cookie for wrong event, because this function bind to event inside hook. Context is here https://github.com/sveltejs/kit/issues/11813

Basically we should have a way to manually provide event object to the auth function.