fix: don't mask errors with a "header is already set" error when rendering the error page

.changeset/tidy-ravens-smile.md

@@ -0,0 +1,5 @@
+---
+'@sveltejs/kit': patch
+---
+
+fix: render the error page with the post-`handle` headers instead of failing on headers the crashed render already set

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

@@ -26,6 +26,8 @@ export async function respond_with_error({ event, state, error, resolve_opts })
 		return static_error_page(transformed.status, transformed.message);
 	}
 
+	state.reset_headers?.();
+
 	/** @type {import('./types.js').Fetched[]} */
 	const fetched = [];
 	try {

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

@@ -175,7 +175,7 @@ export async function internal_respond(request, state) {
 	}
 
 	/** @type {Record<string, string>} */
-	const headers = {};
+	let headers = {};
 
 	const { cookies, new_cookies, get_cookie_header, set_internal, set_trailing_slash } = get_cookies(
 		request,
@@ -583,6 +583,9 @@ export async function internal_respond(request, state) {
 	 * @param {import('@sveltejs/kit/hooks').ResolveOptions} [opts]
 	 */
 	async function resolve(event, page_nodes, opts) {
+		const handle_headers = { ...headers };
+		state.reset_headers ??= () => (headers = { ...handle_headers });
+
 		try {
 			if (opts) {
 				resolve_opts = {

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

@@ -696,6 +696,8 @@ export type RecordSpan = <T>(options: {
  * used for tracking things like remote function calls
  */
 export interface RequestState {
+	/** discards headers set by a render that failed, so the error page starts from the post-`handle` state */
+	reset_headers?(): void;
 	readonly getClientAddress: () => string;
 	readonly platform?: any;
 	/** @internal reads from the filesystem when user code tries to fetch a static asset */

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

@@ -138,7 +138,14 @@ export const handle = sequence(
 		const response = await resolve(event, {
 			transformPageChunk: event.url.pathname.startsWith('/transform-page-chunk')
 				? ({ html }) => html.replace('__REPLACEME__', 'Worked!')
-				: undefined
+				: event.url.pathname.startsWith('/errors/error-page-setheaders')
+					? ({ html }) => {
+							if (html.includes('makes the page transform crash')) {
+								throw new Error('Crashing now');
+							}
+							return html;
+						}
+					: undefined
 		});
 
 		try {
@@ -192,7 +199,10 @@ export const handle = sequence(
 		return resolve(event);
 	},
 	async ({ event, resolve }) => {
-		if (['/non-existent-route', '/non-existent-route-loop'].includes(event.url.pathname)) {
+		if (
+			event.url.pathname.startsWith('/errors/error-page-setheaders') ||
+			['/non-existent-route', '/non-existent-route-loop'].includes(event.url.pathname)
+		) {
 			event.locals.url = new URL(event.request.url);
 		}
 		return resolve(event);

packages/kit/test/apps/basics/src/routes/+layout.server.js

@@ -7,7 +7,11 @@ if (JSON.parse(SOME_JSON).answer !== 42) {
 }
 
 /** @type {import('./$types').LayoutServerLoad} */
-export async function load({ cookies, locals, fetch }) {
+export async function load({ cookies, locals, fetch, setHeaders }) {
+	if (locals.url?.pathname.startsWith('/errors/error-page-setheaders')) {
+		setHeaders({ 'cache-control': 'private, max-age=60' });
+	}
+
 	if (locals.url?.pathname === '/non-existent-route') {
 		await fetch('/prerendering/prerendered-endpoint/api').then((r) => r.json());
 	}

packages/kit/test/apps/basics/src/routes/errors/error-page-setheaders-leak/+page.server.js

@@ -0,0 +1,4 @@
+/** @type {import('./$types').PageServerLoad} */
+export function load({ setHeaders }) {
+	setHeaders({ 'x-failed-render': '1' });
+}

packages/kit/test/apps/basics/src/routes/errors/error-page-setheaders-leak/+page.svelte

@@ -0,0 +1 @@
+<p>this text makes the page transform crash</p>

packages/kit/test/apps/basics/src/routes/errors/error-page-setheaders/+page.svelte

@@ -0,0 +1 @@
+<p>this text makes the page transform crash</p>

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

@@ -1046,6 +1046,23 @@ test.describe('setHeaders', () => {
 		expect(cookies).toMatch('cookie1=value1');
 		expect(cookies).toMatch('cookie2=value2');
 	});
+
+	test('renders the error page when the root layout sets headers', async ({ request }) => {
+		const response = await request.get('/errors/error-page-setheaders');
+
+		expect(response.status()).toBe(500);
+		expect(response.headers()['cache-control']).toBe('private, max-age=60');
+		expect(await response.text()).toContain('This is your custom error page saying:');
+	});
+
+	test('drops headers set by the failed render from the error page', async ({ request }) => {
+		const response = await request.get('/errors/error-page-setheaders-leak');
+
+		expect(response.status()).toBe(500);
+		expect(response.headers()['x-failed-render']).toBeUndefined();
+		expect(response.headers()['cache-control']).toBe('private, max-age=60');
+		expect(await response.text()).toContain('This is your custom error page saying:');
+	});
 });
 
 test.describe('cookies', () => {