From 8425cb894d4867d26fd5f7fae7862b0669f3c717 Mon Sep 17 00:00:00 2001
From: Chocobozzz <florian.bigard@gmail.com>
Date: Fri, 5 Feb 2016 19:02:05 +0100
Subject: [PATCH] Error handling mini refractoring

---
 controllers/api/v1/pods.js        |  7 ++---
 controllers/api/v1/videos.js      |  4 +--
 helpers/peertubeCrypto.js         | 12 +++------
 helpers/utils.js                  |  2 +-
 lib/friends.js                    | 24 ++++++++++-------
 lib/poolRequests.js               | 43 +++++++++++++++++++++----------
 lib/videos.js                     |  4 +--
 lib/webtorrent.js                 |  2 +-
 middlewares/reqValidators/pods.js |  5 +++-
 models/pods.js                    |  2 +-
 models/poolRequests.js            | 19 +++++++++-----
 models/videos.js                  | 20 +++++++-------
 12 files changed, 86 insertions(+), 58 deletions(-)

diff --git a/controllers/api/v1/pods.js b/controllers/api/v1/pods.js
index 29517ba8e..456f53dea 100644
--- a/controllers/api/v1/pods.js
+++ b/controllers/api/v1/pods.js
@@ -39,13 +39,13 @@
 
       fs.readFile(peertubeCrypto.getCertDir() + 'peertube.pub', 'utf8', function (err, cert) {
         if (err) {
-          logger.error('Cannot read cert file.', { error: err })
+          logger.error('Cannot read cert file.')
           return next(err)
         }
 
         Videos.listOwned(function (err, videos_list) {
           if (err) {
-            logger.error('Cannot get the list of owned videos.', { error: err })
+            logger.error('Cannot get the list of owned videos.')
             return next(err)
           }
 
@@ -78,7 +78,8 @@
 
       Videos.removeAllRemotesOf(url, function (err) {
         if (err) logger.error('Cannot remove all remote videos of %s.', url)
-        logger.info('%s pod removed.', url)
+        else logger.info('%s pod removed.', url)
+
         res.sendStatus(204)
       })
     })
diff --git a/controllers/api/v1/videos.js b/controllers/api/v1/videos.js
index 7792059ca..eec95c801 100644
--- a/controllers/api/v1/videos.js
+++ b/controllers/api/v1/videos.js
@@ -56,7 +56,7 @@
 
     videos.seed(video_file.path, function (err, torrent) {
       if (err) {
-        logger.error('Cannot seed this video.', { error: err })
+        logger.error('Cannot seed this video.')
         return next(err)
       }
 
@@ -70,7 +70,7 @@
       Videos.add(video_data, function (err) {
         if (err) {
           // TODO unseed the video
-          logger.error('Cannot insert this video in the database.', { error: err })
+          logger.error('Cannot insert this video in the database.')
           return next(err)
         }
 
diff --git a/helpers/peertubeCrypto.js b/helpers/peertubeCrypto.js
index 3e757659b..36271dba9 100644
--- a/helpers/peertubeCrypto.js
+++ b/helpers/peertubeCrypto.js
@@ -101,7 +101,7 @@
       logger.info('Generating a RSA key...')
       openssl.exec('genrsa', { 'out': certDir + 'peertube.key.pem', '2048': false }, function (err) {
         if (err) {
-          logger.error('Cannot create private key on this pod.', { error: err })
+          logger.error('Cannot create private key on this pod.')
           return callback(err)
         }
         logger.info('RSA key generated.')
@@ -109,7 +109,7 @@
         logger.info('Manage public key...')
         openssl.exec('rsa', { 'in': certDir + 'peertube.key.pem', 'pubout': true, 'out': certDir + 'peertube.pub' }, function (err) {
           if (err) {
-            logger.error('Cannot create public key on this pod .', { error: err })
+            logger.error('Cannot create public key on this pod.')
             return callback(err)
           }
 
@@ -122,9 +122,7 @@
 
   function generatePassword (callback) {
     crypto.randomBytes(32, function (err, buf) {
-      if (err) {
-        return callback(err)
-      }
+      if (err) return callback(err)
 
       callback(null, buf.toString('utf8'))
     })
@@ -139,9 +137,7 @@
 
   function symetricEncrypt (text, callback) {
     generatePassword(function (err, password) {
-      if (err) {
-        return callback(err)
-      }
+      if (err) return callback(err)
 
       var cipher = crypto.createCipher(algorithm, password)
       var crypted = cipher.update(text, 'utf8', 'hex')
diff --git a/helpers/utils.js b/helpers/utils.js
index bd0557977..92684ea81 100644
--- a/helpers/utils.js
+++ b/helpers/utils.js
@@ -8,7 +8,7 @@
   }
 
   function cleanForExit (webtorrent_process) {
-    logger.info('Gracefully exiting')
+    logger.info('Gracefully exiting.')
     process.kill(-webtorrent_process.pid)
   }
 
diff --git a/lib/friends.js b/lib/friends.js
index b0086a38b..badf09c7d 100644
--- a/lib/friends.js
+++ b/lib/friends.js
@@ -30,8 +30,7 @@
   function addVideoToFriends (video) {
     // To avoid duplicates
     var id = video.name + video.magnetUri
-    // namePath is null
-    // TODO
+    // ensure namePath is null
     video.namePath = null
     PoolRequests.addRequest(id, 'add', video)
   }
@@ -51,13 +50,15 @@
     logger.info('Make friends!')
     fs.readFile(peertubeCrypto.getCertDir() + 'peertube.pub', 'utf8', function (err, cert) {
       if (err) {
-        logger.error('Cannot read public cert.', { error: err })
+        logger.error('Cannot read public cert.')
         return callback(err)
       }
 
       var urls = config.get('network.friends')
 
-      async.each(urls, computeForeignPodsList, function () {
+      async.each(urls, computeForeignPodsList, function (err) {
+        if (err) return callback(err)
+
         logger.debug('Pods scores computed.', { pods_score: pods_score })
         var pods_list = computeWinningPods(urls, pods_score)
         logger.debug('Pods that we keep computed.', { pods_to_keep: pods_list })
@@ -72,7 +73,8 @@
       // Let's give 1 point to the pod we ask the friends list
       pods_score[url] = 1
 
-      getForeignPodsList(url, function (foreign_pods_list) {
+      getForeignPodsList(url, function (err, foreign_pods_list) {
+        if (err) return callback(err)
         if (foreign_pods_list.length === 0) return callback()
 
         async.each(foreign_pods_list, function (foreign_pod, callback_each) {
@@ -108,7 +110,10 @@
 
       // Get the list of our videos to send to our new friends
       Videos.listOwned(function (err, videos_list) {
-        if (err) throw err
+        if (err) {
+          logger.error('Cannot get the list of videos we own.')
+          return callback(err)
+        }
 
         var data = {
           url: http + '://' + host + ':' + port,
@@ -145,7 +150,7 @@
             poolRequests.activate()
 
             if (err) {
-              logger.error('There was some errors when we wanted to make friends.', { error: err })
+              logger.error('There was some errors when we wanted to make friends.')
               return callback(err)
             }
 
@@ -212,8 +217,9 @@
     var path = '/api/' + constants.API_VERSION + '/pods'
 
     request.get(url + path, function (err, response, body) {
-      if (err) throw err
-      callback(JSON.parse(body))
+      if (err) return callback(err)
+
+      callback(null, JSON.parse(body))
     })
   }
 })()
diff --git a/lib/poolRequests.js b/lib/poolRequests.js
index f4ab434ad..ccc3489ab 100644
--- a/lib/poolRequests.js
+++ b/lib/poolRequests.js
@@ -44,7 +44,7 @@
     if (!callback) callback = function () {}
 
     Pods.list(function (err, pods) {
-      if (err) throw err
+      if (err) return callback(err)
 
       var params = {
         encrypt: true,
@@ -59,7 +59,7 @@
       } else if (type === 'remove') {
         params.path = '/api/' + constants.API_VERSION + '/remotevideos/remove'
       } else {
-        throw new Error('Unkown pool request type.')
+        return callback(new Error('Unkown pool request type.'))
       }
 
       var bad_pods = []
@@ -91,7 +91,10 @@
     logger.info('Making pool requests to friends.')
 
     PoolRequests.list(function (err, pool_requests) {
-      if (err) throw err
+      if (err) {
+        logger.error('Cannot get the list of pool requests.', { err: err })
+        return // Abort
+      }
 
       if (pool_requests.length === 0) return
 
@@ -114,7 +117,8 @@
           requests_to_make.remove.requests.push(pool_request.request)
           requests_to_make.remove.ids.push(pool_request._id)
         } else {
-          throw new Error('Unkown pool request type.')
+          logger.error('Unkown pool request type.', { request_type: pool_request.type })
+          return // abort
         }
 
         callback_each()
@@ -142,7 +146,10 @@
 
   function removeBadPods () {
     Pods.findBadPods(function (err, pods) {
-      if (err) throw err
+      if (err) {
+        logger.error('Cannot find bad pods.', { error: err })
+        return // abort
+      }
 
       if (pods.length === 0) return
 
@@ -150,15 +157,20 @@
       var ids = pluck(pods, '_id')
 
       Videos.removeAllRemotesOf(urls, function (err, r) {
-        if (err) logger.error('Cannot remove videos from a pod that we removing.', { error: err })
-        var videos_removed = r.result.n
-        logger.info('Removed %d videos.', videos_removed)
+        if (err) {
+          logger.error('Cannot remove videos from a pod that we removing.', { error: err })
+        } else {
+          var videos_removed = r.result.n
+          logger.info('Removed %d videos.', videos_removed)
+        }
 
         Pods.removeAllByIds(ids, function (err, r) {
-          if (err) logger.error('Cannot remove bad pods.', { error: err })
-
-          var pods_removed = r.result.n
-          logger.info('Removed %d pods.', pods_removed)
+          if (err) {
+            logger.error('Cannot remove bad pods.', { error: err })
+          } else {
+            var pods_removed = r.result.n
+            logger.info('Removed %d pods.', pods_removed)
+          }
         })
       })
     })
@@ -167,9 +179,12 @@
   function updatePodsScore (good_pods, bad_pods) {
     logger.info('Updating %d good pods and %d bad pods scores.', good_pods.length, bad_pods.length)
 
-    Pods.incrementScores(good_pods, constants.PODS_SCORE.BONUS)
+    Pods.incrementScores(good_pods, constants.PODS_SCORE.BONUS, function (err) {
+      if (err) logger.error('Cannot increment scores of good pods.')
+    })
+
     Pods.incrementScores(bad_pods, constants.PODS_SCORE.MALUS, function (err) {
-      if (err) throw err
+      if (err) logger.error('Cannot increment scores of bad pods.')
       removeBadPods()
     })
   }
diff --git a/lib/videos.js b/lib/videos.js
index 5d23070a7..0da7715c4 100644
--- a/lib/videos.js
+++ b/lib/videos.js
@@ -28,14 +28,14 @@
   function seedAllExisting (callback) {
     Videos.listOwned(function (err, videos_list) {
       if (err) {
-        logger.error('Cannot get list of the videos to seed.', { error: err })
+        logger.error('Cannot get list of the videos to seed.')
         return callback(err)
       }
 
       async.each(videos_list, function (video, each_callback) {
         seed(uploadDir + video.namePath, function (err) {
           if (err) {
-            logger.error('Cannot seed this video.', { error: err })
+            logger.error('Cannot seed this video.')
             return callback(err)
           }
 
diff --git a/lib/webtorrent.js b/lib/webtorrent.js
index d1ca3c9f2..d0db6e066 100644
--- a/lib/webtorrent.js
+++ b/lib/webtorrent.js
@@ -62,7 +62,7 @@
       try {
         wt.remove(magnetUri, callback)
       } catch (err) {
-        console.log('Cannot remove the torrent from WebTorrent')
+        console.log('Cannot remove the torrent from WebTorrent.')
         return callback(null)
       }
 
diff --git a/middlewares/reqValidators/pods.js b/middlewares/reqValidators/pods.js
index 499cafd8f..4d649b486 100644
--- a/middlewares/reqValidators/pods.js
+++ b/middlewares/reqValidators/pods.js
@@ -12,7 +12,10 @@
 
   function makeFriends (req, res, next) {
     friends.hasFriends(function (err, has_friends) {
-      if (err) return next(err)
+      if (err) {
+        logger.error('Cannot know if we have friends.', { error: err })
+        res.sendStatus(500)
+      }
 
       if (has_friends === true) {
         // We need to quit our friends before make new ones
diff --git a/models/pods.js b/models/pods.js
index 395b1e0b1..04fd042c3 100644
--- a/models/pods.js
+++ b/models/pods.js
@@ -61,7 +61,7 @@
   function list (callback) {
     PodsDB.find(function (err, pods_list) {
       if (err) {
-        logger.error('Cannot get the list of the pods.', { error: err })
+        logger.error('Cannot get the list of the pods.')
         return callback(err)
       }
 
diff --git a/models/poolRequests.js b/models/poolRequests.js
index 0f488ef04..962e75e6a 100644
--- a/models/poolRequests.js
+++ b/models/poolRequests.js
@@ -26,21 +26,28 @@
     logger.debug('Add request to the pool requests.', { id: id, type: type, request: request })
 
     PoolRequestsDB.findOne({ id: id }, function (err, entity) {
-      if (err) logger.error(err)
+      if (err) {
+        logger.error('Cannot find one pool request.', { error: err })
+        return // Abort
+      }
 
       if (entity) {
         if (entity.type === type) {
-          logger.error(new Error('Cannot insert two same requests.'))
-          return
+          logger.error('Cannot insert two same requests.')
+          return // Abort
         }
 
         // Remove the request of the other type
         PoolRequestsDB.remove({ id: id }, function (err) {
-          if (err) logger.error(err)
+          if (err) {
+            logger.error('Cannot remove a pool request.', { error: err })
+            return // Abort
+          }
         })
       } else {
         PoolRequestsDB.create({ id: id, type: type, request: request }, function (err) {
-          if (err) logger.error(err)
+          logger.error('Cannot create a pool request.', { error: err })
+          return // Abort
         })
       }
     })
@@ -54,7 +61,7 @@
     PoolRequestsDB.remove({ _id: { $in: ids } }, function (err) {
       if (err) {
         logger.error('Cannot remove requests from the pool requests database.', { error: err })
-        return
+        return // Abort
       }
 
       logger.info('Pool requests flushed.')
diff --git a/models/videos.js b/models/videos.js
index 10abee6e7..6ea628373 100644
--- a/models/videos.js
+++ b/models/videos.js
@@ -50,7 +50,7 @@
 
     VideosDB.create(params, function (err, video) {
       if (err) {
-        logger.error('Cannot insert this video into database.', { error: err })
+        logger.error('Cannot insert this video into database.')
         return callback(err)
       }
 
@@ -82,7 +82,7 @@
     }, function () {
       VideosDB.create(to_add, function (err, videos) {
         if (err) {
-          logger.error('Cannot insert this remote video.', { error: err })
+          logger.error('Cannot insert this remote video.')
           return callback(err)
         }
 
@@ -94,7 +94,7 @@
   function get (id, callback) {
     VideosDB.findById(id, function (err, video) {
       if (err) {
-        logger.error('Cannot get this video.', { error: err })
+        logger.error('Cannot get this video.')
         return callback(err)
       }
 
@@ -120,14 +120,14 @@
     VideosDB.findById(id, function (err, video) {
       if (err || !video) {
         if (!err) err = new Error('Cannot find this video.')
-        logger.error('Cannot find this video.', { error: err })
+        logger.error('Cannot find this video.')
         return callback(err)
       }
 
       if (video.namePath === null) {
         var error_string = 'Cannot remove the video of another pod.'
         logger.error(error_string)
-        return callback(null, false, video)
+        return callback(new Error(error_string), false, video)
       }
 
       callback(null, true, video)
@@ -137,7 +137,7 @@
   function list (callback) {
     VideosDB.find(function (err, videos_list) {
       if (err) {
-        logger.error('Cannot get list of the videos.', { error: err })
+        logger.error('Cannot get the list of the videos.')
         return callback(err)
       }
 
@@ -149,7 +149,7 @@
     // If namePath is not null this is *our* video
     VideosDB.find({ namePath: { $ne: null } }, function (err, videos_list) {
       if (err) {
-        logger.error('Cannot get list of the videos.', { error: err })
+        logger.error('Cannot get the list of owned videos.')
         return callback(err)
       }
 
@@ -160,13 +160,13 @@
   function removeOwned (id, callback) {
     VideosDB.findByIdAndRemove(id, function (err, video) {
       if (err) {
-        logger.error('Cannot remove the torrent.', { error: err })
+        logger.error('Cannot remove the torrent.')
         return callback(err)
       }
 
       fs.unlink(uploadDir + video.namePath, function (err) {
         if (err) {
-          logger.error('Cannot remove this video file.', { error: err })
+          logger.error('Cannot remove this video file.')
           return callback(err)
         }
 
@@ -222,7 +222,7 @@
   function search (name, callback) {
     VideosDB.find({ name: new RegExp(name) }, function (err, videos) {
       if (err) {
-        logger.error('Cannot search the videos.', { error: err })
+        logger.error('Cannot search the videos.')
         return callback(err)
       }
 
-- 
2.41.0