Skip to content

Commit

Permalink
Better error logging (#1062)
Browse files Browse the repository at this point in the history 
* tidy up render options, show parent of erroring page in prerender

* separate out render options from render state

* small tidy up

* fix

* always log caught errors when rendering

* changeset

* fix test, apply stack fix to non-thrown errors returned from load

* squelch console errors, now that we generate a bunch more of them
  • Loading branch information
Rich Harris authored Apr 17, 2021
1 parent 80f5b76 commit 61d7fa0
Show file tree
Hide file tree
Showing 15 changed files with 294 additions and 216 deletions.
5 changes: 5 additions & 0 deletions .changeset/lazy-actors-add.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

Better error logging
45 changes: 25 additions & 20 deletions packages/kit/src/core/adapt/prerender.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,16 +66,17 @@ export async function prerender({ cwd, out, log, config, build_data, force }) {

app.init({
paths: config.kit.paths,
prerendering: true
prerendering: true,
read: (file) => readFileSync(join(config.kit.files.assets, file))
});

/** @type {(status: number, path: string) => void} */
/** @type {(status: number, path: string, parent: string, verb: string) => void} */
const error = config.kit.prerender.force
? (status, path) => {
log.error(`${status} ${path}`);
? (status, path, parent, verb) => {
log.error(`${status} ${path} (${verb} from ${parent})`);
}
: (status, path) => {
throw new Error(`${status} ${path}`);
: (status, path, parent, verb) => {
throw new Error(`${status} ${path} (${verb} from ${parent})`);
};

const files = new Set([...build_data.static, ...build_data.client]);
Expand All @@ -86,8 +87,11 @@ export async function prerender({ cwd, out, log, config, build_data, force }) {
}
});

/** @param {string} path */
async function visit(path) {
/**
* @param {string} path
* @param {string} parent
*/
async function visit(path, parent) {
if (seen.has(path)) return;
seen.add(path);

Expand All @@ -104,10 +108,11 @@ export async function prerender({ cwd, out, log, config, build_data, force }) {
query: new URLSearchParams()
},
{
local: true,
dependencies,
only_render_prerenderable_pages: !force,
get_static_file: (file) => readFileSync(join(config.kit.files.assets, file))
prerender: {
force,
dependencies,
error: null
}
}
);

Expand Down Expand Up @@ -138,15 +143,15 @@ export async function prerender({ cwd, out, log, config, build_data, force }) {
log.info(`${rendered.status} ${path}`);
writeFileSync(file, rendered.body); // TODO minify where possible?
} else if (response_type !== OK) {
error(rendered.status, path);
error(rendered.status, path, parent, 'linked');
}

dependencies.forEach((result, path) => {
dependencies.forEach((result, dependency_path) => {
const response_type = Math.floor(result.status / 100);

const is_html = result.headers['content-type'] === 'text/html';

const parts = path.split('/');
const parts = dependency_path.split('/');
if (is_html && parts[parts.length - 1] !== 'index.html') {
parts.push('index.html');
}
Expand All @@ -157,9 +162,9 @@ export async function prerender({ cwd, out, log, config, build_data, force }) {
writeFileSync(file, result.body);

if (response_type === OK) {
log.info(`${result.status} ${path}`);
log.info(`${result.status} ${dependency_path}`);
} else {
error(result.status, path);
error(result.status, dependency_path, path, 'fetched');
}
});

Expand Down Expand Up @@ -200,7 +205,7 @@ export async function prerender({ cwd, out, log, config, build_data, force }) {
// TODO warn that query strings have no effect on statically-exported pages
}

await visit(parsed.pathname.replace(config.kit.paths.base, ''));
await visit(parsed.pathname.replace(config.kit.paths.base, ''), path);
}
}
}
Expand All @@ -209,10 +214,10 @@ export async function prerender({ cwd, out, log, config, build_data, force }) {
for (const entry of config.kit.prerender.pages) {
if (entry === '*') {
for (const entry of build_data.entries) {
await visit(entry);
await visit(entry, null);
}
} else {
await visit(entry);
await visit(entry, null);
}
}
}
76 changes: 39 additions & 37 deletions packages/kit/src/core/build/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -279,12 +279,42 @@ async function build_server(
.replace('%svelte.head%', '" + head + "')
.replace('%svelte.body%', '" + body + "')};
set_paths(${s(config.kit.paths)});
let options = null;
// allow paths to be overridden in svelte-kit start
export function init({ paths, prerendering }) {
set_paths(paths);
set_prerendering(prerendering);
// and in prerendering
export function init(settings) {
set_paths(settings.paths);
set_prerendering(settings.prerendering || false);
options = {
amp: ${config.kit.amp},
dev: false,
entry: {
file: ${s(prefix + client_manifest[client_entry_file].file)},
css: ${s(Array.from(entry_css).map(dep => prefix + dep))},
js: ${s(Array.from(entry_js).map(dep => prefix + dep))}
},
fetched: undefined,
get_component_path: id => ${s(`${config.kit.paths.assets}/${config.kit.appDir}/`)} + entry_lookup[id],
get_stack: error => String(error), // for security
handle_error: error => {
console.error(error.stack);
error.stack = options.get_stack(error);
},
hooks: get_hooks(user_hooks),
hydrate: ${s(config.kit.hydrate)},
initiator: undefined,
load_component,
manifest,
paths: settings.paths,
read: settings.read,
root,
router: ${s(config.kit.router)},
ssr: ${s(config.kit.ssr)},
target: ${s(config.kit.target)},
template
};
}
const d = decodeURIComponent;
Expand Down Expand Up @@ -331,8 +361,6 @@ async function build_server(
handle: hooks.handle || (({ request, render }) => render(request))
});
const hooks = get_hooks(user_hooks);
const module_lookup = {
${manifest.components.map(file => `${s(file)}: () => import(${s(app_relative(file))})`)}
};
Expand All @@ -349,39 +377,13 @@ async function build_server(
};
}
init({ paths: ${s(config.kit.paths)} });
export function render(request, {
paths = ${s(config.kit.paths)},
local = false,
dependencies,
only_render_prerenderable_pages = false,
get_static_file
prerender
} = {}) {
return ssr({
...request,
host: ${config.kit.host ? s(config.kit.host) : `request.headers[${s(config.kit.hostHeader || 'host')}]`}
}, {
paths,
local,
template,
manifest,
load_component,
target: ${s(config.kit.target)},
entry: ${s(prefix + client_manifest[client_entry_file].file)},
css: ${s(Array.from(entry_css).map(dep => prefix + dep))},
js: ${s(Array.from(entry_js).map(dep => prefix + dep))},
root,
hooks,
dev: false,
amp: ${config.kit.amp},
dependencies,
only_render_prerenderable_pages,
get_component_path: id => ${s(`${config.kit.paths.assets}/${config.kit.appDir}/`)} + entry_lookup[id],
get_stack: error => error.stack,
get_static_file,
ssr: ${s(config.kit.ssr)},
router: ${s(config.kit.router)},
hydrate: ${s(config.kit.hydrate)}
});
const host = ${config.kit.host ? s(config.kit.host) : `request.headers[${s(config.kit.hostHeader || 'host')}]`};
return ssr({ ...request, host }, options, { prerender });
}
`
.replace(/^\t{3}/gm, '')
Expand Down
138 changes: 71 additions & 67 deletions packages/kit/src/core/dev/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { EventEmitter } from 'events';
import CheapWatch from 'cheap-watch';
import amp_validator from 'amphtml-validator';
import vite from 'vite';
import colors from 'kleur';
import create_manifest_data from '../../core/create_manifest_data/index.js';
import { create_app } from '../../core/create_app/index.js';
import { rimraf } from '../filesystem/index.js';
Expand Down Expand Up @@ -130,9 +131,6 @@ class Watcher extends EventEmitter {

if (req.url === '/favicon.ico') return;

// handle dynamic requests - i.e. pages and endpoints
const template = fs.readFileSync(this.config.kit.files.template, 'utf-8');

const hooks = /** @type {import('types/internal').Hooks} */ (await this.vite
.ssrLoadModule(`/${this.config.kit.files.hooks}`)
.catch(() => ({})));
Expand Down Expand Up @@ -162,9 +160,77 @@ class Watcher extends EventEmitter {
body
},
{
amp: this.config.kit.amp,
dev: true,
entry: {
file: '/.svelte/dev/runtime/internal/start.js',
css: [],
js: []
},
get_stack: (error) => {
this.vite.ssrFixStacktrace(error);
return error.stack;
},
handle_error: (error) => {
this.vite.ssrFixStacktrace(error);
console.error(colors.bold().red(error.message));
console.error(colors.gray(error.stack));
},
hooks: {
getContext: hooks.getContext || (() => ({})),
getSession: hooks.getSession || (() => ({})),
handle: hooks.handle || (({ request, render }) => render(request))
},
hydrate: this.config.kit.hydrate,
paths: this.config.kit.paths,
load_component: async (id) => {
const url = path.resolve(this.cwd, id);

const module = /** @type {SSRComponent} */ (await this.vite.ssrLoadModule(url));
const node = await this.vite.moduleGraph.getModuleByUrl(url);

const deps = new Set();
find_deps(node, deps);

const styles = new Set();

for (const dep of deps) {
const parsed = parse(dep.url);
const query = new URLSearchParams(parsed.query);

// TODO what about .scss files, etc?
if (
dep.file.endsWith('.css') ||
(query.has('svelte') && query.get('type') === 'style')
) {
try {
const mod = await this.vite.ssrLoadModule(dep.url);
styles.add(mod.default);
} catch {
// this can happen with dynamically imported modules, I think
// because the Vite module graph doesn't distinguish between
// static and dynamic imports? TODO investigate, submit fix
}
}
}

return {
module,
entry: `/${id}?import`,
css: [],
js: [],
styles: Array.from(styles)
};
},
manifest: this.manifest,
read: (file) => fs.readFileSync(path.join(this.config.kit.files.assets, file)),
root,
router: this.config.kit.router,
ssr: this.config.kit.ssr,
target: this.config.kit.target,
template: ({ head, body }) => {
let rendered = template
let rendered = fs
.readFileSync(this.config.kit.files.template, 'utf8')
.replace('%svelte.head%', () => head)
.replace('%svelte.body%', () => body);

Expand Down Expand Up @@ -213,69 +279,7 @@ class Watcher extends EventEmitter {
}

return rendered;
},
manifest: this.manifest,
load_component: async (id) => {
const url = path.resolve(this.cwd, id);

const module = /** @type {SSRComponent} */ (await this.vite.ssrLoadModule(url));
const node = await this.vite.moduleGraph.getModuleByUrl(url);

const deps = new Set();
find_deps(node, deps);

const styles = new Set();

for (const dep of deps) {
const parsed = parse(dep.url);
const query = new URLSearchParams(parsed.query);

// TODO what about .scss files, etc?
if (
dep.file.endsWith('.css') ||
(query.has('svelte') && query.get('type') === 'style')
) {
try {
const mod = await this.vite.ssrLoadModule(dep.url);
styles.add(mod.default);
} catch {
// this can happen with dynamically imported modules, I think
// because the Vite module graph doesn't distinguish between
// static and dynamic imports? TODO investigate, submit fix
}
}
}

return {
module,
entry: `/${id}?import`,
css: [],
js: [],
styles: Array.from(styles)
};
},
target: this.config.kit.target,
entry: '/.svelte/dev/runtime/internal/start.js',
css: [],
js: [],
dev: true,
amp: this.config.kit.amp,
root,
hooks: {
getContext: hooks.getContext || (() => ({})),
getSession: hooks.getSession || (() => ({})),
handle: hooks.handle || (({ request, render }) => render(request))
},
only_render_prerenderable_pages: false,
get_stack: (error) => {
this.vite.ssrFixStacktrace(error);
return error.stack;
},
get_static_file: (file) =>
fs.readFileSync(path.join(this.config.kit.files.assets, file)),
ssr: this.config.kit.ssr,
router: this.config.kit.router,
hydrate: this.config.kit.hydrate
}
}
);

Expand Down
Loading

0 comments on commit 61d7fa0

Please sign in to comment.