From b975bb34a872e8974888e17ce46977489f98a641 Mon Sep 17 00:00:00 2001 From: Arjan Singh Date: Mon, 22 Aug 2016 19:52:53 -0700 Subject: [PATCH 1/2] Forward errors along to error handling middlewares --- src/index.js | 12 ++++++-- test/helpers/test-http-server.js | 17 ++++++++++- test/middleware-test.js | 50 +++++++++++++++++++++++++++++++- 3 files changed, 75 insertions(+), 4 deletions(-) diff --git a/src/index.js b/src/index.js index cce9801..036c578 100644 --- a/src/index.js +++ b/src/index.js @@ -36,17 +36,24 @@ function fastbootExpressMiddleware(distPath, options) { result.html() .then(html => { let headers = result.headers; + let statusMessage = result.error ? 'NOT OK ' : 'OK '; for (var pair of headers.entries()) { res.set(pair[0], pair[1]); } - log(result.statusCode, 'OK ' + path); + if (result.error) { + log("RESILIENT MODE CAUGHT:", result.error.stack); + next(result.error); + } + + log(result.statusCode, statusMessage + path); res.status(result.statusCode); res.send(html); }) .catch(error => { - console.log(error.stack); + log(500, error.stack); + next(error); res.sendStatus(500); }); } @@ -55,6 +62,7 @@ function fastbootExpressMiddleware(distPath, options) { if (error.name === "UnrecognizedURLError") { next(); } else { + next(error); log(500, "Unknown Error: " + error.stack); if (error.stack) { res.status(500).send(error.stack); diff --git a/test/helpers/test-http-server.js b/test/helpers/test-http-server.js index 9631483..c268194 100644 --- a/test/helpers/test-http-server.js +++ b/test/helpers/test-http-server.js @@ -21,6 +21,13 @@ class TestHTTPServer { app.get('/*', this.middleware); + if (options.errorHandling) { + app.use((err, req, res, next) => { + res.set('x-test-error', 'error handler called'); + next(); + }); + } + return new Promise((resolve, reject) => { let port = options.port || 3000; let host = options.host || 'localhost'; @@ -42,9 +49,17 @@ class TestHTTPServer { }); } - request(urlPath) { + request(urlPath, options) { let info = this.info; let url = 'http://[' + info.host + ']:' + info.port; + + if (options && options.resolveWithFullResponse) { + return request({ + resolveWithFullResponse: options.resolveWithFullResponse, + uri: url + urlPath + }); + } + return request(url + urlPath); } diff --git a/test/middleware-test.js b/test/middleware-test.js index 5d74dec..ab300b2 100644 --- a/test/middleware-test.js +++ b/test/middleware-test.js @@ -65,7 +65,7 @@ describe("FastBoot", function() { }); }); - it("renders an empty page if the resilient flag is set", function() { + it("renders no FastBoot markup if the resilient flag is set", function() { let middleware = fastbootMiddleware({ distPath: fixture('rejected-promise'), resilient: true @@ -79,6 +79,54 @@ describe("FastBoot", function() { }); }); + it("propagates to error handling middleware if the resilient flag is set", function() { + let middleware = fastbootMiddleware({ + distPath: fixture('rejected-promise'), + resilient: true + }); + server = new TestHTTPServer(middleware, { errorHandling: true }); + + return server.start() + .then(() => server.request('/', { resolveWithFullResponse: true })) + .then(({ body, statusCode, headers }) => { + expect(statusCode).to.equal(200); + expect(headers['x-test-error']).to.match(/error handler called/); + expect(body).to.match(/hello world/); + }); + }); + + it("propagates to error handling middleware if the resilient flag is not set", function() { + let middleware = fastbootMiddleware({ + distPath: fixture('rejected-promise'), + resilient: false, + }); + server = new TestHTTPServer(middleware, { errorHandling: true }); + + return server.start() + .then(() => server.request('/', { resolveWithFullResponse: true })) + .catch(({ statusCode, response: { headers } }) => { + expect(statusCode).to.equal(500); + expect(headers['x-test-error']).to.match(/error handler called/); + }); + }); + + it("is does not propagate errors when the reslient flag is set and there is no error handling middleware", function() { + let middleware = fastbootMiddleware({ + distPath: fixture('rejected-promise'), + resilient: true, + }); + server = new TestHTTPServer(middleware, { errorHandling: false }); + + return server.start() + .then(() => server.request('/', { resolveWithFullResponse: true })) + .then(({ body, statusCode, headers }) => { + expect(statusCode).to.equal(200); + expect(headers['x-test-error']).to.not.match(/error handler called/); + expect(body).to.not.match(/error/); + expect(body).to.match(/hello world/); + }); + }); + it("can be provided with a custom FastBoot instance", function() { let fastboot = new FastBoot({ distPath: fixture('basic-app') From 28e3ee2931ad3a5b46da19a0e8bc401b821ac79a Mon Sep 17 00:00:00 2001 From: Arjan Singh Date: Thu, 1 Sep 2016 15:21:31 -0700 Subject: [PATCH 2/2] Allow post-fastboot middlewares to recover from fastboot failure --- README.md | 9 +- src/index.js | 16 +-- test/helpers/test-http-server.js | 10 +- test/middleware-test.js | 161 +++++++++++++++++++------------ 4 files changed, 120 insertions(+), 76 deletions(-) diff --git a/README.md b/README.md index fcd57ac..867d368 100644 --- a/README.md +++ b/README.md @@ -49,7 +49,7 @@ provide to the middleware to specify which Ember app to load and render. By default, errors during render will cause the middleware to send an HTTP 500 status code as the response. In order to swallow errors and -return a `200 OK` with an empty HTML page, set the `resilient` flag to +return a `200` status code with an empty HTML page, set the `resilient` flag to true: ```js @@ -58,6 +58,13 @@ app.get('/*', fastbootMiddleware('/path/to/dist', { })); ``` +Resilient mode still calls `next(err)` to propagate your error to any subsequent +middleware that you apply after this one. +You can use this feature to track errors or log analytics. + +However, because FastBoot is reslient still sends the response to the client. +***You cannot alter the `response`*** with any of your post-fastboot middleware. + ## Custom FastBoot Instance For more control over the FastBoot instance that is created to render diff --git a/src/index.js b/src/index.js index 036c578..ff21a28 100644 --- a/src/index.js +++ b/src/index.js @@ -52,24 +52,16 @@ function fastbootExpressMiddleware(distPath, options) { res.send(html); }) .catch(error => { - log(500, error.stack); + res.status(500); next(error); - res.sendStatus(500); }); } function failure(error) { - if (error.name === "UnrecognizedURLError") { - next(); - } else { - next(error); - log(500, "Unknown Error: " + error.stack); - if (error.stack) { - res.status(500).send(error.stack); - } else { - res.sendStatus(500); - } + if (error.name !== "UnrecognizedURLError") { + res.status(500); } + next(error); } }; } diff --git a/test/helpers/test-http-server.js b/test/helpers/test-http-server.js index c268194..9eace54 100644 --- a/test/helpers/test-http-server.js +++ b/test/helpers/test-http-server.js @@ -24,7 +24,15 @@ class TestHTTPServer { if (options.errorHandling) { app.use((err, req, res, next) => { res.set('x-test-error', 'error handler called'); - next(); + next(err); + }); + } + + if (options.recoverErrors) { + app.use((err, req, res, next) => { + res.set('x-test-recovery', 'recovered response'); + res.status(200); + res.send('hello world'); }); } diff --git a/test/middleware-test.js b/test/middleware-test.js index ab300b2..da17144 100644 --- a/test/middleware-test.js +++ b/test/middleware-test.js @@ -65,68 +65,6 @@ describe("FastBoot", function() { }); }); - it("renders no FastBoot markup if the resilient flag is set", function() { - let middleware = fastbootMiddleware({ - distPath: fixture('rejected-promise'), - resilient: true - }); - server = new TestHTTPServer(middleware); - - return server.start() - .then(() => server.request('/')) - .then(html => { - expect(html).to.not.match(/error/); - }); - }); - - it("propagates to error handling middleware if the resilient flag is set", function() { - let middleware = fastbootMiddleware({ - distPath: fixture('rejected-promise'), - resilient: true - }); - server = new TestHTTPServer(middleware, { errorHandling: true }); - - return server.start() - .then(() => server.request('/', { resolveWithFullResponse: true })) - .then(({ body, statusCode, headers }) => { - expect(statusCode).to.equal(200); - expect(headers['x-test-error']).to.match(/error handler called/); - expect(body).to.match(/hello world/); - }); - }); - - it("propagates to error handling middleware if the resilient flag is not set", function() { - let middleware = fastbootMiddleware({ - distPath: fixture('rejected-promise'), - resilient: false, - }); - server = new TestHTTPServer(middleware, { errorHandling: true }); - - return server.start() - .then(() => server.request('/', { resolveWithFullResponse: true })) - .catch(({ statusCode, response: { headers } }) => { - expect(statusCode).to.equal(500); - expect(headers['x-test-error']).to.match(/error handler called/); - }); - }); - - it("is does not propagate errors when the reslient flag is set and there is no error handling middleware", function() { - let middleware = fastbootMiddleware({ - distPath: fixture('rejected-promise'), - resilient: true, - }); - server = new TestHTTPServer(middleware, { errorHandling: false }); - - return server.start() - .then(() => server.request('/', { resolveWithFullResponse: true })) - .then(({ body, statusCode, headers }) => { - expect(statusCode).to.equal(200); - expect(headers['x-test-error']).to.not.match(/error handler called/); - expect(body).to.not.match(/error/); - expect(body).to.match(/hello world/); - }); - }); - it("can be provided with a custom FastBoot instance", function() { let fastboot = new FastBoot({ distPath: fixture('basic-app') @@ -181,4 +119,103 @@ describe("FastBoot", function() { }); } }); + + describe('when reslient mode is enabled', function () { + it("renders no FastBoot markup", function() { + let middleware = fastbootMiddleware({ + distPath: fixture('rejected-promise'), + resilient: true + }); + server = new TestHTTPServer(middleware); + + return server.start() + .then(() => server.request('/')) + .then(html => { + expect(html).to.not.match(/error/); + }); + }); + + it("propagates to error handling middleware", function() { + let middleware = fastbootMiddleware({ + distPath: fixture('rejected-promise'), + resilient: true + }); + server = new TestHTTPServer(middleware, { errorHandling: true }); + + return server.start() + .then(() => server.request('/', { resolveWithFullResponse: true })) + .then(({ body, statusCode, headers }) => { + expect(statusCode).to.equal(200); + expect(headers['x-test-error']).to.match(/error handler called/); + expect(body).to.match(/hello world/); + }); + }); + + it("is does not propagate errors when and there is no error handling middleware", function() { + let middleware = fastbootMiddleware({ + distPath: fixture('rejected-promise'), + resilient: true, + }); + server = new TestHTTPServer(middleware, { errorHandling: false }); + + return server.start() + .then(() => server.request('/', { resolveWithFullResponse: true })) + .then(({ body, statusCode, headers }) => { + expect(statusCode).to.equal(200); + expect(headers['x-test-error']).to.not.match(/error handler called/); + expect(body).to.not.match(/error/); + expect(body).to.match(/hello world/); + }); + }); + + it("allows post-fastboot middleware to recover the response when it fails", function() { + let middleware = fastbootMiddleware({ + distPath: fixture('rejected-promise'), + resilient: true + }); + server = new TestHTTPServer(middleware, { recoverErrors: true }); + + return server.start() + .then(() => server.request('/', { resolveWithFullResponse: true })) + .then(({ body, statusCode, headers }) => { + expect(statusCode).to.equal(200); + expect(headers['x-test-recovery']).to.match(/recovered response/); + expect(body).to.match(/hello world/); + }); + }); + }); + + describe('when reslient mode is disabled', function () { + it("propagates to error handling middleware", function() { + let middleware = fastbootMiddleware({ + distPath: fixture('rejected-promise'), + resilient: false, + }); + server = new TestHTTPServer(middleware, { errorHandling: true }); + + return server.start() + .then(() => server.request('/', { resolveWithFullResponse: true })) + .catch(({ statusCode, response: { headers } }) => { + expect(statusCode).to.equal(500); + expect(headers['x-test-error']).to.match(/error handler called/); + }); + }); + + it("allows post-fastboot middleware to recover the response when it fails", function() { + let middleware = fastbootMiddleware({ + distPath: fixture('rejected-promise'), + resilient: false + }); + server = new TestHTTPServer(middleware, { recoverErrors: true }); + + return server.start() + .then(() => server.request('/', { resolveWithFullResponse: true })) + .then(({ body, statusCode, headers }) => { + expect(statusCode).to.equal(200); + expect(headers['x-test-recovery']).to.match(/recovered response/); + expect(body).to.match(/hello world/); + }); + }); + }); + });