diff --git a/.changeset/tidy-ravens-smile.md b/.changeset/tidy-ravens-smile.md new file mode 100644 index 000000000000..7e19dbdb72f4 --- /dev/null +++ b/.changeset/tidy-ravens-smile.md @@ -0,0 +1,5 @@ +--- +'@sveltejs/kit': patch +--- + +fix: render the error page with the post-`handle` headers instead of failing on headers the crashed render already set 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 2e8cad97d229..1e905f0ffb7d 100644 --- a/packages/kit/src/runtime/server/page/respond_with_error.js +++ b/packages/kit/src/runtime/server/page/respond_with_error.js @@ -26,6 +26,8 @@ export async function respond_with_error({ event, state, error, resolve_opts }) return static_error_page(transformed.status, transformed.message); } + state.reset_headers?.(); + /** @type {import('./types.js').Fetched[]} */ const fetched = []; try { diff --git a/packages/kit/src/runtime/server/respond.js b/packages/kit/src/runtime/server/respond.js index b7e158717018..c7e6c5f3b1e8 100644 --- a/packages/kit/src/runtime/server/respond.js +++ b/packages/kit/src/runtime/server/respond.js @@ -175,7 +175,7 @@ export async function internal_respond(request, state) { } /** @type {Record} */ - const headers = {}; + let headers = {}; const { cookies, new_cookies, get_cookie_header, set_internal, set_trailing_slash } = get_cookies( request, @@ -583,6 +583,9 @@ export async function internal_respond(request, state) { * @param {import('@sveltejs/kit/hooks').ResolveOptions} [opts] */ async function resolve(event, page_nodes, opts) { + const handle_headers = { ...headers }; + state.reset_headers ??= () => (headers = { ...handle_headers }); + try { if (opts) { resolve_opts = { diff --git a/packages/kit/src/types/internal.d.ts b/packages/kit/src/types/internal.d.ts index ab935ae0938a..d798d5d573ae 100644 --- a/packages/kit/src/types/internal.d.ts +++ b/packages/kit/src/types/internal.d.ts @@ -696,6 +696,8 @@ export type RecordSpan = (options: { * used for tracking things like remote function calls */ export interface RequestState { + /** discards headers set by a render that failed, so the error page starts from the post-`handle` state */ + reset_headers?(): void; readonly getClientAddress: () => string; readonly platform?: any; /** @internal reads from the filesystem when user code tries to fetch a static asset */ diff --git a/packages/kit/test/apps/basics/src/hooks.server.js b/packages/kit/test/apps/basics/src/hooks.server.js index 2f35ad2d1e0b..b282fd23760d 100644 --- a/packages/kit/test/apps/basics/src/hooks.server.js +++ b/packages/kit/test/apps/basics/src/hooks.server.js @@ -138,7 +138,14 @@ export const handle = sequence( const response = await resolve(event, { transformPageChunk: event.url.pathname.startsWith('/transform-page-chunk') ? ({ html }) => html.replace('__REPLACEME__', 'Worked!') - : undefined + : event.url.pathname.startsWith('/errors/error-page-setheaders') + ? ({ html }) => { + if (html.includes('makes the page transform crash')) { + throw new Error('Crashing now'); + } + return html; + } + : undefined }); try { @@ -192,7 +199,10 @@ export const handle = sequence( return resolve(event); }, async ({ event, resolve }) => { - if (['/non-existent-route', '/non-existent-route-loop'].includes(event.url.pathname)) { + if ( + event.url.pathname.startsWith('/errors/error-page-setheaders') || + ['/non-existent-route', '/non-existent-route-loop'].includes(event.url.pathname) + ) { event.locals.url = new URL(event.request.url); } return resolve(event); diff --git a/packages/kit/test/apps/basics/src/routes/+layout.server.js b/packages/kit/test/apps/basics/src/routes/+layout.server.js index b61ba6c8ef37..a2fa9c4c31cf 100644 --- a/packages/kit/test/apps/basics/src/routes/+layout.server.js +++ b/packages/kit/test/apps/basics/src/routes/+layout.server.js @@ -7,7 +7,11 @@ if (JSON.parse(SOME_JSON).answer !== 42) { } /** @type {import('./$types').LayoutServerLoad} */ -export async function load({ cookies, locals, fetch }) { +export async function load({ cookies, locals, fetch, setHeaders }) { + if (locals.url?.pathname.startsWith('/errors/error-page-setheaders')) { + setHeaders({ 'cache-control': 'private, max-age=60' }); + } + if (locals.url?.pathname === '/non-existent-route') { await fetch('/prerendering/prerendered-endpoint/api').then((r) => r.json()); } diff --git a/packages/kit/test/apps/basics/src/routes/errors/error-page-setheaders-leak/+page.server.js b/packages/kit/test/apps/basics/src/routes/errors/error-page-setheaders-leak/+page.server.js new file mode 100644 index 000000000000..6a891c9a36e0 --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/errors/error-page-setheaders-leak/+page.server.js @@ -0,0 +1,4 @@ +/** @type {import('./$types').PageServerLoad} */ +export function load({ setHeaders }) { + setHeaders({ 'x-failed-render': '1' }); +} diff --git a/packages/kit/test/apps/basics/src/routes/errors/error-page-setheaders-leak/+page.svelte b/packages/kit/test/apps/basics/src/routes/errors/error-page-setheaders-leak/+page.svelte new file mode 100644 index 000000000000..d7d8816a86c5 --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/errors/error-page-setheaders-leak/+page.svelte @@ -0,0 +1 @@ +

this text makes the page transform crash

diff --git a/packages/kit/test/apps/basics/src/routes/errors/error-page-setheaders/+page.svelte b/packages/kit/test/apps/basics/src/routes/errors/error-page-setheaders/+page.svelte new file mode 100644 index 000000000000..d7d8816a86c5 --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/errors/error-page-setheaders/+page.svelte @@ -0,0 +1 @@ +

this text makes the page transform crash

diff --git a/packages/kit/test/apps/basics/test/server.test.js b/packages/kit/test/apps/basics/test/server.test.js index 8232717f4d8f..425e028dd8b0 100644 --- a/packages/kit/test/apps/basics/test/server.test.js +++ b/packages/kit/test/apps/basics/test/server.test.js @@ -1046,6 +1046,23 @@ test.describe('setHeaders', () => { expect(cookies).toMatch('cookie1=value1'); expect(cookies).toMatch('cookie2=value2'); }); + + test('renders the error page when the root layout sets headers', async ({ request }) => { + const response = await request.get('/errors/error-page-setheaders'); + + expect(response.status()).toBe(500); + expect(response.headers()['cache-control']).toBe('private, max-age=60'); + expect(await response.text()).toContain('This is your custom error page saying:'); + }); + + test('drops headers set by the failed render from the error page', async ({ request }) => { + const response = await request.get('/errors/error-page-setheaders-leak'); + + expect(response.status()).toBe(500); + expect(response.headers()['x-failed-render']).toBeUndefined(); + expect(response.headers()['cache-control']).toBe('private, max-age=60'); + expect(await response.text()).toContain('This is your custom error page saying:'); + }); }); test.describe('cookies', () => {