Merge pull request #11 from dollarshaveclub/resilient-error-handling

Forward errors along to error handling middlewares

Author: danmcclain

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

src/index.js

@@ -36,32 +36,32 @@ 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);
-          res.sendStatus(500);
+          res.status(500);
+          next(error);
         });
     }
 
     function failure(error) {
-      if (error.name === "UnrecognizedURLError") {
-        next();
-      } else {
-        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);
     }
   };
 }

test/helpers/test-http-server.js

@@ -21,6 +21,21 @@ class TestHTTPServer {
 
     app.get('/*', this.middleware);
 
+    if (options.errorHandling) {
+      app.use((err, req, res, next) => {
+        res.set('x-test-error', 'error handler called');
+        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');
+      });
+    }
+
     return new Promise((resolve, reject) => {
       let port = options.port || 3000;
       let host = options.host || 'localhost';
@@ -42,9 +57,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);
   }
 

test/middleware-test.js

@@ -65,20 +65,6 @@ describe("FastBoot", function() {
       });
   });
 
-  it("renders an empty page 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("can be provided with a custom FastBoot instance", function() {
     let fastboot = new FastBoot({
       distPath: fixture('basic-app')
@@ -133,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/);
+        });
+    });
+  });
+
 });