Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[breaking] add error.html #6367

Merged
merged 18 commits into from
Aug 30, 2022
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/quiet-camels-shop.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@sveltejs/kit': patch
'create-svelte': patch
---

[breaking] add error.html page
2 changes: 2 additions & 0 deletions documentation/docs/01-project-structure.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ my-project/
│ ├ routes/
│ │ └ [your routes]
│ ├ app.html
│ ├ error.html
│ └ hooks.js
├ static/
│ └ [your static assets]
Expand Down Expand Up @@ -41,6 +42,7 @@ The `src` directory contains the meat of your project.
- `%sveltekit.body%` — the markup for a rendered page. Typically this lives inside a `<div>` or other element, rather than directly inside `<body>`, to prevent bugs caused by browser extensions injecting elements that are then destroyed by the hydration process
- `%sveltekit.assets%` — either [`paths.assets`](/docs/configuration#paths), if specified, or a relative path to [`paths.base`](/docs/configuration#base)
- `%sveltekit.nonce%` — a [CSP](/docs/configuration#csp) nonce for manually included links and scripts, if used
- `error.html` is the page that is rendered when everything else fails
- `hooks.js` (optional) contains your application's [hooks](/docs/hooks)
- `service-worker.js` (optional) contains your [service worker](/docs/service-workers)

Expand Down
2 changes: 1 addition & 1 deletion documentation/docs/03-routing.md
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ If an error occurs during `load`, SvelteKit will render a default error page. Yo
<h1>{$page.status}: {$page.error.message}</h1>
```

SvelteKit will 'walk up the tree' looking for the closest error boundary — if the file above didn't exist it would try `src/routes/blog/+error.svelte` and `src/routes/+error.svelte` before rendering the default error page.
SvelteKit will 'walk up the tree' looking for the closest error boundary — if the file above didn't exist it would try `src/routes/blog/+error.svelte` and `src/routes/+error.svelte` before rendering the default error page. If that fails, the contents of a static error page (`src/error.html` by default) will be rendered instead.
Rich-Harris marked this conversation as resolved.
Show resolved Hide resolved

### +layout

Expand Down
3 changes: 2 additions & 1 deletion documentation/docs/15-configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ const config = {
params: 'src/params',
routes: 'src/routes',
serviceWorker: 'src/service-worker',
template: 'src/app.html'
template: 'src/app.html',
errorPage: 'src/error.html'
dummdidumm marked this conversation as resolved.
Show resolved Hide resolved
},
inlineStyleThreshold: 0,
methodOverride: {
Expand Down
11 changes: 11 additions & 0 deletions packages/create-svelte/templates/default/src/error.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<!DOCTYPE html>
dummdidumm marked this conversation as resolved.
Show resolved Hide resolved
<html lang="en">
<head>
<meta charset="utf-8" />
<title>Error</title>
</head>
<body>
<h1>Error</h1>
<p>An error occurred.</p>
</body>
</html>
11 changes: 11 additions & 0 deletions packages/create-svelte/templates/libskeleton/src/error.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="utf-8" />
<title>Error</title>
</head>
<body>
<h1>Error</h1>
<p>An error occurred.</p>
</body>
</html>
11 changes: 11 additions & 0 deletions packages/create-svelte/templates/skeleton/src/error.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="utf-8" />
<title>Error</title>
</head>
<body>
<h1>Error</h1>
<p>An error occurred.</p>
</body>
</html>
28 changes: 28 additions & 0 deletions packages/kit/src/core/config/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,34 @@ export function load_template(cwd, config) {
return fs.readFileSync(template, 'utf-8');
}

/**
* Loads the error page (src/error.html by default) if it exists.
* Falls back to a generic error page content.
* @param {string} cwd
* @param {import('types').ValidatedConfig} config
*/
export function load_error_page(cwd, config) {
const { errorPage } = config.kit.files;
const relative = path.relative(cwd, errorPage);

if (fs.existsSync(errorPage)) {
return fs.readFileSync(errorPage, 'utf-8');
} else {
console.log(`${relative} does not exist. Using generic error page instead.`);
dummdidumm marked this conversation as resolved.
Show resolved Hide resolved
return `<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="utf-8">
<title>Error</title>
</head>
<body>
<h1>Error</h1>
<p>An error occurred.</p>
</body>
</html>`;
}
}

/**
* Loads and validates svelte.config.js
* @param {{ cwd?: string }} options
Expand Down
3 changes: 2 additions & 1 deletion packages/kit/src/core/config/options.js
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,8 @@ const options = object(
params: string(join('src', 'params')),
routes: string(join('src', 'routes')),
serviceWorker: string(join('src', 'service-worker')),
template: string(join('src', 'app.html'))
template: string(join('src', 'app.html')),
errorPage: string(join('src', 'error.html'))
}),

// TODO: remove this for the 1.0 release
Expand Down
9 changes: 6 additions & 3 deletions packages/kit/src/exports/vite/build/build_server.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import fs from 'fs';
import path from 'path';
import { mkdirp, posixify } from '../../../utils/filesystem.js';
import { get_vite_config, merge_vite_configs, resolve_entry } from '../utils.js';
import { load_template } from '../../../core/config/index.js';
import { load_error_page, load_template } from '../../../core/config/index.js';
import { runtime_directory } from '../../../core/utils.js';
import { create_build, find_deps, get_default_build_config, is_http_method } from './utils.js';
import { s } from '../../../utils/misc.js';
Expand All @@ -14,9 +14,10 @@ import { s } from '../../../utils/misc.js';
* has_service_worker: boolean;
* runtime: string;
* template: string;
* error_page: string;
* }} opts
*/
const server_template = ({ config, hooks, has_service_worker, runtime, template }) => `
const server_template = ({ config, hooks, has_service_worker, runtime, template, error_page }) => `
import root from '__GENERATED__/root.svelte';
import { respond } from '${runtime}/server/index.js';
import { set_paths, assets, base } from '${runtime}/paths.js';
Expand Down Expand Up @@ -80,6 +81,7 @@ export class Server {
router: ${s(config.kit.browser.router)},
template,
template_contains_nonce: ${template.includes('%sveltekit.nonce%')},
error_page: '${error_page}',
trailing_slash: ${s(config.kit.trailingSlash)}
};
}
Expand Down Expand Up @@ -205,7 +207,8 @@ export async function build_server(options, client) {
hooks: app_relative(hooks_file),
has_service_worker: config.kit.serviceWorker.register && !!service_worker_entry_file,
runtime: posixify(path.relative(build_dir, runtime_directory)),
template: load_template(cwd, config)
template: load_template(cwd, config),
error_page: load_error_page(cwd, config)
})
);

Expand Down
3 changes: 2 additions & 1 deletion packages/kit/src/exports/vite/dev/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { getRequest, setResponse } from '../../../exports/node/index.js';
import { installPolyfills } from '../../../exports/node/polyfills.js';
import { coalesce_to_error } from '../../../utils/error.js';
import { posixify } from '../../../utils/filesystem.js';
import { load_template } from '../../../core/config/index.js';
import { load_error_page, load_template } from '../../../core/config/index.js';
import { SVELTE_KIT_ASSETS } from '../../../core/constants.js';
import * as sync from '../../../core/sync/sync.js';
import { get_mime_lookup, runtime_base, runtime_prefix } from '../../../core/utils.js';
Expand Down Expand Up @@ -427,6 +427,7 @@ export async function dev(vite, vite_config, svelte_config, illegal_imports) {
);
},
template_contains_nonce: template.includes('%sveltekit.nonce%'),
error_page: load_error_page(cwd, svelte_config),
trailing_slash: svelte_config.kit.trailingSlash
},
{
Expand Down
6 changes: 3 additions & 3 deletions packages/kit/src/runtime/client/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -731,7 +731,7 @@ export function create_client({ target, base, trailing_slash }) {
}
} catch (e) {
// something went catastrophically wrong — bail and defer to the server
native_navigation(url);
await native_navigation(url);
return;
}

Expand Down Expand Up @@ -841,7 +841,7 @@ export function create_client({ target, base, trailing_slash }) {

// if we get here, it's because the root `load` function failed,
// and we need to fall back to the server
native_navigation(url);
await native_navigation(url);
return;
}
} else {
Expand Down Expand Up @@ -896,7 +896,7 @@ export function create_client({ target, base, trailing_slash }) {

if (!res.ok || server_data_nodes?.type !== 'data') {
// at this point we have no choice but to fall back to the server
native_navigation(url);
await native_navigation(url);
Rich-Harris marked this conversation as resolved.
Show resolved Hide resolved

// @ts-expect-error
return;
Expand Down
5 changes: 2 additions & 3 deletions packages/kit/src/runtime/server/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -482,9 +482,8 @@ export async function respond(request, options, state) {
resolve_opts
});
} catch (/** @type {unknown} */ e) {
const error = coalesce_to_error(e);

return new Response(options.dev ? error.stack : error.message, {
return new Response(options.error_page, {
headers: { 'content-type': 'text/html; charset=utf-8' },
status: 500
});
}
Expand Down
12 changes: 5 additions & 7 deletions packages/kit/src/runtime/server/page/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -278,13 +278,11 @@ export async function render_page(event, route, page, options, state, resolve_op
}

// if we're still here, it means the error happened in the root layout,
// which means we have to fall back to a plain text response
// TODO since the requester is expecting HTML, maybe it makes sense to
// doll this up a bit
return new Response(
error instanceof HttpError ? error.message : options.get_stack(error),
{ status }
);
// which means we have to fall back to error.html
return new Response(options.error_page, {
status,
headers: { 'content-type': 'text/html; charset=utf-8' }
});
}
} else {
// push an empty slot so we can rewind past gaps to the
Expand Down
3 changes: 2 additions & 1 deletion packages/kit/src/runtime/server/page/respond_with_error.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,8 @@ export async function respond_with_error({ event, options, state, status, error,

options.handle_error(error, event);

return new Response(error.stack, {
return new Response(options.error_page, {
headers: { 'content-type': 'text/html; charset=utf-8' },
status: 500
});
}
Expand Down
10 changes: 10 additions & 0 deletions packages/kit/test/apps/basics/src/error.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="utf-8" />
<title>Error</title>
</head>
<body>
<p>This is the static error page</p>
</body>
</html>
12 changes: 12 additions & 0 deletions packages/kit/test/apps/basics/src/routes/+layout.server.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,16 @@
let should_fail = false;
/**
* @param {boolean} value
*/
export function set_should_fail(value) {
should_fail = value;
}

export async function load() {
if (should_fail) {
set_should_fail(false);
throw new Error('Failed to load');
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is somewhat nasty, but the idea could also be reused to test the invalidate() bug I fixed

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from the looks of it this could cause raciness if tests are running in parallel. not suggesting we change anything, just noting it so we know where to look if that starts happening

}
// Do NOT make this load function depend on something which would cause it to rerun
return {
rootlayout: 'rootlayout'
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<script>
async function fail() {
await fetch('./error-html/make-root-fail');
location.reload();
}
</script>

<button on:click={fail}>Let's fail catastrophically</button>
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { set_should_fail } from '../../../+layout.server';

export function GET() {
set_should_fail(true);
return new Response();
}
6 changes: 6 additions & 0 deletions packages/kit/test/apps/basics/test/client.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,12 @@ test.describe('Errors', () => {
);
expect(await page.innerHTML('h1')).toBe('401');
});

test('Root error falls back to error.html', async ({ page }) => {
await page.goto('/errors/error-html');
await page.click('button');
expect(await page.textContent('p')).toBe('This is the static error page');
});
});

test.describe('Load', () => {
Expand Down
1 change: 1 addition & 0 deletions packages/kit/types/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ export interface KitConfig {
routes?: string;
serviceWorker?: string;
template?: string;
errorPage?: string;
};
inlineStyleThreshold?: number;
methodOverride?: {
Expand Down
1 change: 1 addition & 0 deletions packages/kit/types/internal.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,7 @@ export interface SSROptions {
nonce: string;
}): string;
template_contains_nonce: boolean;
error_page: string;
trailing_slash: TrailingSlash;
}

Expand Down