Merge branch 'master' into unique-username

app.js

@@ -9,6 +9,7 @@ const enabled = require('debug').enabled;
 const RedisStore = require('connect-redis')(session);
 const redis = require('./services/redis');
 const csrf = require('csurf');
+const errors = require('./errors');
 
 const app = express();
 
@@ -109,40 +110,47 @@ app.use('/', require('./routes'));
 // ERROR HANDLING
 //==============================================================================
 
-const ErrNotFound = new Error('Not Found');
-ErrNotFound.status = 404;
-
 // Catch 404 and forward to error handler.
 app.use((req, res, next) => {
-  next(ErrNotFound);
+  next(errors.ErrNotFound);
 });
 
 // General error handler. Respond with the message and error if we have it while
 // returning a status code that makes sense.
 app.use('/api', (err, req, res, next) => {
-  if (err !== ErrNotFound) {
+  if (err !== errors.ErrNotFound) {
     if (app.get('env') !== 'test' || enabled('talk:errors')) {
       console.error(err);
     }
   }
 
-  res.status(err.status || 500);
-  res.json({
-    message: err.message,
-    error: app.get('env') === 'development' ? err : {}
-  });
+  if (err instanceof errors.APIError) {
+    res.status(err.status).json({
+      message: err.message,
+      error: err
+    });
+  } else {
+    res.status(500).json({});
+  }
 });
 
 app.use('/', (err, req, res, next) => {
-  if (err !== ErrNotFound) {
+  if (err !== errors.ErrNotFound) {
     console.error(err);
   }
 
-  res.status(err.status || 500);
-  res.render('error', {
-    message: err.message,
-    error: app.get('env') === 'development' ? err : {}
-  });
+  if (err instanceof errors.APIError) {
+    res.status(err.status);
+    res.render('error', {
+      message: err.message,
+      error: app.get('env') === 'development' ? err : {}
+    });
+  } else {
+    res.render('error', {
+      message: err.message,
+      error: app.get('env') === 'development' ? err : {}
+    });
+  }
 });
 
 module.exports = app;

errors.js

@@ -1,50 +1,129 @@
+/**
+ * ExtendableError provides a base Error class to source off of that does not
+ * break the inheritence chain.
+ */
+class ExtendableError {
+  constructor(message = null) {
+    this.message = message;
+    this.stack = (new Error()).stack;
+  }
+}
+
+/**
+ * APIError is the base error that all application issued errors originate, they
+ * are composed of data used by the front end and backend to handle errors
+ * consistently.
+ */
+class APIError extends ExtendableError {
+  constructor(message, {status = 500, translation_key = null}, metadata = {}) {
+    super(message);
+
+    this.status = status;
+    this.translation_key = translation_key;
+    this.metadata = metadata;
+  }
+
+  toJSON() {
+    return {
+      message: this.message,
+      status: this.status,
+      translation_key: this.translation_key,
+      metadata: this.metadata
+    };
+  }
+}
+
 // ErrPasswordTooShort is returned when the password length is too short.
-const ErrPasswordTooShort = new Error('password must be at least 8 characters');
-ErrPasswordTooShort.translation_key = 'PASSWORD_LENGTH';
-ErrPasswordTooShort.status = 400;
+const ErrPasswordTooShort = new APIError('password must be at least 8 characters', {
+  status: 400,
+  translation_key: 'PASSWORD_LENGTH'
+});
 
-const ErrMissingEmail = new Error('email is required');
-ErrMissingEmail.translation_key = 'EMAIL_REQUIRED';
-ErrMissingEmail.status = 400;
+const ErrMissingEmail = new APIError('email is required', {
+  translation_key: 'EMAIL_REQUIRED',
+  status: 400
+});
 
-const ErrMissingPassword = new Error('password is required');
-ErrMissingPassword.translation_key = 'PASSWORD_REQUIRED';
-ErrMissingPassword.status = 400;
+const ErrMissingPassword = new APIError('password is required', {
+  translation_key: 'PASSWORD_REQUIRED',
+  status: 400
+});
 
-const ErrEmailTaken = new Error('Email address already in use');
-ErrEmailTaken.translation_key = 'EMAIL_IN_USE';
-ErrEmailTaken.status = 400;
+const ErrEmailTaken = new APIError('Email address already in use', {
+  translation_key: 'EMAIL_IN_USE',
+  status: 400
+});
 
-const ErrDisplayTaken = new Error('Display name already in use');
-ErrDisplayTaken.translation_key = 'DISPLAYNAME_IN_USE';
-ErrDisplayTaken.status = 400;
+const ErrDisplayTaken = new Error('Display name already in use', {
+  translation_key: 'DISPLAYNAME_IN_USE',
+  status: 400
+});
 
-const ErrSpecialChars = new Error('No special characters are allowed in a display name');
-ErrSpecialChars.translation_key = 'NO_SPECIAL_CHARACTERS';
-ErrSpecialChars.status = 400;
+const ErrSpecialChars = new APIError('No special characters are allowed in a display name', {
+  translation_key: 'NO_SPECIAL_CHARACTERS',
+  status: 400
+});
 
-const ErrMissingDisplay = new Error('A display name is required to create a user');
-ErrMissingDisplay.translation_key = 'DISPLAY_NAME_REQUIRED';
-ErrMissingDisplay.status = 400;
+const ErrMissingDisplay = new APIError('A display name is required to create a user', {
+  translation_key: 'DISPLAY_NAME_REQUIRED',
+  status: 400
+});
 
 // ErrMissingToken is returned in the event that the password reset is requested
 // without a token.
-const ErrMissingToken = new Error('token is required');
-ErrMissingToken.status = 400;
+const ErrMissingToken = new APIError('token is required', {
+  status: 400
+});
+
+// ErrAssetCommentingClosed is returned when a comment or action is attempted on
+// a stream where commenting has been closed.
+class ErrAssetCommentingClosed extends APIError {
+  constructor(closedMessage = null) {
+    super('asset commenting is closed', {
+      status: 400
+    }, {
+
+      // Include the closedMessage in the metadata piece of the error.
+      closedMessage
+    });
+  }
+}
 
 // ErrContainsProfanity is returned in the event that the middleware detects
 // profanity/wordlisted words in the payload.
-const ErrContainsProfanity = new Error('Suspected profanity. If you think this in error, please let us know!');
-ErrContainsProfanity.translation_key = 'PROFANITY_ERROR';
-ErrContainsProfanity.status = 400;
+const ErrContainsProfanity = new APIError('Suspected profanity. If you think this in error, please let us know!', {
+  translation_key: 'PROFANITY_ERROR',
+  status: 400
+});
+
+const ErrNotFound = new APIError('not found', {
+  status: 404
+});
+
+const ErrInvalidAssetURL = new APIError('asset_url is invalid', {
+  status: 400
+});
+
+// ErrNotAuthorized is an error that is returned in the event an operation is
+// deemed not authorized.
+const ErrNotAuthorized = new APIError('not authorized', {
+  status: 401
+});
 
 module.exports = {
+  ExtendableError,
+  APIError,
   ErrPasswordTooShort,
   ErrMissingEmail,
   ErrMissingPassword,
+  ErrMissingToken,
   ErrEmailTaken,
   ErrSpecialChars,
   ErrMissingDisplay,
   ErrContainsProfanity,
-  ErrDisplayTaken
+  ErrDisplayTaken,
+  ErrAssetCommentingClosed,
+  ErrNotFound,
+  ErrInvalidAssetURL,
+  ErrNotAuthorized
 };

middleware/authorization.js

@@ -7,17 +7,7 @@ const authorization = module.exports = {
 };
 
 const debug = require('debug')('talk:middleware:authorization');
-
-/**
- * ErrNotAuthorized is an error that is returned in the event an operation is
- * deemed not authorized.
- * @type {Error}
- */
-const ErrNotAuthorized = new Error('not authorized');
-ErrNotAuthorized.status = 401;
-
-// Add the ErrNotAuthorized error to the authorization object to be exported.
-authorization.ErrNotAuthorized = ErrNotAuthorized;
+const ErrNotAuthorized = require('../errors').ErrNotAuthorized;
 
 /**
  * has returns true if the user has all the roles specified, otherwise it will

models/setting.js

@@ -42,7 +42,7 @@ const SettingSchema = new Schema({
   },
   closedMessage: {
     type: String,
-    default: ''
+    default: 'Expired'
   },
   wordlist: WordlistSchema,
   charCount: {

routes/api/auth/index.js

@@ -1,6 +1,7 @@
 const express = require('express');
 const passport = require('../../../services/passport');
 const authorization = require('../../../middleware/authorization');
+const errors = require('../../../errors');
 
 const router = express.Router();
 
@@ -42,7 +43,7 @@ const HandleAuthCallback = (req, res, next) => (err, user) => {
   }
 
   if (!user) {
-    return next(authorization.ErrNotAuthorized);
+    return next(errors.ErrNotAuthorized);
   }
 
   // Perform the login of the user!
@@ -65,7 +66,7 @@ const HandleAuthPopupCallback = (req, res, next) => (err, user) => {
   }
 
   if (!user) {
-    return res.render('auth-callback', {err: JSON.stringify(authorization.ErrNotAuthorized), data: null});
+    return res.render('auth-callback', {err: JSON.stringify(errors.ErrNotAuthorized), data: null});
   }
 
   // Perform the login of the user!

routes/api/comments/index.js

@@ -1,4 +1,5 @@
 const express = require('express');
+const errors = require('../../../errors');
 const Comment = require('../../../models/comment');
 const Asset = require('../../../models/asset');
 const User = require('../../../models/user');
@@ -20,13 +21,13 @@ router.get('/', (req, res, next) => {
 
   // everything on this route requires admin privileges besides listing comments for owner of said comments
   if (!authorization.has(req.user, 'admin') && !user_id) {
-    next(authorization.ErrNotAuthorized);
+    next(errors.ErrNotAuthorized);
     return;
   }
 
   // if the user is not an admin, only return comment list for the owner of the comments
   if (req.user.id !== user_id && !authorization.has(req.user, 'admin')) {
-    next(authorization.ErrNotAuthorized);
+    next(errors.ErrNotAuthorized);
     return;
   }
 
@@ -102,14 +103,14 @@ router.post('/', wordlist.filter('body'), (req, res, next) => {
     status = Asset
       .rectifySettings(Asset.findById(asset_id).then((asset) => {
         if (!asset) {
-          return Promise.reject(new Error('asset referenced is not found'));
+          return Promise.reject(errors.ErrNotFound);
         }
 
         // Check to see if the asset has closed commenting...
         if (asset.isClosed) {
 
           // They have, ensure that we send back an error.
-          return Promise.reject(new Error(`asset has commenting closed because: ${asset.closedMessage}`));
+          return Promise.reject(new errors.ErrAssetCommentingClosed(asset.closedMessage));
         }
 
         return asset;

routes/api/stream/index.js

@@ -1,6 +1,7 @@
 const express = require('express');
 const _ = require('lodash');
 const scraper = require('../../../services/scraper');
+const errors = require('../../../errors');
 const url = require('url');
 
 const Comment = require('../../../models/comment');
@@ -9,9 +10,6 @@ const Action = require('../../../models/action');
 const Asset = require('../../../models/asset');
 const Setting = require('../../../models/setting');
 
-const ErrInvalidAssetURL = new Error('asset_url is invalid');
-ErrInvalidAssetURL.status = 400;
-
 const router = express.Router();
 
 router.get('/', (req, res, next) => {
@@ -21,7 +19,7 @@ router.get('/', (req, res, next) => {
   // Verify that the asset_url is parsable.
   let parsed_asset_url = url.parse(asset_url);
   if (!parsed_asset_url.protocol) {
-    return next(ErrInvalidAssetURL);
+    return next(errors.ErrInvalidAssetURL);
   }
 
   // Get the asset_id for this url (or create it if it doesn't exist)

tests/routes/api/comments/index.js

@@ -101,7 +101,7 @@ describe('/api/v1/comments', () => {
           expect(res).to.be.empty;
         })
         .catch((err) => {
-          expect(err).to.have.property('status', 401);
+          expect(err).to.have.status(401);
         });
     });
 
@@ -287,7 +287,7 @@ describe('/api/v1/comments', () => {
       .catch((err) => {
         expect(err.response.body).to.not.be.null;
         expect(err.response.body).to.have.property('message');
-        expect(err.response.body.message).to.contain('tests said expired!');
+        expect(err.response.body.error.metadata.closedMessage).to.be.equal('tests said expired!');
       });
     });