fix: allow non-prerendered API endpoint calls during reroute when prerendering

.changeset/khaki-buttons-add.md

@@ -0,0 +1,5 @@
+---
+'@sveltejs/kit': patch
+---
+
+fix: allow non-prerendered API endpoint calls during reroute when prerendering

packages/kit/src/runtime/server/endpoint.js

@@ -29,7 +29,7 @@ export async function render_endpoint(event, mod, state) {
 		throw new Error('Cannot prerender endpoints that have mutative methods');
 	}
 
-	if (state.prerendering && !prerender) {
+	if (state.prerendering && !state.prerendering.inside_reroute && !prerender) {
 		if (state.depth > 0) {
 			// if request came from a prerendered page, bail
 			throw new Error(`${event.route.id} is not prerenderable`);
@@ -41,7 +41,7 @@ export async function render_endpoint(event, mod, state) {
 	}
 
 	try {
-		let response = await with_event(event, () =>
+		const response = await with_event(event, () =>
 			handler(/** @type {import('@sveltejs/kit').RequestEvent<Record<string, any>>} */ (event))
 		);
 
@@ -51,15 +51,28 @@ export async function render_endpoint(event, mod, state) {
 			);
 		}
 
-		if (state.prerendering) {
-			// the returned Response might have immutable Headers
-			// so we should clone them before trying to mutate them
-			response = new Response(response.body, {
+		if (state.prerendering && (!state.prerendering.inside_reroute || prerender)) {
+			// The returned Response might have immutable Headers
+			// so we should clone them before trying to mutate them.
+			// We also need to clone the response body since it may be read twice during prerendering
+			const cloned = new Response(response.clone().body, {
 				status: response.status,
 				statusText: response.statusText,
 				headers: new Headers(response.headers)
 			});
-			response.headers.set('x-sveltekit-prerender', String(prerender));
+			cloned.headers.set('x-sveltekit-prerender', String(prerender));
+
+			if (state.prerendering.inside_reroute && prerender) {
+				// Without this, the route wouldn't be recorded as prerendered,
+				// because there's nothing after reroute that would do that.
+				cloned.headers.set(
+					'x-sveltekit-routeid',
+					encodeURI(/** @type {string} */ (event.route.id))
+				);
+				state.prerendering.dependencies.set(event.url.pathname, { response: cloned, body: null });
+			} else {
+				return cloned;
+			}
 		}
 
 		return response;

packages/kit/src/runtime/server/respond.js

@@ -183,14 +183,21 @@ export async function respond(request, options, manifest, state) {
 
 	let resolved_path;
 
+	const prerendering_reroute_state = state.prerendering?.inside_reroute;
 	try {
+		// For the duration or a reroute, disable the prerendering state as reroute could call API endpoints
+		// which would end up in the wrong logic path if not disabled.
+		if (state.prerendering) state.prerendering.inside_reroute = true;
+
 		// reroute could alter the given URL, so we pass a copy
 		resolved_path =
 			(await options.hooks.reroute({ url: new URL(url), fetch: event.fetch })) ?? url.pathname;
 	} catch {
 		return text('Internal Server Error', {
 			status: 500
 		});
+	} finally {
+		if (state.prerendering) state.prerendering.inside_reroute = prerendering_reroute_state;
 	}
 
 	try {
@@ -349,7 +356,9 @@ export async function respond(request, options, manifest, state) {
 
 		set_trailing_slash(trailing_slash);
 
-		if (state.prerendering && !state.prerendering.fallback) disable_search(url);
+		if (state.prerendering && !state.prerendering.fallback && !state.prerendering.inside_reroute) {
+			disable_search(url);
+		}
 
 		const response = await with_event(event, () =>
 			options.hooks.handle({

packages/kit/src/types/internal.d.ts

@@ -216,6 +216,8 @@ export interface PrerenderOptions {
 	cache?: string; // including this here is a bit of a hack, but it makes it easy to add <meta http-equiv>
 	fallback?: boolean;
 	dependencies: Map<string, PrerenderDependency>;
+	/** True for the duration of a call to the `reroute` hook */
+	inside_reroute?: boolean;
 }
 
 export type RecursiveRequired<T> = {

packages/kit/test/apps/basics/src/hooks.js

@@ -31,6 +31,10 @@ export const reroute = ({ url, fetch }) => {
 		return fetch('/reroute/api').then((r) => r.text());
 	}
 
+	if (url.pathname === '/reroute/async/c') {
+		return fetch('/reroute/api/prerendered').then((r) => r.text());
+	}
+
 	if (url.pathname === '/reroute/prerendered/to-destination') {
 		return '/reroute/prerendered/destination';
 	}

packages/kit/test/apps/basics/src/routes/reroute/api/[prerendered]/+server.ts

@@ -0,0 +1,9 @@
+import { text } from '@sveltejs/kit';
+
+// This is inside a route with a route segment to ensure that the route is marked as prerendered
+// as part of reroute resolution even when no `entries` is given.
+export const prerender = true;
+
+export function GET() {
+	return text('/reroute/async/b');
+}

packages/kit/test/apps/basics/src/routes/reroute/async/+page.svelte

@@ -1 +1,4 @@
 <a href="/reroute/async/a">Go to url that should be rewritten</a>
+<a href="/reroute/async/c"
+	>Go to url that should be rewritten and its reroute api call prerendered</a
+>

packages/kit/test/apps/basics/src/routes/reroute/async/+page.ts

@@ -0,0 +1,3 @@
+// through this we test that the crawler during prerendering does not fail on
+// reroute calls that call non-prerendered endpoints.
+export const prerender = true;

packages/kit/test/apps/basics/svelte.config.js

@@ -25,7 +25,13 @@ const config = {
 				'/routing/prerendered/trailing-slash/never',
 				'/routing/prerendered/trailing-slash/ignore'
 			],
-			handleHttpError: 'warn'
+			handleHttpError: ({ path, message }) => {
+				if (path.includes('/reroute/async')) {
+					throw new Error('shouldnt error on ' + path);
+				}
+
+				console.warn(message);
+			}
 		},
 
 		version: {

packages/kit/test/apps/basics/test/client.test.js

@@ -1265,7 +1265,7 @@
 			expect(page.locator('p.loadingfail')).toBeHidden();
 		});
 
 		test('Catches fetch errors from server load functions (direct hit)', async ({ page }) => {
 			page.goto('/streaming/server-error');
 			await expect(page.locator('p.eager')).toHaveText('eager');
 			await expect(page.locator('p.fail')).toHaveText('fail');
@@ -1461,6 +1461,14 @@
 		);
 	});
 
+	test('Apply async prerendered reroute during client side navigation', async ({ page }) => {
+		await page.goto('/reroute/async');
+		await page.click("a[href='/reroute/async/c']");
+		expect(await page.textContent('h1')).toContain(
+			'Successfully rewritten, URL should still show a: /reroute/async/c'
+		);
+	});
+
 	test('Apply reroute to prerendered page during client side navigation', async ({ page }) => {
 		await page.goto('/reroute/prerendered');
 		await page.click("a[href='/reroute/prerendered/to-destination']");
@@ -1531,7 +1539,7 @@
 });
 
 test.describe('INP', () => {
 	test('does not block next paint', async ({ page }) => {
 		// Thanks to https://publishing-project.rivendellweb.net/measuring-performance-tasks-with-playwright/#interaction-to-next-paint-inp
 		async function measureInteractionToPaint(selector) {
 			return page.evaluate(async (selector) => {

packages/kit/test/apps/basics/test/server.test.js

@@ -683,6 +683,13 @@ test.describe('reroute', () => {
 		);
 	});
 
+	test('Apply async prerendered reroute when directly accessing a page', async ({ page }) => {
+		await page.goto('/reroute/async/c');
+		expect(await page.textContent('h1')).toContain(
+			'Successfully rewritten, URL should still show a: /reroute/async/c'
+		);
+	});
+
 	test('Apply reroute to prerendered page when directly accessing a page', async ({ page }) => {
 		await page.goto('/reroute/prerendered/to-destination');
 		expect(await page.textContent('h1')).toContain('reroute that points to prerendered page works');