fix: don't invoke handleError for server-handled errors during preload

.changeset/preload-server-error-handling.md

@@ -0,0 +1,5 @@
+---
+'@sveltejs/kit': patch
+---
+
+fix: do not invoke client `handleError` for errors already handled on the server when the navigation is a preload

packages/kit/src/runtime/client/client.js

@@ -1330,14 +1330,7 @@ async function load_route({ id, invalidating, url, params, route, preload }) {
 					};
 				}
 
-				if (preload && preload_tokens.has(preload)) {
-					return preload_error({
-						error: await handle_error(err, { params, url, route: { id: route.id } }),
-						url,
-						params,
-						route
-					});
-				}
+				const is_preload = !!preload && preload_tokens.has(preload);
 
 				let status = get_status(err);
 				/** @type {App.Error} */
@@ -1351,17 +1344,23 @@ async function load_route({ id, invalidating, url, params, route, preload }) {
 				} else if (err instanceof HttpError) {
 					error = err.body;
 				} else {
-					// Referenced node could have been removed due to redeploy, check
-					const updated = await stores.updated.check();
-					if (updated) {
-						// Before reloading, try to update the service worker if it exists
-						await update_service_worker();
-						return await native_navigation(url);
+					if (!is_preload) {
+						// Referenced node could have been removed due to redeploy, check
+						const updated = await stores.updated.check();
+						if (updated) {
+							// Before reloading, try to update the service worker if it exists
+							await update_service_worker();
+							return await native_navigation(url);
+						}
 					}
 
 					error = await handle_error(err, { params, url, route: { id: route.id } });
 				}
 
+				if (is_preload) {
+					return preload_error({ error, url, params, route });
+				}
+
 				const error_load = await load_nearest_error_page(i, branch, errors);
 				if (error_load) {
 					return get_navigation_result_from_branch({

packages/kit/test/apps/basics/src/global.d.ts

@@ -5,6 +5,7 @@ declare global {
 		pageContext: any;
 		mounted: number;
 		fulfil_navigation: (value: any) => void;
+		handle_error_calls: Array<{ status: number; message: string }>;
 		promise: Promise<any>;
 		PUBLIC_DYNAMIC: string;
 	}

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

@@ -4,6 +4,10 @@ window.PUBLIC_DYNAMIC = env.PUBLIC_DYNAMIC;
 
 /** @type{import("@sveltejs/kit").HandleClientError} */
 export function handleError({ error, event, status, message }) {
+	(window.handle_error_calls ??= []).push({
+		status,
+		message: /** @type {Error} */ (error).message
+	});
 	return event.url.pathname.endsWith('404-fallback')
 		? undefined
 		: { message: `${/** @type {Error} */ (error).message} (${status} ${message})` };

packages/kit/test/apps/basics/src/routes/preloading/preload-error/+page.svelte

@@ -0,0 +1,16 @@
+<script>
+	import { preloadData } from '$app/navigation';
+
+	/** @type {Record<string, any> | null} */
+	let result = null;
+
+	async function preload() {
+		result = await preloadData('/preloading/preload-error/target');
+	}
+</script>
+
+<button on:click={preload}>Preload</button>
+
+{#if result}
+	<pre id="result">{JSON.stringify(result)}</pre>
+{/if}

packages/kit/test/apps/basics/src/routes/preloading/preload-error/target/+page.server.js

@@ -0,0 +1,5 @@
+import { error } from '@sveltejs/kit';
+
+export function load() {
+	error(404, 'Not found');
+}

packages/kit/test/apps/basics/src/routes/preloading/preload-error/target/+page.svelte

@@ -0,0 +1 @@
+<h1>this page never renders successfully</h1>

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

@@ -409,7 +409,7 @@
 		expect(read_errors('/no-ssr/ssr-page-config/layout/inherit')).toBe(undefined);
 	});
 
 	test('cannot use browser-only global on page because of ssr config in +page.js', async ({
 		page
 	}) => {
 		await page.goto('/no-ssr/ssr-page-config/layout/overwrite');
@@ -958,7 +958,7 @@
 		expect(responses.length).toEqual(1);
 	});
 
 	test('data-sveltekit-preload-data', async ({ page }) => {
 		/** @type {string[]} */
 		const requests = [];
 		page.on('request', (req) => {
@@ -1603,7 +1603,7 @@
 
 			const expectGoback = makeExpectGoback(testFinishPage, testEntryPage);
 
 			test('app.invalidate', async ({ app, page }) => {
 				await app.invalidate('app:goto');
 				await expectGoback(page);
 			});
@@ -1789,6 +1789,17 @@
 	});
 });
 
+test.describe('Preloading', () => {
+	test('does not invoke handleError for server load errors during preload', async ({ page }) => {
+		await page.goto('/preloading/preload-error');
+		await page.locator('button').click();
+		await expect(page.locator('#result')).toContainText('"type":"loaded"');
+
+		// the server already handled the error; the client hook must not run
+		expect(await page.evaluate(() => window.handle_error_calls)).toBeUndefined();
+	});
+});
+
 test.describe('reroute', () => {
 	test('Apply reroute during client side navigation', async ({ page, clicknav }) => {
 		await page.goto('/reroute/basic');
@@ -1887,7 +1898,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
 		/** @param {string} selector */
 		async function measureInteractionToPaint(selector) {