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

[fix] render error page without root layout if that failed to load #6155

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all 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
5 changes: 5 additions & 0 deletions .changeset/silver-buses-suffer.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

[fix] render error page without root layout if that failed to load
21 changes: 13 additions & 8 deletions packages/kit/src/runtime/client/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -818,14 +818,19 @@ export function create_client({ target, base, trailing_slash }) {
/** @type {Record<string, string>} */
const params = {}; // error page does not have params

const root_layout = await load_node({
node: await default_layout,
url,
params,
routeId,
parent: () => Promise.resolve({}),
server_data: null // TODO!!!!!
});
let root_layout;
try {
root_layout = await load_node({
node: await default_layout,
url,
params,
routeId,
parent: () => Promise.resolve({}),
server_data: null // TODO!!!!!
});
} catch (e) {
// Root layout threw an error, we have to ignore it at this point
}

const root_error = {
node: await default_error,
Expand Down
78 changes: 44 additions & 34 deletions packages/kit/src/runtime/server/page/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -242,42 +242,52 @@ export async function render_page(event, route, options, state, resolve_opts) {

const status = error instanceof HttpError ? error.status : 500;

while (i--) {
if (route.errors[i]) {
const index = /** @type {number} */ (route.errors[i]);
const node = await options.manifest._.nodes[index]();

let j = i;
while (!branch[j]) j -= 1;

return await render_response({
event,
options,
state,
resolve_opts,
page_config: { router: true, hydrate: true },
status,
error,
branch: compact(branch.slice(0, j + 1)).concat({
node,
data: null,
server_data: null
}),
fetched,
cookies,
validation_errors: undefined
});
/**
* @param {number} idx
* @param {Array<Loaded>} branch
*/
const render_error = async (idx, branch) => {
const index = /** @type {number} */ (route.errors[idx]);
const node = await options.manifest._.nodes[index]();

return await render_response({
event,
options,
state,
resolve_opts,
page_config: { router: true, hydrate: true },
status,
error,
branch: branch.concat({
node,
data: null,
server_data: null
}),
fetched,
cookies,
validation_errors: undefined
});
};

if (i === 0) {
// if the error is in the root layout, we render the error page only
return await render_error(0, []);
} else {
while (i--) {
if (route.errors[i]) {
let j = i;
while (!branch[j]) j -= 1;

return await render_error(i, compact(branch.slice(0, j + 1)));
}
}
}

// 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 }
);
// It shouldn't be possible to get here since we always have a root error page
return new Response(
error instanceof HttpError ? error.message : options.get_stack(error),
{ status }
);
}
}
} else {
// push an empty slot so we can rewind past gaps to the
Expand Down
3 changes: 3 additions & 0 deletions packages/kit/test/apps/basics/src/routes/+layout.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ export async function load({ fetch, url }) {
const res = await fetch('/errors/error-in-layout/non-existent');
throw error(res.status);
}
if (url.searchParams.has('root-error')) {
throw error(500, 'Root layout error');
}

return {
foo: {
Expand Down
8 changes: 8 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,14 @@ test.describe('Errors', () => {
);
expect(await page.innerHTML('h1')).toBe('401');
});

test('error in root layout renders root error page without layout', async ({ page, app }) => {
await page.goto('/');
await app.goto('/?root-error');
expect(await page.textContent('p')).toBe(
'This is your custom error page saying: "Root layout error"'
);
});
});

test.describe('Load', () => {
Expand Down
7 changes: 7 additions & 0 deletions packages/kit/test/apps/basics/test/server.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,13 @@ test.describe('Errors', () => {

expect(await page.textContent('h1')).toBe('the answer is 42');
});

test('error in root layout renders root error page without layout', async ({ page }) => {
await page.goto('/?root-error');
expect(await page.textContent('p')).toBe(
'This is your custom error page saying: "Root layout error"'
);
});
});

test.describe('Load', () => {
Expand Down