Merge pull request #313 from scottnath/hotfix/error-route

errors not caught with custom err handler
punchcard-cms · Aug 29, 2016 · 7e4cc3f · 7e4cc3f
2 parents 2f7efc2 + 3aede7a
commit 7e4cc3f
Show file tree

Hide file tree

Showing 8 changed files with 270 additions and 56 deletions.
diff --git a/lib/errors/routes.js b/lib/errors/routes.js
@@ -0,0 +1,83 @@
+/**
+ *  @fileoverview Error routes
+ *
+ *  @author  Scott Nath
+ *
+ */
+'use strict';
+
+const _ = require('lodash');
+
+/**
+ * Route for a 404 error
+ * @param {object} req - HTTP Request
+ * @param {object} res - HTTP Response
+ *
+ * @returns {promise} promise containing the route
+ */
+const missing = (req, res) => {
+  return new Promise((resolve) => {
+    const message = _.get(req.session, '404.message', `Not Found: ${req.url}`);
+    const safe = _.get(req.session, '404.safe');
+
+    _.unset(req.session, '404');
+
+    res.status(404);
+    res.render('404', {
+      message,
+      safe,
+    });
+    resolve(true);
+  });
+};
+
+/**
+ * Route for errors
+ *
+ * @param {string|object} err - error object or error message string
+ * @param {object} req - HTTP Request
+ * @param {object} res - HTTP Response
+ * @param {object} next - Express callback
+ * @param {object} app Express application
+ *
+ * @returns {promise} promise containing the route
+ */
+const errors = (err, req, res, next, app) => {
+  return new Promise((resolve) => {
+    let error = {
+      message: 'route error',
+      status: '',
+      safe: '/',
+    };
+
+    if (typeof err === 'object') {
+      error = _.cloneDeep(err);
+    }
+    else if (typeof err === 'string' && err !== '') {
+      error.message = err;
+    }
+
+    error.error = {};
+
+    // If in development, will print stacktrace
+    if (app.get('env') === 'development') {
+      error.error = err;
+    }
+
+    res.status(error.status || 500);
+
+    if (error.status === 404) {
+      res.render('404', error);
+    }
+    else {
+      res.render('error', error);
+    }
+
+    resolve(true);
+  });
+};
+
+module.exports = {
+  missing,
+  errors,
+};
diff --git a/lib/routes/content.js b/lib/routes/content.js
@@ -150,12 +150,13 @@ const routes = application => {
         .where('id', req.params.id)
         .orderBy('revision', 'DESC').then(rws => {
           if (rws.length < 1) {
-            _.set(req.session, '404', {
+            const err = {
               message: config.content.messages.missing.id.replace('%type', req.params.type).replace('%id', req.params.id),
               safe: `/${config.content.base}/${req.params.type}`,
-            });
+              status: 404,
+            };
 
-            return next();
+            return next(err);
           }
 
           // add itentifier to each row
@@ -204,12 +205,13 @@ const routes = application => {
         .where('revision', req.params.revision)
         .orderBy('revision', 'DESC').then(rws => {
           if (rws.length < 1) {
-            _.set(req.session, '404', {
+            const err = {
               message: config.content.messages.missing.revision.replace('%revision', req.params.revision).replace('%type', req.params.type).replace('%id', req.params.id),
               safe: `/${config.content.base}/${req.params.type}`,
-            });
+              status: 404,
+            };
 
-            return next();
+            return next(err);
           }
 
           // add itentifier to each row
@@ -306,12 +308,13 @@ const routes = application => {
             revision: req.params.revision,
           }).then(rows => {
             if (rows.length < 1) {
-              _.set(req.session, '404', {
+              const err = {
                 message: config.content.messages.missing.revision.replace('%type', req.params.type).replace('%id', req.params.id).replace('%revision', req.params.revision),
                 safe: `/${config.content.base}/${req.params.type}`,
-              });
+                status: 404,
+              };
 
-              return next();
+              return next(err);
             }
             data = rows[0].value;
 

diff --git a/lib/routes/errors.js b/lib/routes/errors.js
@@ -1,6 +1,6 @@
 'use strict';
 
-const _ = require('lodash');
+const routes = require('../errors/routes');
 
 /*
  * Error Route Handling
@@ -21,16 +21,7 @@ module.exports = application => {
      * @param {object} res - HTTP Response
      */
     app.use((req, res) => {
-      const message = _.get(req.session, '404.message', `Not Found: ${req.url}`);
-      const safe = _.get(req.session, '404.safe');
-
-      _.unset(req.session, '404');
-
-      res.status(404);
-      res.render('404', {
-        message,
-        safe,
-      });
+      return routes.missing(req, res);
     });
 
     /*
@@ -41,20 +32,8 @@ module.exports = application => {
      * @param {object} req - HTTP Request
      * @param {object} res - HTTP Response
      */
-
-    app.use((err, req, res) => {
-      const error = {
-        message: err.message,
-        error: {},
-      };
-
-      // If in development, will print stacktrace
-      if (app.get('env') === 'development') {
-        error.error = err;
-      }
-
-      res.status(err.status || 500);
-      res.render('error', error);
+    app.use((err, req, res, next) => {
+      return routes.errors(err, req, res, next, app);
     });
 
     resolve(app);

diff --git a/lib/routes/users.js b/lib/routes/users.js
@@ -109,9 +109,13 @@ const routes = application => {
 
       return database('users').where('id', req.params.id).then(rows => {
         if (rows.length < 1) {
-          res.status(404);
-          const error = new Error(`${config.users.messages.errors.edit}`);
-          res.render('error', error);
+          const error = {
+            message: config.users.messages.errors.edit,
+            status: 404,
+            safe: '/users',
+          };
+
+          return next(error);
         }
         user = rows[0];
 
@@ -195,14 +199,19 @@ const routes = application => {
         message = config.users.messages.errors.current;
       }
 
-      database('users').where('id', req.params.id).then(rows => {
+      return database('users').where('id', req.params.id).then(rows => {
         if (rows.length < 1) {
-          res.status(404);
-          const error = new Error(`${config.users.delete.error}`);
-          res.render('error', error);
+          const error = {
+            message: config.users.messages.errors.delete,
+            status: 404,
+            safe: '/users',
+          };
+
+          return next(error);
         }
         const user = rows[0];
-        res.render('users/delete', {
+
+        return res.render('users/delete', {
           title: config.users.actions.delete,
           email: user.email,
           action: `/${config.users.base}/${config.users.actions.delete}`,
@@ -212,7 +221,7 @@ const routes = application => {
         });
       })
       .catch(error => {
-        next(error);
+        return next(error);
       });
     });
 

diff --git a/lib/routes/workflows.js b/lib/routes/workflows.js
@@ -71,12 +71,13 @@ const routes = application => {
           .where('id', req.params.id)
           .then(rws => {
             if (rws.length < 1) {
-              _.set(req.session, '404', {
+              const err = {
                 message: config.content.messages.missing.revision.replace('%revision', req.params.revision).replace('%type', req.params.type).replace('%id', req.params.id),
                 safe: `/${config.content.base}/${req.params.type}`,
-              });
+                status: 404,
+              };
 
-              return next();
+              return next(err);
             }
 
             // add the previous session data back in
@@ -106,12 +107,13 @@ const routes = application => {
 
             // if below zero, this content is already approved
             if (approval < 0) {
-              _.set(req.session, '404', {
+              const err = {
                 message: config.workflows.messages.approved,
                 safe: `/${config.content.base}/${req.params.type}`,
-              });
+                status: 404,
+              };
 
-              return next();
+              return next(err);
             }
 
             // grab the the next step in approval workflow

diff --git a/package.json b/package.json
@@ -89,6 +89,7 @@
     "input-plugin-text": "0.1.0",
     "lorem-ipsum": "^1.0.3",
     "mock-express-response": "^0.1.2",
+    "node-mocks-http": "^1.5.3",
     "nyc": "^6.6.0",
     "punchcard-commit-msg": "^1.0.0",
     "punchcard-runner": "^2.1.2",