fix: prevent navigation when preloading fails due to network error

.changeset/good-roses-design.md

@@ -0,0 +1,5 @@
+---
+"@sveltejs/kit": patch
+---
+
+fix: prevent navigation when `data-sveltekit-preload-data` fails to fetch due to network error

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

@@ -172,7 +172,7 @@ const invalidated = [];
  */
 const components = [];
 
-/** @type {{id: string, promise: Promise<import('./types.js').NavigationResult>} | null} */
+/** @type {{id: string, token: {}, promise: Promise<import('./types.js').NavigationResult>} | null} */
 let load_cache = null;
 
 /** @type {Array<(navigation: import('@sveltejs/kit').BeforeNavigate) => void>} */
@@ -219,6 +219,14 @@ let page;
 /** @type {{}} */
 let token;
 
+/**
+ * A set of tokens which are associated to current preloads.
+ * If a preload becomes a real navigation, it's removed from the set.
+ * If a preload token is in the set and the preload errors, the error
+ * handling logic (for example reloading) is skipped.
+ */
+const preload_tokens = new Set();
+
 /** @type {Promise<void> | null} */
 let pending_invalidate;
 
@@ -375,16 +383,26 @@ async function _goto(url, options, redirect_count, nav_token) {
 
 /** @param {import('./types.js').NavigationIntent} intent */
 async function _preload_data(intent) {
-	load_cache = {
-		id: intent.id,
-		promise: load_route(intent).then((result) => {
-			if (result.type === 'loaded' && result.state.error) {
-				// Don't cache errors, because they might be transient
-				load_cache = null;
-			}
-			return result;
-		})
-	};
+	// Reuse the existing pending preload if it's for the same navigation.
+	// Prevents an edge case where same preload is triggered multiple times,
+	// then a later one is becoming the real navigation and the preload tokens
+	// get out of sync.
+	if (intent.id !== load_cache?.id) {
+		const preload = {};
+		preload_tokens.add(preload);
+		load_cache = {
+			id: intent.id,
+			token: preload,
+			promise: load_route({ ...intent, preload }).then((result) => {
+				preload_tokens.delete(preload);
+				if (result.type === 'loaded' && result.state.error) {
+					// Don't cache errors, because they might be transient
+					load_cache = null;
+				}
+				return result;
+			})
+		};
+	}
 
 	return load_cache.promise;
 }
@@ -803,11 +821,31 @@ function diff_search_params(old_url, new_url) {
 }
 
 /**
- * @param {import('./types.js').NavigationIntent} intent
+ * @param {Omit<import('./types.js').NavigationFinished['state'], 'branch'> & { error: App.Error }} opts
+ * @returns {import('./types.js').NavigationFinished}
+ */
+function preload_error({ error, url, route, params }) {
+	return {
+		type: 'loaded',
+		state: {
+			error,
+			url,
+			route,
+			params,
+			branch: []
+		},
+		props: { page, constructors: [] }
+	};
+}
+
+/**
+ * @param {import('./types.js').NavigationIntent & { preload?: {} }} intent
  * @returns {Promise<import('./types.js').NavigationResult>}
  */
-async function load_route({ id, invalidating, url, params, route }) {
+async function load_route({ id, invalidating, url, params, route, preload }) {
 	if (load_cache?.id === id) {
+		// the preload becomes the real navigation
+		preload_tokens.delete(load_cache.token);
 		return load_cache.promise;
 	}
 
@@ -855,9 +893,15 @@ async function load_route({ id, invalidating, url, params, route }) {
 		try {
 			server_data = await load_data(url, invalid_server_nodes);
 		} catch (error) {
+			const handled_error = await handle_error(error, { url, params, route: { id } });
+
+			if (preload_tokens.has(preload)) {
+				return preload_error({ error: handled_error, url, params, route });
+			}
+
 			return load_root_error_page({
 				status: get_status(error),
-				error: await handle_error(error, { url, params, route: { id: route.id } }),
+				error: handled_error,
 				url,
 				route
 			});
@@ -940,6 +984,15 @@ async function load_route({ id, invalidating, url, params, route }) {
 					};
 				}
 
+				if (preload_tokens.has(preload)) {
+					return preload_error({
+						error: await handle_error(err, { params, url, route: { id: route.id } }),
+						url,
+						params,
+						route
+					});
+				}
+
 				let status = get_status(err);
 				/** @type {App.Error} */
 				let error;
@@ -972,8 +1025,6 @@ async function load_route({ id, invalidating, url, params, route }) {
 						route
 					});
 				} else {
-					// if we get here, it's because the root `load` function failed,
-					// and we need to fall back to the server
 					return await server_fallback(url, { id: route.id }, error, status);
 				}
 			}

packages/kit/test/apps/basics/src/routes/data-sveltekit/preload-data/offline/+layout.server.js

@@ -0,0 +1,5 @@
+export function load({ url }) {
+	return {
+		url: url.toString()
+	};
+}

packages/kit/test/apps/basics/src/routes/data-sveltekit/preload-data/offline/+page.svelte

@@ -0,0 +1,8 @@
+<a id="one" href="/data-sveltekit/preload-data/offline/target" data-sveltekit-preload-data>target</a
+>
+
+<a
+	id="slow-navigation"
+	href="/data-sveltekit/preload-data/offline/slow-navigation"
+	data-sveltekit-preload-data>slow-navigation</a
+>

packages/kit/test/apps/basics/src/routes/data-sveltekit/preload-data/offline/slow-navigation/+page.server.js

@@ -0,0 +1,5 @@
+export async function load() {
+	return new Promise((resolve) => {
+		setTimeout(resolve, 1000);
+	});
+}

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

@@ -665,6 +665,78 @@ test.describe('data-sveltekit attributes', () => {
 		expect(requests.length).toBe(0);
 	});
 
+	test('data-sveltekit-preload-data network failure does not trigger navigation', async ({
+		page,
+		context,
+		browserName
+	}) => {
+		await page.goto('/data-sveltekit/preload-data/offline');
+
+		await context.setOffline(true);
+
+		await page.locator('#one').dispatchEvent('mousemove');
+		await Promise.all([
+			page.waitForTimeout(100), // wait for preloading to start
+			page.waitForLoadState('networkidle') // wait for preloading to finish
+		]);
+
+		let offline_url = /\/data-sveltekit\/preload-data\/offline/;
+		if (browserName === 'chromium') {
+			// it's chrome-error://chromewebdata/ on ubuntu but not on windows
+			offline_url = /chrome-error:\/\/chromewebdata\/|\/data-sveltekit\/preload-data\/offline/;
+		}
+		expect(page).toHaveURL(offline_url);
+	});
+
+	test('data-sveltekit-preload-data error does not block user navigation', async ({
+		page,
+		context,
+		browserName
+	}) => {
+		await page.goto('/data-sveltekit/preload-data/offline');
+
+		await context.setOffline(true);
+
+		await page.locator('#one').dispatchEvent('mousemove');
+		await Promise.all([
+			page.waitForTimeout(100), // wait for preloading to start
+			page.waitForLoadState('networkidle') // wait for preloading to finish
+		]);
+
+		expect(page).toHaveURL('/data-sveltekit/preload-data/offline');
+
+		await page.locator('#one').dispatchEvent('click');
+		await page.waitForTimeout(100); // wait for navigation to start
+		await page.waitForLoadState('networkidle');
+
+		let offline_url = /\/data-sveltekit\/preload-data\/offline/;
+		if (browserName === 'chromium') {
+			// it's chrome-error://chromewebdata/ on ubuntu but not on windows
+			offline_url = /chrome-error:\/\/chromewebdata\/|\/data-sveltekit\/preload-data\/offline/;
+		}
+		expect(page).toHaveURL(offline_url);
+	});
+
+	test('data-sveltekit-preload does not abort ongoing navigation', async ({
+		page,
+		browserName
+	}) => {
+		await page.goto('/data-sveltekit/preload-data/offline');
+
+		await page.locator('#slow-navigation').dispatchEvent('click');
+		await page.waitForTimeout(100); // wait for navigation to start
+		await page.locator('#slow-navigation').dispatchEvent('mousemove');
+		await Promise.all([
+			page.waitForTimeout(100), // wait for preloading to start
+			page.waitForLoadState('networkidle') // wait for preloading to finish
+		]);
+
+		expect(page).toHaveURL(
+			'/data-sveltekit/preload-data/offline/slow-navigation' ||
+				(browserName === 'chromium' && 'chrome-error://chromewebdata/')
+		);
+	});
+
 	test('data-sveltekit-reload', async ({ baseURL, page, clicknav }) => {
 		/** @type {string[]} */
 		const requests = [];