fix: cookies and immutable headers

[JLewinski]

Fixes: #13857

When making a request via the event.fetch to the same exact domain the svelte app is running on, it fails returning a 500 due to the header being immutable.

This only happens when a cookie is set in the hooks before trying to make a request with the event.fetch

This fix just adds a try catch around the code that adds the cookie to the header so that it still attempts to make the request even if it fails to add the cookie.

---

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

• It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
• This message body should clearly illustrate what problems it solves.
• Ideally, include a test that fails without this PR but passes with it.

Tests

• Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

• If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpm changeset and following the prompts. Changesets that add features should be minor and those that fix bugs should be patch. Please prefix changeset messages with feat:, fix:, or chore:.

Edits

• Please ensure that 'Allow edits from maintainers' is checked. PRs without this option may be closed.

[changeset-bot]

🦋 Changeset detected

Latest commit: da51d2b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@sveltejs/kit	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[dummdidumm]

Thank you for the PR but I don't think we should just ignore the error. It would result in worse behavior, like inconsistent states or you don't realize you need to check your logs and are scratching your head why the header is not there

[JLewinski]

Yeah, I can look more into it. I never figured out why the header is immutable and where it is initially being set. A better fix may be to check and see if the cookies are already in the header.

[JLewinski]

Instead of the try catch, I am now checking to see if the request is a sub request. If it is a sub request, I am skipping the attempt to add cookies as the header for sub requests is immutable.

Is it supposed to be that the header is immutable for sub requests? If so, why is that?

[svelte-docs-bot]

Preview: https://svelte-dev-git-preview-kit-13942-svelte.vercel.app/

[Conduitry]

I haven't looked closely enough to see why whether this request has immutable headers depends on whether it's a sub-request, but one thing we're doing elsewhere in the codebase already is to create a new Request object from the existing one - with a new (mutable) Headers object - and then add a new header to that cloned Headers object.

I think that would work here. There doesn't seem to be anything in the code being touched here that requires you use the same Request object.

[JLewinski]

I haven't looked closely enough to see why whether this request has immutable headers depends on whether it's a sub-request, but one thing we're doing elsewhere in the codebase already is to create a new Request object from the existing one - with a new (mutable) Headers object - and then add a new header to that cloned Headers object.

I think that would work here. There doesn't seem to be anything in the code being touched here that requires you use the same Request object.

I updated it to use a new object.

[benmccann]

It seems surprising to me that a new Response would have mutable headers whereas the original one has immutable headers. If we could avoid making the headers immutable in the first place that seems like a better solution to me than cloning the object. I'm pretty sure we're the ones who create the Response, so it feels like we should just fix it instead of adding overhead that will make every request slower to serve

[JLewinski]

It seems surprising to me that a new Response would have mutable headers whereas the original one has immutable headers. If we could avoid making the headers immutable in the first place that seems like a better solution to me than cloning the object. I'm pretty sure we're the ones who create the Response, so it feels like we should just fix it instead of adding overhead that will make every request slower to serve

I have now found why the header is immutable. This is happening when the resolve method makes an http request with fetch. The response object that is returned is directly from fetch which will have an immutable header. Would it be good to take that response and return a copy of it with an immutable header instead of the original?

[benmccann]

can you expand the comment to explain why they need to be mutated?

Suggested change

	// the header for the response needs to be mutable, so we need to clone it
	// the headers for the response needs to be mutable, so we need to clone it

[benmccann]

Thanks. We should probably have a test for this