Merge pull request #610 from softionsrl/patch-1

Standardization of error handling

routes/api.js

@@ -12,29 +12,27 @@ let log = require('npmlog');
 let router = new express.Router();
 let mailHelpers = require('../lib/subscription-mail-helpers');
 
-router.all('/*', (req, res, next) => {
+const handleErrorResponse = (res, log, err, code = 500, message = false) => {
-    if (!req.query.access_token) {
+    if (typeof err != 'undefined')
-        res.status(403);
+        log.error('API', err);
+    res.status(code);
     return res.json({
-            error: 'Missing access_token',
+        error: message || err.message || err,
         data: []
     });
+}
+
+router.all('/*', (req, res, next) => {
+    if (!req.query.access_token) {
+        return handleErrorResponse(res, log, false, 403, 'Missing access_token');
     }
 
     users.findByAccessToken(req.query.access_token, (err, user) => {
         if (err) {
-            res.status(500);
+            return handleErrorResponse(res, log, err);
-            return res.json({
-                error: err.message || err,
-                data: []
-            });
         }
         if (!user) {
-            res.status(403);
+            return handleErrorResponse(res, log, false, 403, 'Invalid or expired access_token');
-            return res.json({
-                error: 'Invalid or expired access_token',
-                data: []
-            });
         }
         next();
     });
@@ -48,35 +46,17 @@ router.post('/subscribe/:listId', (req, res) => {
     });
     lists.getByCid(req.params.listId, (err, list) => {
         if (err) {
-            log.error('API', err);
+            return handleErrorResponse(res, log, false, 403, 'Invalid or expired access_token');
-            res.status(500);
-            return res.json({
-                error: err.message || err,
-                data: []
-            });
         }
         if (!list) {
-            res.status(404);
+            return handleErrorResponse(res, log, false, 404, 'Selected listId not found');
-            return res.json({
-                error: 'Selected listId not found',
-                data: []
-            });
         }
         if (!input.EMAIL) {
-            res.status(400);
+            return handleErrorResponse(res, log, false, 400, 'Missing EMAIL');
-            return res.json({
-                error: 'Missing EMAIL',
-                data: []
-            });
         }
         tools.validateEmail(input.EMAIL, false, err => {
             if (err) {
-                log.error('API', err);
+                return handleErrorResponse(res, log, err, 400);
-                res.status(400);
-                return res.json({
-                    error: err.message || err,
-                    data: []
-                });
             }
 
             let subscription = {
@@ -132,22 +112,12 @@ router.post('/subscribe/:listId', (req, res) => {
 
                     confirmations.addConfirmation(list.id, 'subscribe', req.ip, data, (err, confirmCid) => {
                         if (err) {
-                            log.error('API', err);
+                            return handleErrorResponse(res, log, err);
-                            res.status(500);
-                            return res.json({
-                                error: err.message || err,
-                                data: []
-                            });
                         }
 
                         mailHelpers.sendConfirmSubscription(list, input.EMAIL, confirmCid, subscription, (err) => {
                             if (err) {
-                                log.error('API', err);
+                                return handleErrorResponse(res, log, err);
-                                res.status(500);
-                                return res.json({
-                                    error: err.message || err,
-                                    data: []
-                                });
                             }
 
                             res.status(200);
@@ -161,12 +131,7 @@ router.post('/subscribe/:listId', (req, res) => {
                 } else {
                     subscriptions.insert(list.id, meta, subscription, (err, response) => {
                         if (err) {
-                            log.error('API', err);
+                            return handleErrorResponse(res, log, err);
-                            res.status(500);
-                            return res.json({
-                                error: err.message || err,
-                                data: []
-                            });
                         }
                         res.status(200);
                         res.json({
@@ -188,51 +153,26 @@ router.post('/unsubscribe/:listId', (req, res) => {
     });
     lists.getByCid(req.params.listId, (err, list) => {
         if (err) {
-            res.status(500);
+            return handleErrorResponse(res, log, err);
-            return res.json({
-                error: err.message || err,
-                data: []
-            });
         }
         if (!list) {
-            res.status(404);
+            return handleErrorResponse(res, log, false, 404, 'Selected listId not found');
-            return res.json({
-                error: 'Selected listId not found',
-                data: []
-            });
         }
         if (!input.EMAIL) {
-            res.status(400);
+            return handleErrorResponse(res, log, false, 400, 'Missing EMAIL');
-            return res.json({
-                error: 'Missing EMAIL',
-                data: []
-            });
         }
 
         subscriptions.getByEmail(list.id, input.EMAIL, (err, subscription) => {
             if (err) {
-                res.status(500);
+                return handleErrorResponse(res, log, err);
-                return res.json({
-                    error: err.message || err,
-                    data: []
-                });
             }
-
             if (!subscription) {
-                res.status(404);
+                return handleErrorResponse(res, log, false, 404, 'Subscription with given email not found');
-                return res.json({
-                    error: 'Subscription with given email not found',
-                    data: []
-                });
             }
 
             subscriptions.changeStatus(list.id, subscription.id, false, subscriptions.Status.UNSUBSCRIBED, (err, found) => {
                 if (err) {
-                    res.status(500);
+                    return handleErrorResponse(res, log, err);
-                    return res.json({
-                        error: err.message || err,
-                        data: []
-                    });
                 }
                 res.status(200);
                 res.json({
@@ -253,55 +193,27 @@ router.post('/delete/:listId', (req, res) => {
     });
     lists.getByCid(req.params.listId, (err, list) => {
         if (err) {
-            res.status(500);
+            return handleErrorResponse(res, log, err);
-            return res.json({
-                error: err.message || err,
-                data: []
-            });
         }
         if (!list) {
-            res.status(404);
+            return handleErrorResponse(res, log, false, 404, 'Selected listId not found');
-            return res.json({
-                error: 'Selected listId not found',
-                data: []
-            });
         }
         if (!input.EMAIL) {
-            res.status(400);
+            return handleErrorResponse(res, log, false, 400, 'Missing EMAIL');
-            return res.json({
-                error: 'Missing EMAIL',
-                data: []
-            });
         }
         subscriptions.getByEmail(list.id, input.EMAIL, (err, subscription) => {
             if (err) {
-                res.status(500);
+                return handleErrorResponse(res, log, err);
-                return res.json({
-                    error: err.message || err,
-                    data: []
-                });
             }
             if (!subscription) {
-                res.status(404);
+                return handleErrorResponse(res, log, false, 404, 'Subscription not found');
-                return res.json({
-                    error: 'Subscription not found',
-                    data: []
-                });
             }
             subscriptions.delete(list.id, subscription.cid, (err, subscription) => {
                 if (err) {
-                    res.status(500);
+                    return handleErrorResponse(res, log, err);
-                    return res.json({
-                        error: err.message || err,
-                        data: []
-                    });
                 }
                 if (!subscription) {
-                    res.status(404);
+                    return handleErrorResponse(res, log, false, 404, 'Subscription not found');
-                    return res.json({
-                        error: 'Subscription not found',
-                        data: []
-                    });
                 }
                 res.status(200);
                 res.json({
@@ -321,19 +233,11 @@ router.get('/subscriptions/:listId', (req, res) => {
 
     lists.getByCid(req.params.listId, (err, list) => {
         if (err) {
-            res.status(500);
+            return handleErrorResponse(res, log, err);
-            return res.json({
-		error: err.message || err,
-		data: []
-            });
         }
         subscriptions.list(list.id, start, limit, (err, rows, total) => {
             if (err) {
-		res.status(500);
+                return handleErrorResponse(res, log, err);
-		return res.json({
-		    error: err.message || err,
-		    data: []
-		});
             }
             res.status(200);
             res.json({
@@ -351,11 +255,7 @@ router.get('/subscriptions/:listId', (req, res) => {
 router.get('/lists', (req, res) => {
     lists.quicklist((err, lists) => {
         if (err) {
-            res.status(500);
+            return handleErrorResponse(res, log, err);
-            return res.json({
-                error: err.message || err,
-                data: []
-            });
         }
         res.status(200);
         res.json({
@@ -367,10 +267,7 @@ router.get('/lists', (req, res) => {
 router.get('/list/:id', (req, res) => {
     lists.get(req.params.id, (err, list) => {
         if (err) {
-            res.status(500);
+            return handleErrorResponse(res, log, err);
-            return res.json({
-                error: err.message || err,
-            });
         }
         res.status(200);
         res.json({
@@ -382,11 +279,7 @@ router.get('/list/:id', (req, res) => {
 router.get('/lists/:email', (req, res) => {
     lists.getListsWithEmail(req.params.email, (err, lists) => {
         if (err) {
-            res.status(500);
+            return handleErrorResponse(res, log, err);
-		    return res.json({
-		        error: err.message || err,
-		        data: []
-		    });
         }
         res.status(200);
         res.json({
@@ -402,19 +295,10 @@ router.post('/field/:listId', (req, res) => {
     });
     lists.getByCid(req.params.listId, (err, list) => {
         if (err) {
-            log.error('API', err);
+            return handleErrorResponse(res, log, err);
-            res.status(500);
-            return res.json({
-                error: err.message || err,
-                data: []
-            });
         }
         if (!list) {
-            res.status(404);
+            return handleErrorResponse(res, log, false, 404, 'Selected listId not found');
-            return res.json({
-                error: 'Selected listId not found',
-                data: []
-            });
         }
 
         let field = {
@@ -429,11 +313,7 @@ router.post('/field/:listId', (req, res) => {
 
         fields.create(list.id, field, (err, id, tag) => {
             if (err) {
-                res.status(500);
+                return handleErrorResponse(res, log, err);
-                return res.json({
-                    error: err.message || err,
-                    data: []
-                });
             }
             res.status(200);
             res.json({
@@ -452,19 +332,11 @@ router.post('/blacklist/add', (req, res) => {
         input[(key || '').toString().trim().toUpperCase()] = (req.body[key] || '').toString().trim();
     });
     if (!(input.EMAIL) || (input.EMAIL === ''))  {
-      res.status(500);
+        return handleErrorResponse(res, log, err);
-      return res.json({
-          error: 'EMAIL argument are required',
-          data: []
-      });
     }
     blacklist.add(input.EMAIL, (err) =>{
         if (err) {
-          res.status(500);
+            return handleErrorResponse(res, log, err);
-          return res.json({
-              error: err.message || err,
-              data: []
-          });
         }
         res.status(200);
         res.json({
@@ -479,19 +351,11 @@ router.post('/blacklist/delete', (req, res) => {
         input[(key || '').toString().trim().toUpperCase()] = (req.body[key] || '').toString().trim();
     });
     if (!(input.EMAIL) || (input.EMAIL === ''))  {
-      res.status(500);
+        return handleErrorResponse(res, log, false, 500, 'EMAIL argument are required');
-      return res.json({
-          error: 'EMAIL argument are required',
-          data: []
-      });
     }
     blacklist.delete(input.EMAIL, (err) =>{
       if (err) {
-          res.status(500);
+          return handleErrorResponse(res, log, err);
-          return res.json({
-              error: err.message || err,
-              data: []
-          });
       }
       res.status(200);
       res.json({
@@ -507,11 +371,7 @@ router.get('/blacklist/get', (req, res) => {
 
     blacklist.get(start, limit, search, (err, data, total) => {
       if (err) {
-          res.status(500);
+          return handleErrorResponse(res, log, err);
-          return res.json({
-              error: err.message || err,
-              data: []
-          });
       }
       res.status(200);
       res.json({
@@ -531,92 +391,47 @@ router.post('/changeemail/:listId', (req, res) => {
         input[(key || '').toString().trim().toUpperCase()] = (req.body[key] || '').toString().trim();
     });
     if (!(input.EMAILOLD) || (input.EMAILOLD === ''))  {
-      res.status(500);
+        return handleErrorResponse(res, log, false, 500, 'EMAILOLD argument is required');
-      return res.json({
-          error: 'EMAILOLD argument is required',
-          data: []
-      });
     }
     if (!(input.EMAILNEW) || (input.EMAILNEW === ''))  {
-      res.status(500);
+        return handleErrorResponse(res, log, false, 500, 'EMAILNEW argument is required');
-      return res.json({
-          error: 'EMAILNEW argument is required',
-          data: []
-      });
     }
     lists.getByCid(req.params.listId, (err, list) => {
         if (err) {
-            log.error('API', err);
+            return handleErrorResponse(res, log, err);
-            res.status(500);
-            return res.json({
-                error: err.message || err,
-                data: []
-            });
         }
         if (!list) {
-            res.status(404);
+            return handleErrorResponse(res, log, false, 404, 'Selected listId not found');
-            return res.json({
-                error: 'Selected listId not found',
-                data: []
-            });
         }
-        blacklist.isblacklisted(input.EMAILNEW, (err, blacklisted) =>{
+        blacklist.isblacklisted(input.EMAILNEW, (err, blacklisted) => {
             if (err) {
-              res.status(500);
+                return handleErrorResponse(res, log, err);
-              return res.json({
-                  error: err.message || err,
-                  data: []
-              });
             }
             if (blacklisted) {
-            res.status(500);
+                return handleErrorResponse(res, log, false, 500, 'New email is blacklisted');
-            return res.json({
-                error: 'New email is blacklisted',
-                data: []
-            });
             }
 
             subscriptions.getByEmail(list.id, input.EMAILOLD, (err, subscription) => {
                 if (err) {
-                  res.status(500);
+                    return handleErrorResponse(res, log, err);
-                  return res.json({
-                      error: err.message || err,
-                      data: []
-                  });
                 }
 
                 if (!subscription) {
-                  res.status(404);
+                    return handleErrorResponse(res, log, false, 404, 'Subscription with given old email not found');
-                  return res.json({
-                      error: 'Subscription with given old email not found',
-                      data: []
-                  });
                 }
 
                 subscriptions.updateAddressCheck(list, subscription.cid, input.EMAILNEW, null, (err, old, valid) => {
                     if (err) {
-                      res.status(500);
+                        return handleErrorResponse(res, log, err);
-                      return res.json({
-                          error: err.message || err,
-                          data: []
-                      });
                     }
 
                     if (!valid) {
-                      res.status(500);
+                        return handleErrorResponse(res, log, false, 500, 'New email not valid');
-                      return res.json({
-                          error: 'New email not valid',
-                          data: []
-                      });
                     }
 
                     subscriptions.updateAddress(list.id, subscription.id, input.EMAILNEW, (err) => {
                         if (err) {
-                          res.status(500);
+                            return handleErrorResponse(res, log, err);
-                          return res.json({
-                              error: err.message || err,
-                              data: []
-                          });
                         }
                         res.status(200);
                         res.json({