Repository: sveltejs/kit (js)

Issue:
Svelte kit's provided fetch is stripping body when "mode: no-cors" is provided and the fetch happens from a +server.js module
### Describe the bug

https://github.com/sveltejs/kit/discussions/8097

I'm using svelte-kit's provided fetch to call a MS api that respond with an error when the Origin Header is provided.

I've noticed that the Origin Header is not provided when `mode: "no-cors"` is provided but this will then cause the body to be stripped away from the response.

I'm using the Node's global fetch method to workaround the issue at the moment.

### Reproduction

https://github.com/raythurnevoid/svelte-kit-no-cors-strip-body-bug-report

1.
```
npm i
npm run dev -- --open
```

2. An error like this will show up in console
```
Unexpected end of JSON input
SyntaxError: Unexpected end of JSON input
    at JSON.parse (<anonymous>)
    at packageData (C:\workspace\rt0\svelte-layout-server-error\node_modules\undici\lib\fetch\body.js:553:19)
    at specConsumeBody (C:\workspace\rt0\svelte-layout-server-error\node_modules\undici\lib\fetch\body.js:517:10)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async GET (C:/workspace/rt0/svelte-layout-server-error/src/routes/test.json/+server.ts:5:18)
    at async render_endpoint (file:///C:/workspace/rt0/svelte-layout-server-error/node_modules/@sveltejs/kit/src/runtime/server/endpoint.js:45:20)
    at async resolve (file:///C:/workspace/rt0/svelte-layout-server-error/node_modules/@sveltejs/kit/src/runtime/server/index.js:401:17)
    at async respond (file:///C:/workspace/rt0/svelte-layout-server-error/node_modules/@sveltejs/kit/src/runtime/server/index.js:264:20)
    at async fetch (file:///C:/workspace/rt0/svelte-layout-server-error/node_modules/@sveltejs/kit/src/runtime/server/fetch.js:142:16)
    at async Object.fetch (file:///C:/workspace/rt0/svelte-layout-server-error/node_modules/@sveltejs/kit/src/runtime/server/fetch.js:25:10)
```

3. In `src/routes/+server.ts`, replace `event.fetch` with `fetch` to make it work

### Logs

_No response_

### System Info

```Shell
System:
    OS: Windows 10 10.0.22621
    CPU: (16) x64 AMD Ryzen 9 4900H with Radeon Graphics
    Memory: 22.95 GB / 31.42 GB
  Binaries:
    Node: 18.10.0 - C:\Program Files\nodejs\node.EXE
    npm: 8.19.2 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Edge: Spartan (44.22621.963.0), Chromium (108.0.1462.46)
    Internet Explorer: 11.0.22621.1
  npmPackages:
    @sveltejs/adapter-node: 1.0.0-next.102 => 1.0.0-next.102 
    @sveltejs/kit: 1.0.0-next.571 => 1.0.0-next.571 
    svelte: ^3.53.1 => 3.53.1
```


### Severity

annoyance

### Additional Information

I guess that the correct behavior should be to never provide the Origin Header when the call is performed from a server-only module to match the behavior of the Node's fetch API.

I've taken a look at the undici fetch docs, and actually looks like they don't even read the mode argument.

Pull request:
[fix] don't strip response body for server-only no-cors 
Fixes #8153
A similar fix to #7454

Also refactored the code so we can unit test that part (which removes 3 e2e tests). I don't love the stubbing that's required but it's certainly better than the e2e setup dance IMO.

### Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
- [x] 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
- [x] This message body should clearly illustrate what problems it solves.
- [x] Ideally, include a test that fails without this PR but passes with it.

### Tests
- [x] Run the tests with `pnpm test` and lint the project with `pnpm lint` and `pnpm check`

### Changesets
- [x] 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. All changesets should be `patch` until SvelteKit 1.0


Patch:
diff --git a/.changeset/rotten-mails-study.md b/.changeset/rotten-mails-study.md
new file mode 100644
index 000000000..31add12e5
--- /dev/null
+++ b/.changeset/rotten-mails-study.md
@@ -0,0 +1,5 @@
+---
+'@sveltejs/kit': patch
+---
+
+Don't strip body in no-cors mode on the server
diff --git a/documentation/docs/30-advanced/25-errors.md b/documentation/docs/30-advanced/25-errors.md
index 40ac2269e..249aa8dd2 100644
--- a/documentation/docs/30-advanced/25-errors.md
+++ b/documentation/docs/30-advanced/25-errors.md
@@ -106,6 +106,8 @@ export function handleError({ error, event }) {
 }
 ```
 
+> Make sure that `handleError` _never_ throws an error
+
 ## Responses
 
 If an error occurs inside `handle` or inside a [`+server.js`](/docs/routing#server) request handler, SvelteKit will respond with either a fallback error page or a JSON representation of the error object, depending on the request's `Accept` headers.
diff --git a/documentation/docs/60-appendix/10-migrating.md b/documentation/docs/60-appendix/10-migrating.md
index 759bec196..2e04f115c 100644
--- a/documentation/docs/60-appendix/10-migrating.md
+++ b/documentation/docs/60-appendix/10-migrating.md
@@ -150,7 +150,7 @@ See [the FAQ](/faq#integrations) for detailed information about integrations.
 
 ### HTML minifier
 
-Sapper includes `html-minifier` by default. SvelteKit does not include this, but it can be added as a [hook](/docs/hooks#server-hooks-handle):
+Sapper includes `html-minifier` by default. SvelteKit does not include this, but you can add it as a prod dependency and then use it through a [hook](/docs/hooks#server-hooks-handle):
 
 ```js
 // @filename: ambient.d.ts
diff --git a/packages/kit/src/runtime/server/fetch.js b/packages/kit/src/runtime/server/fetch.js
index 89f21b5db..8ff06eec8 100644
--- a/packages/kit/src/runtime/server/fetch.js
+++ b/packages/kit/src/runtime/server/fetch.js
@@ -63,17 +63,7 @@ export function create_fetch({ event, options, state, get_cookie_header }) {
 						if (cookie) request.headers.set('cookie', cookie);
 					}
 
-					let response = await fetch(request);
-
-					if (mode === 'no-cors') {
-						response = new Response('', {
-							status: response.status,
-							statusText: response.statusText,
-							headers: response.headers
-						});
-					}
-
-					return response;
+					return fetch(request);
 				}
 
 				/** @type {Response} */
diff --git a/packages/kit/src/runtime/server/page/load_data.js b/packages/kit/src/runtime/server/page/load_data.js
index f5f20c44f..7b12a46fe 100644
--- a/packages/kit/src/runtime/server/page/load_data.js
+++ b/packages/kit/src/runtime/server/page/load_data.js
@@ -107,129 +107,150 @@ export async function load_data({
 		params: event.params,
 		data: server_data_node?.data ?? null,
 		route: event.route,
-		fetch: async (input, init) => {
-			const cloned_body = input instanceof Request && input.body ? input.clone().body : null;
-			const response = await event.fetch(input, init);
+		fetch: create_universal_fetch(event, state, fetched, csr, resolve_opts),
+		setHeaders: event.setHeaders,
+		depends: () => {},
+		parent
+	});
 
-			const url = new URL(input instanceof Request ? input.url : input, event.url);
-			const same_origin = url.origin === event.url.origin;
+	const data = result ? await unwrap_promises(result) : null;
+	validate_load_response(data, /** @type {string} */ (event.route.id));
+	return data;
+}
+
+/**
+ * @param {Pick<import('types').RequestEvent, 'fetch' | 'url' | 'request' | 'route'>} event
+ * @param {import("types").SSRState} state
+ * @param {import("./types").Fetched[]} fetched
+ * @param {boolean} csr
+ * @param {Pick<Required<import("types").ResolveOptions>, 'filterSerializedResponseHeaders'>} resolve_opts
+ */
+export function create_universal_fetch(event, state, fetched, csr, resolve_opts) {
+	/**
+	 * @param {URL | RequestInfo} input
+	 * @param {RequestInit} [init]
+	 */
+	return async (input, init) => {
+		const cloned_body = input instanceof Request && input.body ? input.clone().body : null;
+		let response = await event.fetch(input, init);
 
-			/** @type {import('types').PrerenderDependency} */
-			let dependency;
+		const url = new URL(input instanceof Request ? input.url : input, event.url);
+		const same_origin = url.origin === event.url.origin;
 
-			if (same_origin) {
-				if (state.prerendering) {
-					dependency = { response, body: null };
-					state.prerendering.dependencies.set(url.pathname, dependency);
-				}
+		/** @type {import('types').PrerenderDependency} */
+		let dependency;
+
+		if (same_origin) {
+			if (state.prerendering) {
+				dependency = { response, body: null };
+				state.prerendering.dependencies.set(url.pathname, dependency);
+			}
+		} else {
+			// simulate CORS errors and "no access to body in no-cors mode" server-side for consistency with client-side behaviour
+			const mode = input instanceof Request ? input.mode : init?.mode ?? 'cors';
+			if (mode === 'no-cors') {
+				response = new Response('', {
+					status: response.status,
+					statusText: response.statusText,
+					headers: response.headers
+				});
 			} else {
-				// simulate CORS errors server-side for consistency with client-side behaviour
-				const mode = input instanceof Request ? input.mode : init?.mode ?? 'cors';
-				if (mode !== 'no-cors') {
-					const acao = response.headers.get('access-control-allow-origin');
-					if (!acao || (acao !== event.url.origin && acao !== '*')) {
-						throw new Error(
-							`CORS error: ${
-								acao ? 'Incorrect' : 'No'
-							} 'Access-Control-Allow-Origin' header is present on the requested resource`
-						);
-					}
+				const acao = response.headers.get('access-control-allow-origin');
+				if (!acao || (acao !== event.url.origin && acao !== '*')) {
+					throw new Error(
+						`CORS error: ${
+							acao ? 'Incorrect' : 'No'
+						} 'Access-Control-Allow-Origin' header is present on the requested resource`
+					);
 				}
 			}
+		}
 
-			const proxy = new Proxy(response, {
-				get(response, key, _receiver) {
-					async function text() {
-						const body = await response.text();
-
-						if (!body || typeof body === 'string') {
-							const status_number = Number(response.status);
-							if (isNaN(status_number)) {
-								throw new Error(
-									`response.status is not a number. value: "${
-										response.status
-									}" type: ${typeof response.status}`
-								);
-							}
-
-							fetched.push({
-								url: same_origin ? url.href.slice(event.url.origin.length) : url.href,
-								method: event.request.method,
-								request_body: /** @type {string | ArrayBufferView | undefined} */ (
-									input instanceof Request && cloned_body
-										? await stream_to_string(cloned_body)
-										: init?.body
-								),
-								response_body: body,
-								response: response
-							});
-						}
+		const proxy = new Proxy(response, {
+			get(response, key, _receiver) {
+				async function text() {
+					const body = await response.text();
 
-						if (dependency) {
-							dependency.body = body;
+					if (!body || typeof body === 'string') {
+						const status_number = Number(response.status);
+						if (isNaN(status_number)) {
+							throw new Error(
+								`response.status is not a number. value: "${
+									response.status
+								}" type: ${typeof response.status}`
+							);
 						}
 
-						return body;
+						fetched.push({
+							url: same_origin ? url.href.slice(event.url.origin.length) : url.href,
+							method: event.request.method,
+							request_body: /** @type {string | ArrayBufferView | undefined} */ (
+								input instanceof Request && cloned_body
+									? await stream_to_string(cloned_body)
+									: init?.body
+							),
+							response_body: body,
+							response: response
+						});
 					}
 
-					if (key === 'arrayBuffer') {
-						return async () => {
-							const buffer = await response.arrayBuffer();
+					if (dependency) {
+						dependency.body = body;
+					}
 
-							if (dependency) {
-								dependency.body = new Uint8Array(buffer);
-							}
+					return body;
+				}
 
-							// TODO should buffer be inlined into the page (albeit base64'd)?
-							// any conditions in which it shouldn't be?
+				if (key === 'arrayBuffer') {
+					return async () => {
+						const buffer = await response.arrayBuffer();
 
-							return buffer;
-						};
-					}
+						if (dependency) {
+							dependency.body = new Uint8Array(buffer);
+						}
 
-					if (key === 'text') {
-						return text;
-					}
+						// TODO should buffer be inlined into the page (albeit base64'd)?
+						// any conditions in which it shouldn't be?
 
-					if (key === 'json') {
-						return async () => {
-							return JSON.parse(await text());
-						};
-					}
+						return buffer;
+					};
+				}
 
-					return Reflect.get(response, key, response);
+				if (key === 'text') {
+					return text;
+				}
+
+				if (key === 'json') {
+					return async () => {
+						return JSON.parse(await text());
+					};
 				}
-			});
-
-			if (csr) {
-				// ensure that excluded headers can't be read
-				const get = response.headers.get;
-				response.headers.get = (key) => {
-					const lower = key.toLowerCase();
-					const value = get.call(response.headers, lower);
-					if (value && !lower.startsWith('x-sveltekit-')) {
-						const included = resolve_opts.filterSerializedResponseHeaders(lower, value);
-						if (!included) {
-							throw new Error(
-								`Failed to get response header "${lower}" — it must be included by the \`filterSerializedResponseHeaders\` option: https://kit.svelte.dev/docs/hooks#server-hooks-handle (at ${event.route.id})`
-							);
-						}
-					}
 
-					return value;
-				};
+				return Reflect.get(response, key, response);
 			}
+		});
 
-			return proxy;
-		},
-		setHeaders: event.setHeaders,
-		depends: () => {},
-		parent
-	});
+		if (csr) {
+			// ensure that excluded headers can't be read
+			const get = response.headers.get;
+			response.headers.get = (key) => {
+				const lower = key.toLowerCase();
+				const value = get.call(response.headers, lower);
+				if (value && !lower.startsWith('x-sveltekit-')) {
+					const included = resolve_opts.filterSerializedResponseHeaders(lower, value);
+					if (!included) {
+						throw new Error(
+							`Failed to get response header "${lower}" — it must be included by the \`filterSerializedResponseHeaders\` option: https://kit.svelte.dev/docs/hooks#server-hooks-handle (at ${event.route.id})`
+						);
+					}
+				}
 
-	const data = result ? await unwrap_promises(result) : null;
-	validate_load_response(data, /** @type {string} */ (event.route.id));
-	return data;
+				return value;
+			};
+		}
+
+		return proxy;
+	};
 }
 
 /**
diff --git a/packages/kit/src/runtime/server/page/load_data.spec.js b/packages/kit/src/runtime/server/page/load_data.spec.js
new file mode 100644
index 000000000..493cbcfc5
--- /dev/null
+++ b/packages/kit/src/runtime/server/page/load_data.spec.js
@@ -0,0 +1,73 @@
+import { test } from 'uvu';
+import * as assert from 'uvu/assert';
+import { create_universal_fetch } from './load_data.js';
+
+/**
+ * @param {Partial<Pick<import('types').RequestEvent, 'fetch' | 'url' | 'request' | 'route'>>} event
+ */
+function create_fetch(event) {
+	event.fetch = event.fetch || (async () => new Response('foo'));
+	event.request = event.request || new Request('doesnt:matter');
+	event.route = event.route || { id: 'foo' };
+	event.url = event.url || new URL('https://domain-a.com');
+	return create_universal_fetch(
+		/** @type {Pick<import('types').RequestEvent, 'fetch' | 'url' | 'request' | 'route'>} */ (
+			event
+		),
+		{ getClientAddress: () => '' },
+		[],
+		true,
+		{
+			filterSerializedResponseHeaders: () => false
+		}
+	);
+}
+
+test('sets body to empty when mode is no-cors', async () => {
+	const fetch = create_fetch({});
+	const response = await fetch('https://domain-b.com', { mode: 'no-cors' });
+	const text = await response.text();
+	assert.equal(text, '');
+});
+
+test('keeps body when mode isnt no-cors on same domain', async () => {
+	const fetch = create_fetch({});
+	const response = await fetch('https://domain-a.com');
+	const text = await response.text();
+	assert.equal(text, 'foo');
+});
+
+test('succeeds when acao header present on cors', async () => {
+	const fetch = create_fetch({
+		fetch: async () => new Response('foo', { headers: { 'access-control-allow-origin': '*' } })
+	});
+	const response = await fetch('https://domain-a.com');
+	const text = await response.text();
+	assert.equal(text, 'foo');
+});
+
+test('errors when no acao header present on cors', async () => {
+	const fetch = create_fetch({});
+	try {
+		const response = await fetch('https://domain-b.com');
+		await response.text();
+		assert.unreachable('should have thrown cors error');
+	} catch (e) {
+		assert.ok(e instanceof Error);
+		assert.match(
+			e.message,
+			/CORS error: No 'Access-Control-Allow-Origin' header is present on the requested resource/
+		);
+	}
+});
+
+test('errors when trying to access non-serialized request headers on the server', async () => {
+	const fetch = create_fetch({});
+	const response = await fetch('https://domain-a.com');
+	assert.throws(
+		() => response.headers.get('content-type'),
+		/Failed to get response header "content-type" — it must be included by the `filterSerializedResponseHeaders` option/
+	);
+});
+
+test.run();


Test patch:
diff --git a/packages/kit/test/apps/basics/src/routes/load/cors/+page.js b/packages/kit/test/apps/basics/src/routes/load/cors/+page.js
deleted file mode 100644
index 9fbd10aed..000000000
--- a/packages/kit/test/apps/basics/src/routes/load/cors/+page.js
+++ /dev/null
@@ -1,8 +0,0 @@
-/** @type {import('./$types').PageLoad} */
-export async function load({ fetch, url }) {
-	const res = await fetch(`http://localhost:${url.searchParams.get('port')}`);
-
-	return {
-		text: await res.text()
-	};
-}
diff --git a/packages/kit/test/apps/basics/src/routes/load/cors/+page.svelte b/packages/kit/test/apps/basics/src/routes/load/cors/+page.svelte
deleted file mode 100644
index a5063216b..000000000
--- a/packages/kit/test/apps/basics/src/routes/load/cors/+page.svelte
+++ /dev/null
@@ -1,6 +0,0 @@
-<script>
-	/** @type {import('./$types').PageData} */
-	export let data;
-</script>
-
-<h1>{@html data.text}</h1>
diff --git a/packages/kit/test/apps/basics/src/routes/load/cors/no-cors/+page.js b/packages/kit/test/apps/basics/src/routes/load/cors/no-cors/+page.js
deleted file mode 100644
index 70614cbbc..000000000
--- a/packages/kit/test/apps/basics/src/routes/load/cors/no-cors/+page.js
+++ /dev/null
@@ -1,10 +0,0 @@
-/** @type {import('./$types').PageLoad} */
-export async function load({ fetch, url }) {
-	const res = await fetch(`http://localhost:${url.searchParams.get('port')}`, {
-		mode: 'no-cors'
-	});
-
-	return {
-		text: await res.text()
-	};
-}
diff --git a/packages/kit/test/apps/basics/src/routes/load/cors/no-cors/+page.svelte b/packages/kit/test/apps/basics/src/routes/load/cors/no-cors/+page.svelte
deleted file mode 100644
index d689a601e..000000000
--- a/packages/kit/test/apps/basics/src/routes/load/cors/no-cors/+page.svelte
+++ /dev/null
@@ -1,6 +0,0 @@
-<script>
-	/** @type {import('./$types').PageData} */
-	export let data;
-</script>
-
-<h1>result: {@html data.text}</h1>
diff --git a/packages/kit/test/apps/basics/src/routes/load/cors/server-only/+page.server.js b/packages/kit/test/apps/basics/src/routes/load/cors/server-only/+page.server.js
deleted file mode 100644
index 7a412012c..000000000
--- a/packages/kit/test/apps/basics/src/routes/load/cors/server-only/+page.server.js
+++ /dev/null
@@ -1,8 +0,0 @@
-/** @type {import('./$types').PageServerLoad} */
-export async function load({ fetch, url }) {
-	const res = await fetch(`http://localhost:${url.searchParams.get('port')}`);
-
-	return {
-		text: await res.text()
-	};
-}
diff --git a/packages/kit/test/apps/basics/src/routes/load/cors/server-only/+page.svelte b/packages/kit/test/apps/basics/src/routes/load/cors/server-only/+page.svelte
deleted file mode 100644
index a5063216b..000000000
--- a/packages/kit/test/apps/basics/src/routes/load/cors/server-only/+page.svelte
+++ /dev/null
@@ -1,6 +0,0 @@
-<script>
-	/** @type {import('./$types').PageData} */
-	export let data;
-</script>
-
-<h1>{@html data.text}</h1>
diff --git a/packages/kit/test/apps/basics/src/routes/load/fetch-request-headers-invalid-access/+page.js b/packages/kit/test/apps/basics/src/routes/load/fetch-request-headers-invalid-access/+page.js
deleted file mode 100644
index eda2a2da8..000000000
--- a/packages/kit/test/apps/basics/src/routes/load/fetch-request-headers-invalid-access/+page.js
+++ /dev/null
@@ -1,8 +0,0 @@
-/** @type {import('./$types').PageLoad} */
-export async function load({ fetch }) {
-	let res = await fetch('./irrelevant', {
-		method: 'GET'
-	});
-
-	res.headers.get('content-type');
-}
diff --git a/packages/kit/test/apps/basics/src/routes/load/fetch-request-headers-invalid-access/+page.svelte b/packages/kit/test/apps/basics/src/routes/load/fetch-request-headers-invalid-access/+page.svelte
deleted file mode 100644
index e69de29bb..000000000
diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js
index 0a8476d16..459828418 100644
--- a/packages/kit/test/apps/basics/test/test.js
+++ b/packages/kit/test/apps/basics/test/test.js
@@ -868,17 +868,6 @@ test.describe('Load', () => {
 		}
 	});
 
-	test('errors when trying to access non-serialized request headers on the server', async ({
-		page,
-		read_errors
-	}) => {
-		await page.goto('/load/fetch-request-headers-invalid-access');
-
-		expect(read_errors(`/load/fetch-request-headers-invalid-access`).message).toContain(
-			'Failed to get response header "content-type" — it must be included by the `filterSerializedResponseHeaders` option'
-		);
-	});
-
 	test('exposes rawBody as a DataView to endpoints', async ({ page, clicknav }) => {
 		await page.goto('/load/raw-body');
 		await clicknav('[href="/load/raw-body/dataview"]');
@@ -952,37 +941,6 @@ test.describe('Load', () => {
 
 		expect(await page.textContent('h1')).toBe('true');
 	});
-
-	test('CORS errors are simulated server-side for shared load functions', async ({
-		page,
-		read_errors
-	}) => {
-		const { port, close } = await start_server(async (req, res) => {
-			res.end('hello');
-		});
-
-		await page.goto(`/load/cors?port=${port}`);
-		expect(await page.textContent('h1')).toBe('500');
-		expect(read_errors(`/load/cors`).message).toContain(
-			`CORS error: No 'Access-Control-Allow-Origin' header is present on the requested resource`
-		);
-
-		await page.goto(`/load/cors/no-cors?port=${port}`);
-		expect(await page.textContent('h1')).toBe('result: ');
-
-		await close();
-	});
-
-	test('CORS errors are skipped for server-only load functions', async ({ page }) => {
-		const { port, close } = await start_server(async (req, res) => {
-			res.end('hello');
-		});
-
-		await page.goto(`/load/cors/server-only?port=${port}`);
-		expect(await page.textContent('h1')).toBe('hello');
-
-		await close();
-	});
 });
 
 test.describe('Nested layouts', () => {