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

[SvelteKit] Session cannot be streamed using Promise #10530

Open
3 tasks
deronek opened this issue Apr 10, 2024 · 0 comments
Open
3 tasks

[SvelteKit] Session cannot be streamed using Promise #10530

deronek opened this issue Apr 10, 2024 · 0 comments
Labels
bug Something isn't working triage Unseen or unconfirmed by a maintainer yet. Provide extra information in the meantime.

Comments

@deronek
Copy link
Contributor

deronek commented Apr 10, 2024

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:

const authCookies = parse(response.headers.getSetCookie())
for (const cookie of authCookies) {
const { name, value, ...options } = cookie
// @ts-expect-error - Review: SvelteKit and set-cookie-parser are mismatching
event.cookies.set(name, value, { path: "/", ...options })
}

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:

  • Session promise should NEVER be streamed, and we should indicate that explicitly in the documentation,
  • Session promise should allow to be streamed, and it would be fairly easy to refactor the auth function to allow for this,
  • Session promise should theoretically allow to be streamed, but it would a significant refactor of library's core functionality and the handler hooks.
@deronek deronek added bug Something isn't working triage Unseen or unconfirmed by a maintainer yet. Provide extra information in the meantime. labels Apr 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working triage Unseen or unconfirmed by a maintainer yet. Provide extra information in the meantime.
Projects
None yet
Development

No branches or pull requests

1 participant