diff --git a/.changeset/fresh-errors-render.md b/.changeset/fresh-errors-render.md new file mode 100644 index 000000000000..9c32fb68cac2 --- /dev/null +++ b/.changeset/fresh-errors-render.md @@ -0,0 +1,5 @@ +--- +'@sveltejs/kit': major +--- + +breaking: render the root `+error.svelte` page for `handle` and `+server.js` errors when the request expects HTML diff --git a/documentation/docs/20-core-concepts/10-routing.md b/documentation/docs/20-core-concepts/10-routing.md index 45bc5620a74e..84792689fbab 100644 --- a/documentation/docs/20-core-concepts/10-routing.md +++ b/documentation/docs/20-core-concepts/10-routing.md @@ -163,7 +163,7 @@ If the error occurs inside a `load` function in `+layout(.server).js`, the close If no route can be found (404), `src/routes/+error.svelte` (or the default error page, if that file does not exist) will be used. -> [!NOTE] `+error.svelte` is _not_ used when an error occurs inside [`handle`](hooks#handle) or a [+server.js](#server) request handler. +> [!NOTE] When an error occurs inside [`handle`](hooks#handle) or a [+server.js](#server) request handler, the root `+error.svelte` is rendered for document requests that accept HTML, rather than the closest error boundary. You can read more about error handling [here](errors). @@ -320,7 +320,7 @@ The first argument to `Response` can be a [`ReadableStream`](https://developer.m You can use the [`error`](@sveltejs-kit#error), [`redirect`](@sveltejs-kit#redirect) and [`json`](@sveltejs-kit#json) methods from `@sveltejs/kit` for convenience (but you don't have to). -If an error is thrown (either `error(...)` or an unexpected error), the response will be a JSON representation of the error or a fallback error page — which can be customised via `src/error.html` — depending on the `Accept` header. The [`+error.svelte`](#error) component will _not_ be rendered in this case. You can read more about error handling [here](errors). +If an error is thrown (either `error(...)` or an unexpected error), the root [`+error.svelte`](#error) component will be rendered for document requests that accept HTML, while requests that prefer JSON receive a JSON representation of the error. If the error page cannot be rendered, SvelteKit will use a fallback error page, which can be customised via `src/error.html`. You can read more about error handling [here](errors). > [!NOTE] When creating an `OPTIONS` handler, note that Vite will inject `Access-Control-Allow-Origin` and `Access-Control-Allow-Methods` headers — these will not be present in production unless you add them. diff --git a/documentation/docs/30-advanced/20-hooks.md b/documentation/docs/30-advanced/20-hooks.md index 6d1275d8665d..ade855e8f19b 100644 --- a/documentation/docs/30-advanced/20-hooks.md +++ b/documentation/docs/30-advanced/20-hooks.md @@ -61,7 +61,7 @@ export async function handle({ event, resolve }) { } ``` -Note that `resolve(...)` will never throw an error, it will always return a `Promise` with the appropriate status code. If an error is thrown elsewhere during `handle`, it is treated as fatal, and SvelteKit will respond with a JSON representation of the error or a fallback error page — which can be customised via `src/error.html` — depending on the `Accept` header. You can read more about error handling [here](errors). +Note that `resolve(...)` will never throw an error, it will always return a `Promise` with the appropriate status code. If an error is thrown elsewhere during `handle`, SvelteKit will render the root [`+error.svelte`](routing#error) component for document requests that accept HTML, while requests that prefer JSON receive a JSON representation of the error. If the error page cannot be rendered, SvelteKit will use a fallback error page, which can be customised via `src/error.html`. You can read more about error handling [here](errors). ### locals diff --git a/documentation/docs/30-advanced/25-errors.md b/documentation/docs/30-advanced/25-errors.md index fcb0f2daabeb..edd53ceab99e 100644 --- a/documentation/docs/30-advanced/25-errors.md +++ b/documentation/docs/30-advanced/25-errors.md @@ -122,7 +122,7 @@ Errors that occur during `load` or rendering (for example inside a component's ` ## Responses -If an error occurs inside `handle` or inside a [`+server.js`](routing#server) request handler, SvelteKit will respond with either a fallback error page or a JSON representation of the error object, depending on the request's `Accept` headers. +If an error occurs inside `handle` or inside a [`+server.js`](routing#server) request handler, SvelteKit will render the root [`+error.svelte`](routing#error) component for document requests that accept HTML, while requests that prefer JSON receive a JSON representation of the error object. If the error page cannot be rendered, SvelteKit will use the fallback error page. You can customise the fallback error page by adding a `src/error.html` file: diff --git a/packages/kit/src/runtime/server/respond.js b/packages/kit/src/runtime/server/respond.js index be9b86052bec..d7a291036fb3 100644 --- a/packages/kit/src/runtime/server/respond.js +++ b/packages/kit/src/runtime/server/respond.js @@ -40,6 +40,8 @@ import { server_data_serializer } from './page/data_serializer.js'; import { get_remote_id, handle_remote_call } from './remote-functions.js'; import { record_span } from '../telemetry/record_span.js'; import { otel } from '../telemetry/otel.js'; +import { get_status } from '../../utils/error.js'; +import { negotiate } from '../../utils/http.js'; /** @type {import('types').RequiredResolveOptions['transformPageChunk']} */ const default_transform = ({ html }) => html; @@ -296,6 +298,33 @@ export async function internal_respond(request, options, manifest, state) { preload: default_preload }; + /** @param {unknown} error */ + async function fatal_error(error) { + if ( + state.depth === 0 && + !state.prerendering && + !is_data_request && + !is_route_resolution_request && + !remote_id && + !non_html_fetch_destinations.has(request.headers.get('sec-fetch-dest') ?? '') && + negotiate(request.headers.get('accept') || 'text/html', ['application/json', 'text/html']) === + 'text/html' + ) { + return await respond_with_error({ + event, + event_state, + options, + manifest, + state, + status: get_status(error), + error, + resolve_opts + }); + } + + return await handle_fatal_error(event, event_state, options, error); + } + /** @type {import('types').TrailingSlash} */ let trailing_slash = 'never'; @@ -339,7 +368,7 @@ export async function internal_respond(request, options, manifest, state) { statusText: response.statusText }); } catch (error) { - return await handle_fatal_error(event, event_state, options, error); + return await fatal_error(error); } } @@ -379,7 +408,7 @@ export async function internal_respond(request, options, manifest, state) { event.params = result.params; } } catch (e) { - return await handle_fatal_error(event, event_state, options, e); + return await fatal_error(e); } } @@ -461,10 +490,10 @@ export async function internal_respond(request, options, manifest, state) { add_cookies_to_headers(response.headers, new_cookies.values()); return response; } catch (err) { - return await handle_fatal_error(event, event_state, options, err); + return await fatal_error(err); } } - return await handle_fatal_error(event, event_state, options, e); + return await fatal_error(e); } async function handle() { @@ -802,7 +831,7 @@ export async function internal_respond(request, options, manifest, state) { // and I don't even know how to describe it. need to investigate at some point // HttpError from endpoint can end up here - TODO should it be handled there instead? - return await handle_fatal_error(event, event_state, options, e); + return await fatal_error(e); } finally { event.cookies.set = () => { throw new Error('Cannot use `cookies.set(...)` after the response has been generated'); diff --git a/packages/kit/test/apps/basics/test/server.test.js b/packages/kit/test/apps/basics/test/server.test.js index 201b0c20d93f..66b160713264 100644 --- a/packages/kit/test/apps/basics/test/server.test.js +++ b/packages/kit/test/apps/basics/test/server.test.js @@ -591,9 +591,9 @@ test.describe('Errors', () => { expect(error).toBe(undefined); expect(res.status()).toBe(401); - expect(await res.text()).toContain( - 'This is the static error page with the following message: You shall not pass' - ); + const body = await res.text(); + expect(body).toContain('This is your custom error page saying:'); + expect(body).toContain('You shall not pass'); } // JSON (default) @@ -659,6 +659,21 @@ test.describe('Errors', () => { } }); + expect(res.status()).toBe(500); + const body = await res.text(); + expect(body).toContain('This is your custom error page saying:'); + expect(body).toContain('Error in handle (500 Internal Error)'); + } + + // HTML subresource + { + const res = await request.get('/errors/error-in-handle', { + headers: { + accept: 'text/html', + 'sec-fetch-dest': 'image' + } + }); + expect(res.status()).toBe(500); expect(await res.text()).toContain( 'This is the static error page with the following message: Error in handle' @@ -692,9 +707,9 @@ test.describe('Errors', () => { }); expect(res.status()).toBe(500); - expect(await res.text()).toContain( - 'This is the static error page with the following message: Expected error in handle' - ); + const body = await res.text(); + expect(body).toContain('This is your custom error page saying:'); + expect(body).toContain('Expected error in handle'); } // JSON (default) diff --git a/packages/kit/test/apps/writes/test/test.js b/packages/kit/test/apps/writes/test/test.js index 03dd3a7efecc..c9fc3af4725a 100644 --- a/packages/kit/test/apps/writes/test/test.js +++ b/packages/kit/test/apps/writes/test/test.js @@ -73,7 +73,8 @@ test.describe('Filesystem updates', () => { // than racing a fixed `waitForTimeout` against Vite's invalidation) await expect(async () => { await page.goto('/universal', { wait_for_started: false }); - await expect(page.locator('h1')).toHaveText('Internal Error', { timeout: 1000 }); + // the SSR failure now renders the default root error page, whose h1 is the status + await expect(page.locator('h1')).toHaveText('500', { timeout: 1000 }); }).toPass(); expect(await get_computed_style('body', 'background-color')).not.toBe('rgb(255, 0, 0)'); } finally { @@ -89,7 +90,7 @@ test.describe('Filesystem updates', () => { fs.writeFileSync(file, contents.replace(/export const ssr = .*;/, 'export const ssr = !1;')); await page.waitForTimeout(500); // this is the rare time we actually need waitForTimeout; we have no visibility into whether the module graph has been invalidated expect(await get_computed_style('body', 'background-color')).not.toBe('rgb(255, 0, 0)'); - await expect(page.locator('h1')).toHaveText('Internal Error'); + await expect(page.locator('h1')).toHaveText('500'); } finally { fs.writeFileSync(file, contents.replace(/\\nexport const ssr = false;\\n/, '')); } @@ -114,7 +115,7 @@ test.describe('Filesystem updates', () => { fs.writeFileSync(file, contents.replace(/export const ssr = false;/, '')); await page.waitForTimeout(500); // this is the rare time we actually need waitForTimeout; we have no visibility into whether the module graph has been invalidated expect(await get_computed_style('body', 'background-color')).not.toBe('rgb(255, 0, 0)'); - await expect(page.locator('h1')).toHaveText('Internal Error'); + await expect(page.locator('h1')).toHaveText('500'); } finally { fs.writeFileSync(file, contents); }