[fix] reading from same response body twice during prerender (#3473) (#…

…3521)

* Add failing test for #3473 prerender error body used already

* Fix reading from same response body twice during prerender

* Fix quotes

* Add changeset

* Fix casing for internal variables

* Revert change of cloning response as bug was actually in prerender.js

* Avoid reading response body twice

* Revert "Avoid reading response body twice"

This reverts commit cecf7dd.

Revert being stupid #1

* Revert "Revert change of cloning response as bug was actually in prerender.js"

This reverts commit 5e5f30b.

Revert being stupid #2

* store buffered depenedency bodies for prerendering

* failing test for non-buffered endpoint data

* use buffered body if available, otherwise buffer

Co-authored-by: Rich Harris <[email protected]>

.changeset/nine-dots-occur.md

@@ -0,0 +1,5 @@
+---
+'@sveltejs/kit': patch
+---
+
+[fix] reading from same response body twice during prerender (#3473)

packages/kit/src/core/adapt/prerender/prerender.js

@@ -130,8 +130,9 @@ export async function prerender({ cwd, out, log, config, build_data, fallback, a
 	 * @param {string?} referrer
 	 */
 	async function visit(path, decoded_path, referrer) {
-		/** @type {Map<string, Response>} */
+		/** @type {Map<string, import('types/internal').PrerenderDependency>} */
 		const dependencies = new Map();
+
 		const render_path = config.kit.paths?.base
 			? `http://sveltekit-prerender${config.kit.paths.base}${path === '/' ? '' : path}`
 			: `http://sveltekit-prerender${path}`;
@@ -192,9 +193,11 @@ export async function prerender({ cwd, out, log, config, build_data, fallback, a
 			}
 
 			for (const [dependency_path, result] of dependencies) {
-				const response_type = Math.floor(result.status / 100);
+				const { status, headers } = result.response;
+
+				const response_type = Math.floor(status / 100);
 
-				const is_html = result.headers.get('content-type') === 'text/html';
+				const is_html = headers.get('content-type') === 'text/html';
 
 				const parts = dependency_path.split('/');
 				if (is_html && parts[parts.length - 1] !== 'index.html') {
@@ -204,16 +207,17 @@ export async function prerender({ cwd, out, log, config, build_data, fallback, a
 				const file = `${out}${parts.join('/')}`;
 				mkdirp(dirname(file));
 
-				if (result.body) {
-					writeFileSync(file, await result.text());
-					paths.push(dependency_path);
-				}
+				writeFileSync(
+					file,
+					result.body === null ? new Uint8Array(await result.response.arrayBuffer()) : result.body
+				);
+				paths.push(dependency_path);
 
 				if (response_type === OK) {
-					log.info(`${result.status} ${dependency_path}`);
+					log.info(`${status} ${dependency_path}`);
 				} else {
 					error({
-						status: result.status,
+						status,
 						path: dependency_path,
 						referrer: path,
 						referenceType: 'fetched'

packages/kit/src/runtime/server/page/load_node.js

@@ -99,6 +99,9 @@ export async function load_node({
 				/** @type {Response} */
 				let response;
 
+				/** @type {import('types/internal').PrerenderDependency} */
+				let dependency;
+
 				// handle fetch requests for static assets. e.g. prebaked data, etc.
 				// we need to support everything the browser's fetch supports
 				const prefix = options.paths.assets || options.paths.base;
@@ -125,8 +128,6 @@ export async function load_node({
 						response = await fetch(`${url.origin}/${file}`, /** @type {RequestInit} */ (opts));
 					}
 				} else if (is_root_relative(resolved)) {
-					const relative = resolved;
-
 					if (opts.credentials !== 'omit') {
 						uses_credentials = true;
 
@@ -150,20 +151,15 @@ export async function load_node({
 						throw new Error('Request body must be a string');
 					}
 
-					const rendered = await respond(
-						new Request(new URL(requested, event.url).href, opts),
-						options,
-						{
-							fetched: requested,
-							initiator: route
-						}
-					);
+					response = await respond(new Request(new URL(requested, event.url).href, opts), options, {
+						fetched: requested,
+						initiator: route
+					});
 
 					if (state.prerender) {
-						state.prerender.dependencies.set(relative, rendered);
+						dependency = { response, body: null };
+						state.prerender.dependencies.set(resolved, dependency);
 					}
-
-					response = rendered;
 				} else {
 					// external
 					if (resolved.startsWith('//')) {
@@ -219,9 +215,28 @@ export async function load_node({
 									});
 							}
 
+							if (dependency) {
+								dependency.body = body;
+							}
+
 							return body;
 						}
 
+						if (key === 'arrayBuffer') {
+							return async () => {
+								const buffer = await response.arrayBuffer();
+
+								if (dependency) {
+									dependency.body = new Uint8Array(buffer);
+								}
+
+								// TODO should buffer be inlined into the page (albeit base64'd)?
+								// any conditions in which it shouldn't be?
+
+								return buffer;
+							};
+						}
+
 						if (key === 'text') {
 							return text;
 						}

packages/kit/test/prerendering/basics/src/routes/fetch-endpoint/buffered.json.js

@@ -0,0 +1,5 @@
+export async function get() {
+	return {
+		body: { answer: 42 }
+	};
+}

packages/kit/test/prerendering/basics/src/routes/fetch-endpoint/buffered.svelte

@@ -0,0 +1,18 @@
+<script context="module">
+	/** @type {import('@sveltejs/kit').Load} */
+	export async function load({ fetch }) {
+		const url = '/fetch-endpoint/buffered.json';
+		const res = await fetch(url);
+
+		return {
+			props: await res.json()
+		};
+	}
+</script>
+
+<script>
+	/** @type {number} */
+	export let answer;
+</script>
+
+<h1>the answer is {answer}</h1>

packages/kit/test/prerendering/basics/src/routes/fetch-endpoint/not-buffered.json.js

@@ -0,0 +1,5 @@
+export async function get() {
+	return {
+		body: { answer: 42 }
+	};
+}

packages/kit/test/prerendering/basics/src/routes/fetch-endpoint/not-buffered.svelte

@@ -0,0 +1,20 @@
+<script context="module">
+	/** @type {import('@sveltejs/kit').Load} */
+	export async function load({ fetch }) {
+		const url = '/fetch-endpoint/not-buffered.json';
+		const res = await fetch(url);
+
+		return {
+			props: {
+				headers: res.headers
+			}
+		};
+	}
+</script>
+
+<script>
+	/** @type {Headers} */
+	export let headers;
+</script>
+
+<h1>content-type: {headers.get('content-type')}</h1>

packages/kit/test/prerendering/basics/test/test.js

@@ -42,4 +42,20 @@ test('inserts http-equiv tag for cache-control headers', () => {
 	assert.ok(content.includes('<meta http-equiv="cache-control" content="max-age=300">'));
 });
 
+test('renders page with data from endpoint', () => {
+	const content = read('fetch-endpoint/buffered/index.html');
+	assert.ok(content.includes('<h1>the answer is 42</h1>'));
+
+	const json = read('fetch-endpoint/buffered.json');
+	assert.equal(json, JSON.stringify({ answer: 42 }));
+});
+
+test('renders page with unbuffered data from endpoint', () => {
+	const content = read('fetch-endpoint/not-buffered/index.html');
+	assert.ok(content.includes('<h1>content-type: application/json; charset=utf-8</h1>'), content);
+
+	const json = read('fetch-endpoint/not-buffered.json');
+	assert.equal(json, JSON.stringify({ answer: 42 }));
+});
+
 test.run();

packages/kit/types/internal.d.ts

@@ -5,10 +5,15 @@ import { Either } from './helper';
 import { ExternalFetch, GetSession, Handle, HandleError, RequestEvent } from './hooks';
 import { Load } from './page';
 
+export interface PrerenderDependency {
+	response: Response;
+	body: null | string | Uint8Array;
+}
+
 export interface PrerenderOptions {
 	fallback?: string;
 	all: boolean;
-	dependencies: Map<string, Response>;
+	dependencies: Map<string, PrerenderDependency>;
 }
 
 export interface AppModule {