From ae31033c0cdb8ed677c010a63df6964dd0a9efe1 Mon Sep 17 00:00:00 2001
From: Bobby Wibowo <bobbywibowo98@gmail.com>
Date: Fri, 8 Jan 2021 07:29:14 +0700
Subject: [PATCH] refactor: init UserError, a custom Error object

This will be used for errors that are to be delivered to users, AND not
to be logged into the server (as in it stacktraces and all).
This will eventually remove the need to throw string literals.

In this commit, this has only been implemented on albumsController.js,
but more will soon to come.
---
 controllers/albumsController.js          | 219 +++++++++++------------
 controllers/handlers/apiErrorsHandler.js |  30 ++++
 controllers/utils/UserError.js           |  11 ++
 3 files changed, 147 insertions(+), 113 deletions(-)
 create mode 100644 controllers/handlers/apiErrorsHandler.js
 create mode 100644 controllers/utils/UserError.js

diff --git a/controllers/albumsController.js b/controllers/albumsController.js
index fb2364c..3322b9d 100644
--- a/controllers/albumsController.js
+++ b/controllers/albumsController.js
@@ -7,6 +7,8 @@ const paths = require('./pathsController')
 const perms = require('./permissionController')
 const uploadController = require('./uploadController')
 const utils = require('./utilsController')
+const apiErrorsHandler = require('./handlers/apiErrorsHandler.js')
+const UserError = require('./utils/UserError')
 const config = require('./../config')
 const logger = require('./../logger')
 const db = require('knex')(config.database)
@@ -66,28 +68,28 @@ self.getUniqueRandomName = async () => {
     return identifier
   }
 
-  throw 'Sorry, we could not allocate a unique random identifier. Try again?'
+  throw new UserError('Failed to allocate a unique identifier for the album. Try again?', 500)
 }
 
 self.list = async (req, res, next) => {
-  const user = await utils.authorize(req, res)
-  if (!user) return
-
-  const all = req.headers.all === '1'
-  const simple = req.headers.simple
-  const ismoderator = perms.is(user, 'moderator')
-  if (all && !ismoderator) return res.status(403).end()
-
-  const filter = function () {
-    if (!all) {
-      this.where({
-        enabled: 1,
-        userid: user.id
-      })
-    }
-  }
-
   try {
+    const user = await utils.authorize(req, res)
+    if (!user) return
+
+    const all = req.headers.all === '1'
+    const simple = req.headers.simple
+    const ismoderator = perms.is(user, 'moderator')
+    if (all && !ismoderator) return res.status(403).end()
+
+    const filter = function () {
+      if (!all) {
+        this.where({
+          enabled: 1,
+          userid: user.id
+        })
+      }
+    }
+
     // Query albums count for pagination
     const count = await db.table('albums')
       .where(filter)
@@ -164,22 +166,21 @@ self.list = async (req, res, next) => {
 
     return res.json({ success: true, albums, count, users, homeDomain })
   } catch (error) {
-    logger.error(error)
-    return res.status(500).json({ success: false, description: 'An unexpected error occurred. Try again?' })
+    return apiErrorsHandler(error, req, res, next)
   }
 }
 
 self.create = async (req, res, next) => {
-  const user = await utils.authorize(req, res)
-  if (!user) return
-
-  const name = typeof req.body.name === 'string'
-    ? utils.escape(req.body.name.trim().substring(0, self.titleMaxLength))
-    : ''
-
-  if (!name) return res.json({ success: false, description: 'No album name specified.' })
-
   try {
+    const user = await utils.authorize(req, res)
+    if (!user) return
+
+    const name = typeof req.body.name === 'string'
+      ? utils.escape(req.body.name.trim().substring(0, self.titleMaxLength))
+      : ''
+
+    if (!name) return res.json({ success: false, description: 'No album name specified.' })
+
     const album = await db.table('albums')
       .where({
         name,
@@ -211,8 +212,7 @@ self.create = async (req, res, next) => {
 
     return res.json({ success: true, id: ids[0] })
   } catch (error) {
-    logger.error(error)
-    return res.status(500).json({ success: false, description: 'An unexpected error occurred. Try again?' })
+    return apiErrorsHandler(error, req, res, next)
   }
 }
 
@@ -222,14 +222,14 @@ self.delete = async (req, res, next) => {
 }
 
 self.disable = async (req, res, next) => {
-  const user = await utils.authorize(req, res)
-  if (!user) return
-
-  const id = req.body.id
-  const purge = req.body.purge
-  if (!Number.isFinite(id)) return res.json({ success: false, description: 'No album specified.' })
-
   try {
+    const user = await utils.authorize(req, res)
+    if (!user) return
+
+    const id = req.body.id
+    const purge = req.body.purge
+    if (!Number.isFinite(id)) return res.json({ success: false, description: 'No album specified.' })
+
     if (purge) {
       const files = await db.table('files')
         .where({
@@ -264,43 +264,39 @@ self.disable = async (req, res, next) => {
       .then(row => row.identifier)
 
     await paths.unlink(path.join(paths.zips, `${identifier}.zip`))
+    return res.json({ success: true })
   } catch (error) {
-    if (error && error.code !== 'ENOENT') {
-      logger.error(error)
-      return res.status(500).json({ success: false, description: 'An unexpected error occurred. Try again?' })
-    }
+    return apiErrorsHandler(error, req, res, next)
   }
-
-  return res.json({ success: true })
 }
 
 self.edit = async (req, res, next) => {
-  const user = await utils.authorize(req, res)
-  if (!user) return
-
-  const ismoderator = perms.is(user, 'moderator')
-
-  const id = parseInt(req.body.id)
-  if (isNaN(id)) return res.json({ success: false, description: 'No album specified.' })
-
-  const name = typeof req.body.name === 'string'
-    ? utils.escape(req.body.name.trim().substring(0, self.titleMaxLength))
-    : ''
-
-  if (!name) return res.json({ success: false, description: 'No name specified.' })
-
-  const filter = function () {
-    this.where('id', id)
-
-    if (!ismoderator) {
-      this.andWhere({
-        enabled: 1,
-        userid: user.id
-      })
-    }
-  }
-
   try {
+    const user = await utils.authorize(req, res)
+    if (!user) return
+
+    const ismoderator = perms.is(user, 'moderator')
+
+    const id = parseInt(req.body.id)
+    if (isNaN(id)) return res.json({ success: false, description: 'No album specified.' })
+
+    const name = typeof req.body.name === 'string'
+      ? utils.escape(req.body.name.trim().substring(0, self.titleMaxLength))
+      : ''
+
+    if (!name) return res.json({ success: false, description: 'No name specified.' })
+
+    const filter = function () {
+      this.where('id', id)
+
+      if (!ismoderator) {
+        this.andWhere({
+          enabled: 1,
+          userid: user.id
+        })
+      }
+    }
+
     const album = await db.table('albums')
       .where(filter)
       .first()
@@ -358,8 +354,7 @@ self.edit = async (req, res, next) => {
       return res.json({ success: true, name })
     }
   } catch (error) {
-    logger.error(error)
-    return res.status(500).json({ success: false, description: 'An unexpected error occurred. Try again?' })
+    return apiErrorsHandler(error, req, res, next)
   }
 }
 
@@ -370,12 +365,12 @@ self.rename = async (req, res, next) => {
 }
 
 self.get = async (req, res, next) => {
-  const identifier = req.params.identifier
-  if (identifier === undefined) {
-    return res.status(401).json({ success: false, description: 'No identifier provided.' })
-  }
-
   try {
+    const identifier = req.params.identifier
+    if (identifier === undefined) {
+      return res.status(401).json({ success: false, description: 'No identifier provided.' })
+    }
+
     const album = await db.table('albums')
       .where({
         identifier,
@@ -416,30 +411,29 @@ self.get = async (req, res, next) => {
       files
     })
   } catch (error) {
-    logger.error(error)
-    return res.status(500).json({ success: false, description: 'An unexpected error occcured. Try again?' })
+    return apiErrorsHandler(error, req, res, next)
   }
 }
 
 self.generateZip = async (req, res, next) => {
-  const versionString = parseInt(req.query.v)
-
-  const identifier = req.params.identifier
-  if (identifier === undefined) {
-    return res.status(401).json({
-      success: false,
-      description: 'No identifier provided.'
-    })
-  }
-
-  if (!config.uploads.generateZips) {
-    return res.status(401).json({
-      success: false,
-      description: 'ZIP generation disabled.'
-    })
-  }
-
   try {
+    const versionString = parseInt(req.query.v)
+
+    const identifier = req.params.identifier
+    if (identifier === undefined) {
+      return res.status(401).json({
+        success: false,
+        description: 'No identifier provided.'
+      })
+    }
+
+    if (!config.uploads.generateZips) {
+      return res.status(401).json({
+        success: false,
+        description: 'ZIP generation disabled.'
+      })
+    }
+
     const album = await db.table('albums')
       .where({
         identifier,
@@ -547,8 +541,7 @@ self.generateZip = async (req, res, next) => {
     self.zipEmitters.get(identifier).emit('done', filePath, fileName)
     return res.download(filePath, fileName)
   } catch (error) {
-    logger.error(error)
-    return res.status(500).json({ success: false, description: 'An unexpected error occurred. Try again?' })
+    return apiErrorsHandler(error, req, res, next)
   }
 }
 
@@ -589,20 +582,21 @@ self.listFiles = async (req, res, next) => {
 }
 
 self.addFiles = async (req, res, next) => {
-  const user = await utils.authorize(req, res)
-  if (!user) return
-
-  const ids = req.body.ids
-  if (!Array.isArray(ids) || !ids.length) {
-    return res.json({ success: false, description: 'No files specified.' })
-  }
-
-  let albumid = parseInt(req.body.albumid)
-  if (isNaN(albumid) || albumid < 0) albumid = null
-
-  let failed = []
-  const albumids = []
+  let ids, albumid, failed, albumids
   try {
+    const user = await utils.authorize(req, res)
+    if (!user) return
+
+    ids = req.body.ids
+    if (!Array.isArray(ids) || !ids.length) {
+      return res.json({ success: false, description: 'No files specified.' })
+    }
+
+    albumid = parseInt(req.body.albumid)
+    if (isNaN(albumid) || albumid < 0) albumid = null
+
+    failed = []
+    albumids = []
     if (albumid !== null) {
       const album = await db.table('albums')
         .where('id', albumid)
@@ -646,14 +640,13 @@ self.addFiles = async (req, res, next) => {
 
     return res.json({ success: true, failed })
   } catch (error) {
-    logger.error(error)
-    if (failed.length === ids.length) {
+    if (Array.isArray(failed) && (failed.length === ids.length)) {
       return res.json({
         success: false,
         description: `Could not ${albumid === null ? 'add' : 'remove'} any files ${albumid === null ? 'to' : 'from'} the album.`
       })
     } else {
-      return res.status(500).json({ success: false, description: 'An unexpected error occurred. Try again?' })
+      return apiErrorsHandler(error, req, res, next)
     }
   }
 }
diff --git a/controllers/handlers/apiErrorsHandler.js b/controllers/handlers/apiErrorsHandler.js
new file mode 100644
index 0000000..ebc4259
--- /dev/null
+++ b/controllers/handlers/apiErrorsHandler.js
@@ -0,0 +1,30 @@
+const UserError = require('./../utils/UserError')
+const logger = require('./../../logger')
+
+module.exports = (error, req, res, next) => {
+  if (!res) {
+    return logger.error(new Error('Missing "res" object.'))
+  }
+
+  // Intentional error messages to be delivered to users
+  const isUserError = error instanceof UserError
+
+  // ENOENT or missing file errors, typically harmless, so do not log stacktrace
+  const isENOENTError = error instanceof Error && error.code === 'ENOENT'
+
+  if (!isUserError && !isENOENTError) {
+    logger.error(error)
+  }
+
+  const statusCode = isUserError
+    ? error.statusCode
+    : 500
+
+  const description = isUserError
+    ? error.message
+    : 'An unexpected error occurred. Try again?'
+
+  return res
+    .status(statusCode)
+    .json({ success: false, description })
+}
diff --git a/controllers/utils/UserError.js b/controllers/utils/UserError.js
new file mode 100644
index 0000000..efc57a8
--- /dev/null
+++ b/controllers/utils/UserError.js
@@ -0,0 +1,11 @@
+class UserError extends Error {
+  constructor (message, statusCode) {
+    super(message)
+
+    this.statusCode = statusCode !== undefined
+      ? statusCode
+      : 400
+  }
+}
+
+module.exports = UserError