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

[Bug?]: useSession().data value inconsistent #1192

Closed
2 tasks done
metruzanca opened this issue Dec 27, 2023 · 8 comments
Closed
2 tasks done

[Bug?]: useSession().data value inconsistent #1192

metruzanca opened this issue Dec 27, 2023 · 8 comments
Labels
bug Something isn't working vinxi related to vinxi

Comments

@metruzanca
Copy link

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the latest version

Current behavior 😯

Sometimes session.data.userId is undefined when expected to be defined.

Expected behavior 🤔

Expecting session.data to be consistent e.g. logged in user always has data.userId.

Steps to reproduce 🕹

I've made a repository based off the with-auth example/template that reproduces the bug. I've just added a "create project" function and "list project" function, the latter is what causes the issue.

And I've got a video in the readme with a demonstration.

https://github.com/metruzanca/solidstart-getSession-bug-repro

Context 🔦

Trying to build my next app with solidstart beta 2, but this issue is making it impossible to reliably get any data from the server.
Had good success with beta 1 and wanted to use the latest. I would like to keep this project using beta 2 and not have to fall back to v1.

Your environment 🌎

Tested on a fresh Ubuntu LTS 20 desktop with latest node v20.10 and pnpm v8.13, https://github.com/metruzanca/solidstart-getSession-bug-repro/blob/main/package.json
@metruzanca metruzanca added the bug Something isn't working label Dec 27, 2023
@brenelz
Copy link
Contributor

brenelz commented Dec 28, 2023

I played around with this a bit and could replicate. No idea what is happening tho.

I had an idea that this function should be async and have await but made no difference

https://github.com/solidjs/solid-start/blob/main/examples/with-auth/src/api/server.ts#L33

@metruzanca
Copy link
Author

Learned that whichever createAsync is executed second, is the one that has the unreliable session.data value.

@brenelz
Copy link
Contributor

brenelz commented Dec 28, 2023

I think from what I saw it was just the 2nd call to getSession regardless of createAsync but I could be wrong

@metruzanca
Copy link
Author

metruzanca commented Dec 29, 2023

Out of curiosity I dug into useSession and found that this might be an issue with h3's useSession since if we jerry-rig this out of its internals, we seem to reliably get sessionData.

import {
getCookie, unsealSession, H3Event, EventHandlerRequest
} from "@solidjs/start/server"

async function getSessionData<SessionDataT>() {
  const event = getRequestEvent()!
  const cookie = getCookie(event, "h3")!
  const { data } = await unsealSession(
    event as H3Event<EventHandlerRequest>,
    { password: SESSION_SECRET },
    cookie
  )
  return data! as SessionDataT
}

By no means have I done any rigorous testing, nor do I really know what I'm doing with h3.


Made a branch in my repo, workaround works in dev and build.
https://github.com/metruzanca/solidstart-getSession-bug-repro/tree/workaround

@nksaraf
Copy link
Member

nksaraf commented Dec 30, 2023

If this workaround is comprehensive, I see some other stuff has been left out, is there no reason to have that? We should keep whatever's stable in solid-start's flow. So if this works we can use this as the exported version of useSession()

@metruzanca
Copy link
Author

metruzanca commented Jan 2, 2024

@nksaraf I actually have made a more complete version, since getCookie does seem to not return expected value on fresh users.

export async function getSessionData(): Promise<Record<string, any>> {
  const event = getRequestEvent()!
  const cookie = getCookie(event, "h3")
  if (!cookie) return {}

  const { data } = await unsealSession(
    event as H3Event<EventHandlerRequest>,
    { password: SESSION_SECRET },
    cookie
  )
  if (data) return data
}

This seems to work pretty reliably in my app, though I'm still using useSession for updating the session, as that seems to work as expected. But like you said, we could pull in the other props like the update method to make this comprehensive.

I'll throw something together tomorrow.

@mikalaikabash
Copy link

This may be offtopic but I'm trying to save data into session:

export async function signin({accessToken, profile}: { accessToken: string, profile: string }) {
    try {
        const session = await getSession();
        await session.update((d: UserSession) => (d.accessToken = accessToken));
        await session.update((d: UserSession) => (d.profile = profile));
    } catch (err) {
        return err as Error;
    }
}

And I get response back from api with the error in Set-Cookie header:

Set failed, cookie was too large.

Actual length of access token is 244, profile is 334.
Do I have an option to compress data?

@nksaraf
Copy link
Member

nksaraf commented Jan 14, 2024

And I get response back from api with the error in Set-Cookie header:

Set failed, cookie was too large.

Actual length of access token is 244, profile is 334. Do I have an option to compress data?

moved this to new issue #1260.

The existing issue should be fixed when #1258 is merged and release.

@nksaraf nksaraf closed this as completed Jan 14, 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 vinxi related to vinxi
Projects
None yet
Development

No branches or pull requests

5 participants