From 67e2342149847d267eb0c50809a1f93f41fa529b Mon Sep 17 00:00:00 2001
From: Rich Harris <richard.a.harris@gmail.com>
Date: Thu, 14 Jul 2022 18:52:06 -0400
Subject: [PATCH] handle non-200 status codes from root `__layout.svelte`
 (#4665)

* failing test for #4659

* prevent infinite loop when loading non-existent route in __layout

* update test

* tighten up

* changeset

* Update .changeset/purple-rats-wink.md

* wut
---
 .changeset/purple-rats-wink.md                         |  5 +++++
 packages/kit/src/runtime/load.js                       |  5 ++++-
 packages/kit/src/runtime/server/index.js               |  9 ++++++++-
 packages/kit/src/runtime/server/page/load_node.js      |  2 +-
 .../kit/src/runtime/server/page/respond_with_error.js  |  9 +++++++--
 packages/kit/src/runtime/server/utils.js               |  5 +++++
 .../kit/test/apps/basics/src/routes/__layout.svelte    | 10 +++++++++-
 .../src/routes/errors/error-in-layout/index.svelte     |  1 +
 packages/kit/test/apps/basics/test/client.test.js      |  2 +-
 packages/kit/test/apps/basics/test/server.test.js      |  9 +++++++++
 packages/kit/test/apps/basics/test/test.js             |  2 +-
 packages/kit/types/internal.d.ts                       |  6 +++++-
 12 files changed, 56 insertions(+), 9 deletions(-)
 create mode 100644 .changeset/purple-rats-wink.md
 create mode 100644 packages/kit/test/apps/basics/src/routes/errors/error-in-layout/index.svelte

diff --git a/.changeset/purple-rats-wink.md b/.changeset/purple-rats-wink.md
new file mode 100644
index 000000000000..1f9365720c5c
--- /dev/null
+++ b/.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
diff --git a/packages/kit/src/runtime/load.js b/packages/kit/src/runtime/load.js
index 5eaac5988d36..974ba0ff104a 100644
--- a/packages/kit/src/runtime/load.js
+++ b/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;
diff --git a/packages/kit/src/runtime/server/index.js b/packages/kit/src/runtime/server/index.js
index 9140a00018fb..16b90f07537e 100644
--- a/packages/kit/src/runtime/server/index.js
+++ b/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({
diff --git a/packages/kit/src/runtime/server/page/load_node.js b/packages/kit/src/runtime/server/page/load_node.js
index de6df8cd25d6..40789d1c790a 100644
--- a/packages/kit/src/runtime/server/page/load_node.js
+++ b/packages/kit/src/runtime/server/page/load_node.js
@@ -13,7 +13,7 @@ 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>;
diff --git a/packages/kit/src/runtime/server/page/respond_with_error.js b/packages/kit/src/runtime/server/page/respond_with_error.js
index cac5b4fc34b8..c3defa2d244e 100644
--- a/packages/kit/src/runtime/server/page/respond_with_error.js
+++ b/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,7 +42,7 @@ export async function respond_with_error({
 					event,
 					options,
 					state,
-					route: null,
+					route: GENERIC_ERROR,
 					node: default_layout,
 					$session,
 					stuff: {},
@@ -50,12 +51,16 @@ export async function respond_with_error({
 				})
 			);
 
+			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 : {},
diff --git a/packages/kit/src/runtime/server/utils.js b/packages/kit/src/runtime/server/utils.js
index de9da9cf660c..6e68070b54b5 100644
--- a/packages/kit/src/runtime/server/utils.js
+++ b/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'
+};
diff --git a/packages/kit/test/apps/basics/src/routes/__layout.svelte b/packages/kit/test/apps/basics/src/routes/__layout.svelte
index c5421be53848..1cb9805e3ec6 100644
--- a/packages/kit/test/apps/basics/src/routes/__layout.svelte
+++ b/packages/kit/test/apps/basics/src/routes/__layout.svelte
@@ -1,6 +1,14 @@
 <script context="module">
 	/** @type {import('@sveltejs/kit').Load} */
-	export async function load() {
+	export async function load({ fetch, url }) {
+		if (url.pathname.startsWith('/errors/error-in-layout')) {
+			const res = await fetch('/errors/error-in-layout/non-existent');
+
+			return {
+				status: res.status
+			};
+		}
+
 		return {
 			props: {
 				foo: {
diff --git a/packages/kit/test/apps/basics/src/routes/errors/error-in-layout/index.svelte b/packages/kit/test/apps/basics/src/routes/errors/error-in-layout/index.svelte
new file mode 100644
index 000000000000..6e7c1e6ac83f
--- /dev/null
+++ b/packages/kit/test/apps/basics/src/routes/errors/error-in-layout/index.svelte
@@ -0,0 +1 @@
+<h1>hello</h1>
diff --git a/packages/kit/test/apps/basics/test/client.test.js b/packages/kit/test/apps/basics/test/client.test.js
index b79aa6c0eeec..b64954822ca2 100644
--- a/packages/kit/test/apps/basics/test/client.test.js
+++ b/packages/kit/test/apps/basics/test/client.test.js
@@ -317,7 +317,7 @@ test.describe('Errors', () => {
 		await page.goto('/errors/load-status-without-error-client');
 
 		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');
 	});
 });
diff --git a/packages/kit/test/apps/basics/test/server.test.js b/packages/kit/test/apps/basics/test/server.test.js
index 2f9390ab4d27..3ac1ccc93ec8 100644
--- a/packages/kit/test/apps/basics/test/server.test.js
+++ b/packages/kit/test/apps/basics/test/server.test.js
@@ -356,6 +356,15 @@ test.describe('Errors', () => {
 	});
 });
 
+test.describe('Load', () => {
+	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.describe('Routing', () => {
 	test('event.params are available in handle', async ({ request }) => {
 		const response = await request.get('/routing/params-in-handle/banana');
diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js
index 44f8fe2f3e58..46ff3902d467 100644
--- a/packages/kit/test/apps/basics/test/test.js
+++ b/packages/kit/test/apps/basics/test/test.js
@@ -505,7 +505,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);
 	});
 
diff --git a/packages/kit/types/internal.d.ts b/packages/kit/types/internal.d.ts
index c45d0e9ea711..6316e457b2d5 100644
--- a/packages/kit/types/internal.d.ts
+++ b/packages/kit/types/internal.d.ts
@@ -290,6 +290,10 @@ export interface SSRPage {
 	b: Array<number | undefined>;
 }
 
+export interface SSRErrorPage {
+	id: '__error';
+}
+
 export interface SSRPagePart {
 	id: string;
 	load: SSRComponentLoader;
@@ -300,7 +304,7 @@ export type SSRRoute = SSREndpoint | SSRPage;
 export interface SSRState {
 	fallback?: string;
 	getClientAddress: () => string;
-	initiator?: SSRPage | null;
+	initiator?: SSRPage | SSRErrorPage;
 	platform?: any;
 	prerendering?: PrerenderOptions;
 }