From b1d072d43a798e90b7d8b059c813f1f35446437b Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 28 Feb 2018 16:54:38 -0500 Subject: [PATCH 1/2] dont serve error page for unhandled server route errors - fixes #138 --- src/middleware/index.ts | 22 +++++++++++++++++----- test/app/routes/throw-an-error.js | 3 +++ test/common/test.js | 8 ++++++++ 3 files changed, 28 insertions(+), 5 deletions(-) create mode 100644 test/app/routes/throw-an-error.js diff --git a/src/middleware/index.ts b/src/middleware/index.ts index b9336b2..38115b8 100644 --- a/src/middleware/index.ts +++ b/src/middleware/index.ts @@ -115,7 +115,7 @@ function get_asset_handler({ pathname, type, cache, body }: { const resolved = Promise.resolve(); function get_route_handler(chunks: Record, routes: RouteObject[], template: Template) { - function handle_route(route: RouteObject, req: Req, res: ServerResponse) { + function handle_route(route: RouteObject, req: Req, res: ServerResponse, next: () => void) { req.params = route.params(route.pattern.exec(req.pathname)); const mod = route.module; @@ -234,9 +234,21 @@ function get_route_handler(chunks: Record, routes: RouteObject[] }; } - handler(req, res, () => { - handle_not_found(req, res, 404, 'Not found'); - }); + const handle_error = err => { + if (err) { + console.error(err.stack); + res.statusCode = 500; + res.end(err.message); + } else { + handle_not_found(req, res, 404, 'Not found'); + } + }; + + try { + handler(req, res, handle_error); + } catch (err) { + handle_error(err); + } } else { // no matching handler for method — 404 handle_not_found(req, res, 404, 'Not found'); @@ -299,7 +311,7 @@ function get_route_handler(chunks: Record, routes: RouteObject[] try { for (const route of routes) { - if (!route.error && route.pattern.test(url)) return handle_route(route, req, res); + if (!route.error && route.pattern.test(url)) return handle_route(route, req, res, next); } handle_not_found(req, res, 404, 'Not found'); diff --git a/test/app/routes/throw-an-error.js b/test/app/routes/throw-an-error.js new file mode 100644 index 0000000..523f2bc --- /dev/null +++ b/test/app/routes/throw-an-error.js @@ -0,0 +1,3 @@ +export function get() { + throw new Error('nope'); +} \ No newline at end of file diff --git a/test/common/test.js b/test/common/test.js index 9119394..297c8d5 100644 --- a/test/common/test.js +++ b/test/common/test.js @@ -391,6 +391,14 @@ function run(env) { JSON.parse(text); }); }); + + it('does not serve error page for non-page errors', () => { + return nightmare.goto(`${base}/throw-an-error`) + .page.text() + .then(text => { + assert.equal(text, 'nope'); + }); + }); }); describe('headers', () => { From 5dd04eb35c43816e8ddbae35c278eb84aa6b7b29 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 28 Feb 2018 16:59:39 -0500 Subject: [PATCH 2/2] tidy up - next is unused --- src/middleware/index.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/middleware/index.ts b/src/middleware/index.ts index 38115b8..bc5ff66 100644 --- a/src/middleware/index.ts +++ b/src/middleware/index.ts @@ -115,7 +115,7 @@ function get_asset_handler({ pathname, type, cache, body }: { const resolved = Promise.resolve(); function get_route_handler(chunks: Record, routes: RouteObject[], template: Template) { - function handle_route(route: RouteObject, req: Req, res: ServerResponse, next: () => void) { + function handle_route(route: RouteObject, req: Req, res: ServerResponse) { req.params = route.params(route.pattern.exec(req.pathname)); const mod = route.module; @@ -234,7 +234,7 @@ function get_route_handler(chunks: Record, routes: RouteObject[] }; } - const handle_error = err => { + const handle_error = (err?: Error) => { if (err) { console.error(err.stack); res.statusCode = 500; @@ -306,12 +306,12 @@ function get_route_handler(chunks: Record, routes: RouteObject[] })); } - return function find_route(req: Req, res: ServerResponse, next: () => void) { + return function find_route(req: Req, res: ServerResponse) { const url = req.pathname; try { for (const route of routes) { - if (!route.error && route.pattern.test(url)) return handle_route(route, req, res, next); + if (!route.error && route.pattern.test(url)) return handle_route(route, req, res); } handle_not_found(req, res, 404, 'Not found');