Expose status and error to layouts, if rendering a generic error page

.changeset/purple-rats-wink.md

@@ -0,0 +1,5 @@
+---
+'@sveltejs/kit': patch
+---
+
+Render generic error page if \_\_layout returns error while rendering full error page

packages/kit/src/runtime/load.js

@@ -26,7 +26,10 @@ export function normalize(loaded) {
 		const status = loaded.status;
 
 		if (!loaded.error && has_error_status) {
-			return { status: status || 500, error: new Error() };
+			return {
+				status: status || 500,
+				error: new Error(`${status}`)
+			};
 		}
 
 		const error = typeof loaded.error === 'string' ? new Error(loaded.error) : loaded.error;

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

@@ -3,7 +3,7 @@ import { render_page } from './page/index.js';
 import { render_response } from './page/render.js';
 import { respond_with_error } from './page/respond_with_error.js';
 import { coalesce_to_error } from '../../utils/error.js';
-import { decode_params, serialize_error } from './utils.js';
+import { decode_params, serialize_error, GENERIC_ERROR } from './utils.js';
 import { normalize_path } from '../../utils/url.js';
 import { exec } from '../../utils/routing.js';
 import { negotiate } from '../../utils/http.js';
@@ -274,6 +274,12 @@ export async function respond(request, options, state) {
 					}
 				}
 
+				if (state.initiator === GENERIC_ERROR) {
+					return new Response('Internal Server Error', {
+						status: 500
+					});
+				}
+
 				// if this request came direct from the user, rather than
 				// via a `fetch` in a `load`, render a 404 page
 				if (!state.initiator) {
@@ -328,6 +334,7 @@ export async function respond(request, options, state) {
 			});
 		}
 
+		// TODO is this necessary? should we just return a plain 500 at this point?
 		try {
 			const $session = await options.hooks.getSession(event);
 			return await respond_with_error({

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

@@ -13,14 +13,13 @@ import { domain_matches, path_matches } from './cookie.js';
  *   event: import('types').RequestEvent;
  *   options: import('types').SSROptions;
  *   state: import('types').SSRState;
- *   route: import('types').SSRPage | null;
+ *   route: import('types').SSRPage | import('types').SSRErrorPage;
  *   node: import('types').SSRNode;
  *   $session: any;
  *   stuff: Record<string, any>;
- *   is_error: boolean;
  *   is_leaf: boolean;
- *   status?: number;
- *   error?: Error;
+ *   status: number | null;
+ *   error: Error | null;
  * }} opts
  * @returns {Promise<import('./types').Loaded>}
  */
@@ -32,7 +31,6 @@ export async function load_node({
 	node,
 	$session,
 	stuff,
-	is_error,
 	is_leaf,
 	status,
 	error
@@ -344,8 +342,8 @@ export async function load_node({
 				return proxy;
 			},
 			stuff: { ...stuff },
-			status: (is_error ? status : shadow.status) ?? null,
-			error: is_error ? error ?? null : null
+			status: shadow.status ?? status ?? null,
+			error: error ?? null
 		};
 
 		if (options.dev) {

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

@@ -23,7 +23,7 @@ const updated = {
  *   state: import('types').SSRState;
  *   $session: any;
  *   page_config: { hydrate: boolean, router: boolean };
- *   status: number;
+ *   status: number | null;
  *   error: Error | null;
  *   event: import('types').RequestEvent;
  *   resolve_opts: import('types').RequiredResolveOptions;
@@ -128,7 +128,7 @@ export async function render_response({
 				error,
 				params: event.params,
 				routeId: event.routeId,
-				status,
+				status: status || 200,
 				stuff,
 				url: state.prerendering ? new PrerenderingURL(event.url) : event.url
 			},
@@ -321,7 +321,7 @@ export async function render_response({
 	}
 
 	return new Response(html, {
-		status,
+		status: status || 200,
 		headers
 	});
 }

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

@@ -36,7 +36,7 @@ export async function respond(opts) {
 				hydrate: true,
 				router: true
 			},
-			status: 200,
+			status: null,
 			error: null,
 			event,
 			stuff: {}
@@ -84,8 +84,8 @@ export async function respond(opts) {
 	/** @type {Array<Loaded>} */
 	let branch = [];
 
-	/** @type {number} */
-	let status = 200;
+	/** @type {number | null} */
+	let status = null;
 
 	/** @type {Error | null} */
 	let error = null;
@@ -105,10 +105,15 @@ export async function respond(opts) {
 			if (node) {
 				try {
 					loaded = await load_node({
-						...opts,
+						event,
+						options,
+						state,
+						route,
 						node,
+						$session,
 						stuff,
-						is_error: false,
+						status,
+						error,
 						is_leaf: i === nodes.length - 1
 					});
 
@@ -159,10 +164,13 @@ export async function respond(opts) {
 							try {
 								const error_loaded = /** @type {import('./types').Loaded} */ (
 									await load_node({
-										...opts,
+										event,
+										options,
+										state,
+										route,
+										$session,
 										node: error_node,
 										stuff: node_loaded.stuff,
-										is_error: true,
 										is_leaf: false,
 										status,
 										error

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

@@ -1,6 +1,7 @@
 import { render_response } from './render.js';
 import { load_node } from './load_node.js';
 import { coalesce_to_error } from '../../../utils/error.js';
+import { GENERIC_ERROR } from '../utils.js';
 
 /**
  * @typedef {import('./types.js').Loaded} Loaded
@@ -41,25 +42,29 @@ export async function respond_with_error({
 					event,
 					options,
 					state,
-					route: null,
+					route: GENERIC_ERROR,
 					node: default_layout,
 					$session,
 					stuff: {},
-					is_error: false,
+					status,
+					error,
 					is_leaf: false
 				})
 			);
 
+			if (layout_loaded.loaded.error) {
+				throw layout_loaded.loaded.error;
+			}
+
 			const error_loaded = /** @type {Loaded} */ (
 				await load_node({
 					event,
 					options,
 					state,
-					route: null,
+					route: GENERIC_ERROR,
 					node: default_error,
 					$session,
 					stuff: layout_loaded ? layout_loaded.stuff : {},
-					is_error: true,
 					is_leaf: false,
 					status,
 					error

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

@@ -96,3 +96,8 @@ function clone_error(error, get_stack) {
 
 	return object;
 }
+
+/** @type {import('types').SSRErrorPage} */
+export const GENERIC_ERROR = {
+	id: '__error'
+};

packages/kit/test/apps/basics/src/routes/__layout.svelte

@@ -1,8 +1,26 @@
 <script context="module">
 	/** @type {import('@sveltejs/kit').Load} */
-	export async function load() {
+	export async function load({ fetch, url, status }) {
+		if (url.pathname.startsWith('/errors/error-in-layout')) {
+			const res = await fetch('/errors/error-in-layout/non-existent');
+
+			return {
+				status: res.status
+			};
+		}
+
+		if (url.pathname === '/errors/status-in-layout' && !status) {
+			console.log('returning 500');
+
+			return {
+				status: 500,
+				error: new Error('status is accessible in __layout')
+			};
+		}
+
 		return {
 			props: {
+				status,
 				foo: {
 					bar: 'Custom layout'
 				}
@@ -34,10 +52,14 @@
 
 	/** @type {{ bar: string }} */
 	export let foo;
+
+	/** @type {number} */
+	export let status;
 </script>
 
 <slot />
 
+<p id="status">layout status: {status}</p>
 <footer>{foo.bar}</footer>
 
 <style>

packages/kit/test/apps/basics/src/routes/errors/error-in-layout/index.svelte

@@ -0,0 +1 @@
+<h1>hello</h1>

packages/kit/test/apps/basics/src/routes/errors/status-in-layout/index.svelte

@@ -0,0 +1,2 @@
+<!-- this file exists only so that /errors/status-in-layout is a valid route -->
+<h1>this should not be visible</h1>

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

@@ -1103,7 +1103,7 @@ test.describe('Errors', () => {
 		const response = await page.goto('/errors/load-status-without-error-server');
 
 		expect(await page.textContent('footer')).toBe('Custom layout');
-		expect(await page.textContent('#message')).toBe('This is your custom error page saying: ""');
+		expect(await page.textContent('#message')).toBe('This is your custom error page saying: "401"');
 		expect(/** @type {Response} */ (response).status()).toBe(401);
 	});
 
@@ -1112,7 +1112,9 @@ test.describe('Errors', () => {
 
 		if (javaScriptEnabled) {
 			expect(await page.textContent('footer')).toBe('Custom layout');
-			expect(await page.textContent('#message')).toBe('This is your custom error page saying: ""');
+			expect(await page.textContent('#message')).toBe(
+				'This is your custom error page saying: "401"'
+			);
 			expect(await page.innerHTML('h1')).toBe('401');
 		}
 	});
@@ -1301,6 +1303,13 @@ test.describe('Errors', () => {
 			expect(/** @type {Response} */ (response).status()).toBe(400);
 		}
 	});
+
+	test('status is accessible in __layout, if set', async ({ page }) => {
+		await page.goto('/errors/status-in-layout');
+		expect(await page.textContent('#message')).toBe(
+			'This is your custom error page saying: "status is accessible in __layout"'
+		);
+	});
 });
 
 test.describe('ETags', () => {
@@ -1623,6 +1632,13 @@ test.describe('Load', () => {
 		expect(cookies.doubled).toBe('84');
 	});
 
+	test('fetching a non-existent resource in root layout fails without hanging', async ({
+		request
+	}) => {
+		const response = await request.get('/errors/error-in-layout');
+		expect(await response.text()).toContain('Error: 500');
+	});
+
 	test('accessing url.hash from load errors and suggests using page store', async ({
 		page,
 		javaScriptEnabled

packages/kit/types/internal.d.ts

@@ -293,6 +293,10 @@ export interface SSRPage {
 	b: Array<number | undefined>;
 }
 
+export interface SSRErrorPage {
+	id: '__error';
+}
+
 export interface SSRPagePart {
 	id: string;
 	load: SSRComponentLoader;
@@ -303,7 +307,7 @@ export type SSRRoute = SSREndpoint | SSRPage;
 export interface SSRState {
 	fallback?: string;
 	getClientAddress: () => string;
-	initiator?: SSRPage | null;
+	initiator?: SSRPage | SSRErrorPage;
 	platform?: any;
 	prerendering?: PrerenderOptions;
 }