fix: propagate errors from prerendered remote responses instead of re-running the function

.changeset/remote-prerender-error-propagation.md

@@ -0,0 +1,5 @@
+---
+'@sveltejs/kit': patch
+---
+
+fix: propagate errors from prerendered remote responses instead of re-running the function

packages/kit/src/runtime/app/server/remote/prerender.js

@@ -1,7 +1,8 @@
 /** @import { RemoteResource, RemotePrerenderFunction } from '@sveltejs/kit' */
 /** @import { RemoteFunctionResponse, RemotePrerenderInputsGenerator, RemotePrerenderInternals, MaybePromise } from 'types' */
 /** @import { StandardSchemaV1 } from '@standard-schema/spec' */
-import { json, error } from '@sveltejs/kit';
+import { json } from '@sveltejs/kit';
+import { HttpError } from '@sveltejs/kit/internal';
 import { get_request_store } from '@sveltejs/kit/internal/server';
 import { stringify, stringify_remote_arg } from '../../../shared.js';
 import { noop } from '../../../../utils/functions.js';
@@ -99,25 +100,28 @@ export function prerender(validate_or_fn, fn_or_options, maybe_options) {
 			const url = `${base}/${app_dir}/remote/${id}${payload ? `/${payload}` : ''}`;
 
 			if (!state.prerendering && !__SVELTEKIT_DEV__ && !event.isRemoteRequest) {
+				/** @type {RemoteFunctionResponse | undefined} */
+				let prerendered;
+
 				try {
 					// TODO adapters can provide prerendered data more efficiently than
 					// fetching from the public internet
 					// `request.url` rather than `event.url`, which throws inside queries
 					const response = await fetch(new URL(url, event.request.url).href);
 
-					if (!response.ok) {
-						throw new Error('Prerendered response not found');
+					if (response.ok) {
+						prerendered = /** @type {RemoteFunctionResponse} */ (await response.json());
 					}
+				} catch {
+					// not available prerendered, fallback to normal function
+				}
 
-					const prerendered = /** @type {RemoteFunctionResponse} */ (await response.json());
-
+				if (prerendered) {
 					if (prerendered.type === 'error') {
-						error(prerendered.error.status, prerendered.error);
+						throw new HttpError(prerendered.error);
 					}
 
 					return parse_remote_response(prerendered.data, state.transport)._;
-				} catch {
-					// not available prerendered, fallback to normal function
 				}
 			}
 

packages/kit/src/runtime/app/server/remote/prerender.spec.js

@@ -0,0 +1,100 @@
+/** @import { RequestEvent } from '@sveltejs/kit' */
+/** @import { RequestState } from 'types' */
+import { expect, test, vi } from 'vitest';
+import { HttpError } from '@sveltejs/kit/internal';
+import { prerender } from './prerender.js';
+import { stringify } from '../../../shared.js';
+
+const store = vi.hoisted(() => ({ current: /** @type {any} */ (null) }));
+
+vi.mock(import('@sveltejs/kit/internal/server'), async (actualPromise) => {
+	const actual = await actualPromise();
+	return {
+		...actual,
+		get_request_store: () => store.current
+	};
+});
+
+vi.stubGlobal('__SVELTEKIT_DEV__', false);
+
+/**
+ * Creates a prerender function whose self-fetch of the prerendered response
+ * resolves as specified, mimicking the production SSR path
+ * @param {() => Response | Promise<Response>} fetch_impl
+ */
+function setup(fetch_impl) {
+	const fn = vi.fn(() => 'from function');
+	const wrapper = prerender(fn);
+	/** @type {any} */ (wrapper).__.id = 'hash/fn';
+
+	store.current = {
+		event: /** @type {RequestEvent} */ (
+			/** @type {unknown} */ ({
+				request: { url: 'http://localhost/' },
+				isRemoteRequest: false,
+				cookies: {}
+			})
+		),
+		state: /** @type {RequestState} */ (
+			/** @type {unknown} */ ({
+				transport: {},
+				remote: {},
+				prerendering: undefined,
+				is_in_remote_query: false
+			})
+		)
+	};
+
+	vi.stubGlobal('fetch', vi.fn(fetch_impl));
+
+	return { fn, wrapper };
+}
+
+test('propagates an error response instead of running the function', async () => {
+	const { fn, wrapper } = setup(
+		() =>
+			new Response(JSON.stringify({ type: 'error', error: { status: 418, message: 'teapot' } }), {
+				status: 200
+			})
+	);
+
+	const rejection = await wrapper().catch((e) => e);
+
+	expect(rejection).toBeInstanceOf(HttpError);
+	expect(rejection.status).toBe(418);
+	expect(rejection.body).toEqual({ status: 418, message: 'teapot' });
+	expect(fn).not.toHaveBeenCalled();
+});
+
+test('parses a prerendered result without running the function', async () => {
+	const { fn, wrapper } = setup(
+		() =>
+			new Response(JSON.stringify({ type: 'result', data: stringify({ _: 'prerendered' }, {}) }), {
+				status: 200
+			})
+	);
+
+	await expect(wrapper()).resolves.toBe('prerendered');
+	expect(fn).not.toHaveBeenCalled();
+});
+
+test('falls back to the function when the fetch rejects', async () => {
+	const { fn, wrapper } = setup(() => Promise.reject(new Error('connection refused')));
+
+	await expect(wrapper()).resolves.toBe('from function');
+	expect(fn).toHaveBeenCalledOnce();
+});
+
+test('falls back to the function on a non-ok response', async () => {
+	const { fn, wrapper } = setup(() => new Response('not found', { status: 404 }));
+
+	await expect(wrapper()).resolves.toBe('from function');
+	expect(fn).toHaveBeenCalledOnce();
+});
+
+test('falls back to the function on a non-JSON response', async () => {
+	const { fn, wrapper } = setup(() => new Response('<html></html>', { status: 200 }));
+
+	await expect(wrapper()).resolves.toBe('from function');
+	expect(fn).toHaveBeenCalledOnce();
+});