fix: prevent endless SPA 404 loop (#11354)

sveltejs · Dec 18, 2023 · ca5d5ec · ca5d5ec
1 parent 87da73e
commit ca5d5ec
Show file tree

Hide file tree

Showing 17 changed files with 167 additions and 28 deletions.
diff --git a/.changeset/angry-lamps-deny.md b/.changeset/angry-lamps-deny.md
@@ -0,0 +1,5 @@
+---
+"@sveltejs/kit": patch
+---
+
+fix: prevent endless SPA 404 loop
diff --git a/packages/kit/src/runtime/client/client.js b/packages/kit/src/runtime/client/client.js
@@ -2118,16 +2118,21 @@ async function load_data(url, invalid) {
 
 	const res = await native_fetch(data_url.href);
 
-	// if `__data.json` doesn't exist or the server has an internal error,
-	// fallback to native navigation so we avoid parsing the HTML error page as a JSON
-	if (res.headers.get('content-type')?.includes('text/html')) {
-		await native_navigation(url);
-	}
-
 	if (!res.ok) {
 		// error message is a JSON-stringified string which devalue can't handle at the top level
 		// turn it into a HttpError to not call handleError on the client again (was already handled on the server)
-		throw new HttpError(res.status, await res.json());
+		// if `__data.json` doesn't exist or the server has an internal error,
+		// avoid parsing the HTML error page as a JSON
+		/** @type {string | undefined} */
+		let message;
+		if (res.headers.get('content-type')?.includes('application/json')) {
+			message = await res.json();
+		} else if (res.status === 404) {
+			message = 'Not Found';
+		} else if (res.status === 500) {
+			message = 'Internal Error';
+		}
+		throw new HttpError(res.status, message);
 	}
 
 	// TODO: fix eslint error / figure out if it actually applies to our situation

diff --git a/packages/kit/src/runtime/control.js b/packages/kit/src/runtime/control.js
@@ -33,6 +33,7 @@ export class Redirect {
 /**
  * An error that was thrown from within the SvelteKit runtime that is not fatal and doesn't result in a 500, such as a 404.
  * `SvelteKitError` goes through `handleError`.
+ * @extends Error
  */
 export class SvelteKitError extends Error {
 	/**

diff --git a/packages/kit/src/runtime/server/page/respond_with_error.js b/packages/kit/src/runtime/server/page/respond_with_error.js
@@ -86,7 +86,7 @@ export async function respond_with_error({
 			state,
 			page_config: {
 				ssr,
-				csr: get_option([default_layout], 'csr') ?? true
+				csr
 			},
 			status,
 			error: await handle_error_and_jsonify(event, options, error),

diff --git a/packages/kit/test/apps/no-ssr/package.json b/packages/kit/test/apps/no-ssr/package.json
@@ -0,0 +1,24 @@
+{
+	"name": "test-no-ssr",
+	"private": true,
+	"version": "0.0.1",
+	"scripts": {
+		"dev": "vite dev",
+		"build": "vite build",
+		"preview": "vite preview",
+		"check": "svelte-kit sync && tsc && svelte-check",
+		"test": "pnpm test:dev && pnpm test:build",
+		"test:dev": "cross-env DEV=true playwright test",
+		"test:build": "playwright test"
+	},
+	"devDependencies": {
+		"@sveltejs/kit": "workspace:^",
+		"@sveltejs/vite-plugin-svelte": "^3.0.1",
+		"cross-env": "^7.0.3",
+		"svelte": "^4.2.8",
+		"svelte-check": "^3.6.2",
+		"typescript": "^5.3.3",
+		"vite": "^5.0.8"
+	},
+	"type": "module"
+}
diff --git a/packages/kit/test/apps/no-ssr/playwright.config.js b/packages/kit/test/apps/no-ssr/playwright.config.js
@@ -0,0 +1 @@
+export { config as default } from '../../utils.js';
diff --git a/packages/kit/test/apps/no-ssr/src/app.html b/packages/kit/test/apps/no-ssr/src/app.html
@@ -0,0 +1,12 @@
+<!doctype html>
+<html lang="en">
+	<head>
+		<meta charset="utf-8" />
+		<meta name="viewport" content="width=device-width, initial-scale=1" />
+		<link rel="icon" type="image/png" href="%sveltekit.assets%/favicon.png" />
+		%sveltekit.head%
+	</head>
+	<body>
+		<div style="display: contents">%sveltekit.body%</div>
+	</body>
+</html>
diff --git a/packages/kit/test/apps/no-ssr/src/routes/+layout.js b/packages/kit/test/apps/no-ssr/src/routes/+layout.js
@@ -0,0 +1 @@
+export const ssr = false;
diff --git a/packages/kit/test/apps/no-ssr/src/routes/+layout.server.js b/packages/kit/test/apps/no-ssr/src/routes/+layout.server.js
diff --git a/packages/kit/test/apps/no-ssr/src/routes/+layout.svelte b/packages/kit/test/apps/no-ssr/src/routes/+layout.svelte
@@ -0,0 +1,7 @@
+<script>
+	import { setup } from '../../../../setup.js';
+
+	setup();
+</script>
+
+<slot />
diff --git a/packages/kit/test/apps/no-ssr/src/routes/+page.svelte b/packages/kit/test/apps/no-ssr/src/routes/+page.svelte
diff --git a/packages/kit/test/apps/no-ssr/static/favicon.png b/packages/kit/test/apps/no-ssr/static/favicon.png
diff --git a/packages/kit/test/apps/no-ssr/svelte.config.js b/packages/kit/test/apps/no-ssr/svelte.config.js
@@ -0,0 +1,6 @@
+/** @type {import('@sveltejs/kit').Config} */
+const config = {
+	kit: {}
+};
+
+export default config;
diff --git a/packages/kit/test/apps/no-ssr/test/test.js b/packages/kit/test/apps/no-ssr/test/test.js
@@ -0,0 +1,15 @@
+import { expect } from '@playwright/test';
+import { test } from '../../../utils.js';
+
+/** @typedef {import('@playwright/test').Response} Response */
+
+test.skip(({ javaScriptEnabled }) => !javaScriptEnabled);
+
+test.describe.configure({ mode: 'parallel' });
+
+test('navigating to a non-existent route renders the default error page', async ({ page }) => {
+	test.setTimeout(3000);
+	await page.goto('/non-existent-route');
+	await page.waitForLoadState('networkidle');
+	expect(await page.textContent('h1')).toBe('404');
+});
diff --git a/packages/kit/test/apps/no-ssr/tsconfig.json b/packages/kit/test/apps/no-ssr/tsconfig.json
@@ -0,0 +1,14 @@
+{
+	"compilerOptions": {
+		"allowJs": true,
+		"checkJs": true,
+		"esModuleInterop": true,
+		"noEmit": true,
+		"paths": {
+			"@sveltejs/kit": ["../../../types"],
+			"types": ["../../../types/internal"]
+		},
+		"resolveJsonModule": true
+	},
+	"extends": "./.svelte-kit/tsconfig.json"
+}
diff --git a/packages/kit/test/apps/no-ssr/vite.config.js b/packages/kit/test/apps/no-ssr/vite.config.js
@@ -0,0 +1,18 @@
+import * as path from 'node:path';
+import { sveltekit } from '@sveltejs/kit/vite';
+
+/** @type {import('vite').UserConfig} */
+const config = {
+	build: {
+		minify: false
+	},
+	clearScreen: false,
+	plugins: [sveltekit()],
+	server: {
+		fs: {
+			allow: [path.resolve('../../../src')]
+		}
+	}
+};
+
+export default config;
diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml
-Original file line number
+Diff line change
@@ Expand Up / @@ -86,7 +86,7 @@ export async function respond_with_error({ @@
     			state,
     			page_config: {
     				ssr,
-    				csr: get_option([default_layout], 'csr') ?? true
+    				csr
     			},
     			status,
     			error: await handle_error_and_jsonify(event, options, error),
@@ Expand Down @@
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		export { config as default } from '../../utils.js';