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

cookies are not being set when returning custom Response objects in handle #7611

Closed
ivanhofer opened this issue Nov 11, 2022 · 2 comments
Closed

Comments

@ivanhofer
Copy link
Contributor

Describe the bug

When returning custom Response objecs inside the handle function of the hooks.server file, all interactions with the event.cookies parameter get ignored.

In order to properly set cookies, somewone would need to manually specify them in the headers object.

return new Response(payload, {
	headers: {
		'set-cookie', event.cookies.serialize('custom', 'value',)
	}
})

I would expect that cookies are beeing set even if someone returns a custom Response object.


When using events.cookies.set after resolve we will get an error message.

const response = await resolve(event);

// this will throw an error
event.cookies.set('test', 'cookie') 

return response

But I think it makes sense also to support that use case and allow cookies beeing set after the rendering process.

Reproduction

https://github.com/ivanhofer/sveltekit-request-cookie-bug

  1. start the dev server
  2. navigate to http:https://localhost:5173/custom
  3. see that the custom cookie does not get applied

Logs

No response

System Info

System:
    OS: Linux 5.15 Ubuntu 20.04.5 LTS (Focal Fossa)
    CPU: (8) x64 11th Gen Intel(R) Core(TM) i7-11370H @ 3.30GHz
    Memory: 2.08 GB / 7.65 GB
    Container: Yes
    Shell: 5.8 - /usr/bin/zsh
  Binaries:
    Node: 16.16.0 - ~/.volta/tools/image/node/16.16.0/bin/node
    Yarn: 1.22.17 - ~/.volta/tools/image/yarn/1.22.17/bin/yarn
    npm: 8.18.0 - ~/.volta/tools/image/npm/8.18.0/bin/npm
  Browsers:
    Chrome: 107.0.5304.68
  npmPackages:
    @sveltejs/adapter-auto: next => 1.0.0-next.87 
    @sveltejs/kit: next => 1.0.0-next.542 
    svelte: ^3.44.0 => 3.53.1 
    vite: ^3.1.0 => 3.2.3

Severity

serious, but I can work around it

Additional Information

No response

@dummdidumm
Copy link
Member

This is a deliberate decision. event.cookies is related to the context of resolve - else, we would have to apply cookies after the Response is returned, leaving a gap where when you access the Response object in handle the cookies aren't applied yet, leading to subtle gotchas. For the same reason, using event.cookies isn't allowed after generating the Response through resolve.

@dummdidumm dummdidumm closed this as not planned Won't fix, can't repro, duplicate, stale Nov 11, 2022
@ivanhofer
Copy link
Contributor Author

ivanhofer commented Nov 11, 2022

Is it wrong to reset cookies internally after resolve was executed and then apply cookies after the handle function if cookies still containes not consumed items?

So having an "apply-cookies" process for everything resolve related an another for custom Responses.

At least a warning would be really useful when a custom Response was returned, but cookies contains items that are not beeing applied.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants