Merge pull request #71 from bluewave-labs/feat/error-handler

Feat/error handler, resolves #64

README.md

@@ -18,7 +18,8 @@ BlueWave uptime monitoring application
     - <code>POST</code> [/api/v1/monitors](#post-monitors)
     - <code>POST</code> [/api/v1/monitors/delete/{monitorId}](#post-monitors-del-id)
     - <code>POST</code> [/api/v1/monitors/edit/{monitorId}](#post-monitors-edit-id)
-5.  [Contributors](#contributors)
+5.  [Error Handling](#error-handling)
+6.  [Contributors](#contributors)
 
 ---
 
@@ -544,6 +545,40 @@ curl --request POST \
 
 ---
 
+### Error handling {#error-handling}
+
+Errors are returned in a standard format:
+
+`{"success": false, "msg": "No token provided"}`
+
+Errors are handled by error handling middleware and should be thrown with the following parameters
+
+| Name    | Type      | Default                | Notes                                |
+| ------- | --------- | ---------------------- | ------------------------------------ |
+| status  | `integer` | 500                    | Standard HTTP codes                  |
+| message | `string`  | "Something went wrong" | An error message                     |
+| service | `string`  | "Unknown Service"      | Name of service that threw the error |
+
+Example:
+
+```
+const myRoute = async(req, res, next) => {
+  try{
+    const result = myRiskyOperationHere();
+  }
+  catch(error){
+    error.status = 404
+    error.message = "Resource not found"
+    error.service = service name
+    next(error)
+    return;
+  }
+}
+
+```
+
+Errors should not be handled at the controller level and should be left to the middleware to handle.
+
 ## Contributors
 
 <a href="https://github.com/bluewave-labs/bluewave-uptime/graphs/contributors">

Server/controllers/authController.js

@@ -21,35 +21,32 @@ const issueToken = (payload) => {
  * @param {express.Response} res
  * @returns {{success: Boolean, msg: String}}
  */
-const registerController = async (req, res) => {
+const registerController = async (req, res, next) => {
   // joi validation
   try {
     await registerValidation.validateAsync(req.body);
   } catch (error) {
-    return res
-      .status(400)
-      .json({ success: false, msg: error.details[0].message });
+    error.status = 400;
+    error.service = SERVICE_NAME;
+    error.message = error.details[0].message;
+    next(error);
+    return;
   }
 
   // Check if the user exists
   try {
     const isUser = await req.db.getUserByEmail(req, res);
     if (isUser) {
-      logger.error("User already exists", {
-        service: SERVICE_NAME,
-        userId: isUser._id,
-      });
-      return res
-        .status(400)
-        .json({ success: false, msg: "User already exists" });
+      throw new Error("User already exists");
     }
   } catch (error) {
-    logger.error(error.message, { service: SERVICE_NAME });
-    return res.status(500).json({ success: false, msg: error.message });
+    error.service = SERVICE_NAME;
+    next(error);
+    return;
   }
 
+  // Create a new user
   try {
-    // Create a new user
     const newUser = await req.db.insertUser(req, res);
     // TODO: Send an email to user
     // Will add this later
@@ -63,8 +60,8 @@ const registerController = async (req, res) => {
       .status(200)
       .json({ success: true, msg: "User created", data: token });
   } catch (error) {
-    logger.error(error.message, { service: SERVICE_NAME });
-    return res.status(500).json({ success: false, msg: error.message });
+    error.service = SERVICE_NAME;
+    next(error);
   }
 };
 
@@ -76,7 +73,7 @@ const registerController = async (req, res) => {
  * @returns {Promise<Express.Response>}
  * @throws {Error}
  */
-const loginController = async (req, res) => {
+const loginController = async (req, res, next) => {
   try {
     // Validate input
     await loginValidation.validateAsync(req.body);
@@ -104,9 +101,9 @@ const loginController = async (req, res) => {
       .status(200)
       .json({ success: true, msg: "Found user", data: token });
   } catch (error) {
+    error.status = 500;
     // Anything else should be an error
-    logger.error(error.message, { service: SERVICE_NAME });
-    return res.status(500).json({ success: false, msg: error.message });
+    next(error);
   }
 };
 

Server/controllers/monitorController.js

@@ -4,7 +4,6 @@ const {
   monitorValidation,
 } = require("../validation/joi");
 
-const logger = require("../utils/logger");
 const SERVICE_NAME = "monitorController";
 
 /**
@@ -15,13 +14,13 @@ const SERVICE_NAME = "monitorController";
  * @returns {Promise<Express.Response>}
  * @throws {Error}
  */
-const getAllMonitors = async (req, res) => {
+const getAllMonitors = async (req, res, next) => {
   try {
     const monitors = await req.db.getAllMonitors();
     return res.json({ success: true, msg: "Monitors found", data: monitors });
   } catch (error) {
-    logger.error(error.message, { service: SERVICE_NAME });
-    return res.status(500).json({ success: false, msg: error.message });
+    error.service = SERVICE_NAME;
+    next(error);
   }
 };
 
@@ -33,25 +32,28 @@ const getAllMonitors = async (req, res) => {
  * @returns {Promise<Express.Response>}
  * @throws {Error}
  */
-const getMonitorById = async (req, res) => {
-  const { error } = getMonitorByIdValidation.validate(req.params);
-  if (error) {
-    return res
-      .status(422)
-      .json({ success: false, msg: error.details[0].message });
+const getMonitorById = async (req, res, next) => {
+  try {
+    await getMonitorByIdValidation.validateAsync(req.params);
+  } catch (error) {
+    error.status = 422;
+    error.message = error.details[0].message;
+    next(error);
+    return;
   }
 
   try {
     const monitor = await req.db.getMonitorById(req, res);
     if (!monitor) {
-      logger.error("Monitor not found", { service: SERVICE_NAME });
-      return res.status(404).json({ success: false, msg: "Monitor not found" });
+      const error = new Error("Monitor not found");
+      error.status = 404;
+      throw error;
     }
 
     return res.json({ success: true, msg: "Monitor found", data: monitor });
   } catch (error) {
-    logger.error(error.message, { service: SERVICE_NAME });
-    return res.status(500).json({ success: false, msg: error.message });
+    error.service = SERVICE_NAME;
+    next(error);
   }
 };
 
@@ -63,7 +65,7 @@ const getMonitorById = async (req, res) => {
  * @returns {Promise<Express.Response>}
  * @throws {Error}
  */
-const getMonitorsByUserId = async (req, res) => {
+const getMonitorsByUserId = async (req, res, next) => {
   const { error } = getMonitorsByUserIdValidation.validate(req.params);
   if (error) {
     return res
@@ -76,9 +78,9 @@ const getMonitorsByUserId = async (req, res) => {
     const monitors = await req.db.getMonitorsByUserId(req, res);
 
     if (monitors && monitors.length === 0) {
-      return res
-        .status(404)
-        .json({ success: false, msg: "No monitors not found" });
+      const err = new Error("No monitors found");
+      err.status = 404;
+      throw err;
     }
 
     return res.json({
@@ -87,8 +89,8 @@ const getMonitorsByUserId = async (req, res) => {
       data: monitors,
     });
   } catch (error) {
-    logger.error(error.message, { service: SERVICE_NAME });
-    return res.status(500).json({ success: false, msg: error.message });
+    error.service = SERVICE_NAME;
+    next(error);
   }
 };
 
@@ -101,12 +103,15 @@ const getMonitorsByUserId = async (req, res) => {
  * @throws {Error}
  */
 
-const createMonitor = async (req, res) => {
-  const { error } = monitorValidation.validate(req.body);
-  if (error) {
-    return res
-      .status(422)
-      .json({ success: false, msg: error.details[0].message });
+const createMonitor = async (req, res, next) => {
+  try {
+    await monitorValidation.validateAsync(req.body);
+  } catch (error) {
+    error.status = 422;
+    error.service = SERVICE_NAME;
+    error.message = error.details[0].message;
+    next(error);
+    return;
   }
 
   try {
@@ -115,8 +120,8 @@ const createMonitor = async (req, res) => {
       .status(201)
       .json({ success: true, msg: "Monitor created", data: monitor });
   } catch (error) {
-    logger.error(error.message, { service: SERVICE_NAME });
-    return res.status(500).json({ success: false, msg: error.message });
+    error.service = SERVICE_NAME;
+    next(error);
   }
 };
 
@@ -129,15 +134,19 @@ const createMonitor = async (req, res) => {
  * @throws {Error}
  */
 
-const deleteMonitor = async (req, res) => {
-  const { error } = getMonitorByIdValidation.validate(req.params);
-  if (error) {
-    return res
-      .status(422)
-      .json({ success: false, msg: error.details[0].message });
-  }
+const deleteMonitor = async (req, res, next) => {
   try {
-    const monitor = await req.db.deleteMonitor(req, res);
+    await getMonitorByIdValidation.validateAsync(req.params);
+  } catch (error) {
+    error.status = 422;
+    error.service = SERVICE_NAME;
+    error.message = error.details[0].message;
+    next(error);
+    return;
+  }
+
+  try {
+    const monitor = await req.db.deleteMonitor(req, res, next);
     /**
      * TODO
      * We should remove all checks and alerts associated with this monitor
@@ -146,8 +155,8 @@ const deleteMonitor = async (req, res) => {
      */
     return res.status(200).json({ success: true, msg: "Monitor deleted" });
   } catch (error) {
-    logger.error(error.message, { service: SERVICE_NAME });
-    return res.status(500).json({ success: false, msg: error.message });
+    error.service = SERVICE_NAME;
+    next(error);
   }
 };
 
@@ -160,18 +169,15 @@ const deleteMonitor = async (req, res) => {
  * @throws {Error}
  */
 const editMonitor = async (req, res, next) => {
-  let { paramError } = getMonitorByIdValidation.validate(req.params);
-  if (paramError) {
-    return res
-      .status(422)
-      .json({ success: false, msg: paramError.error.details[0].message });
-  }
-
-  let { error } = monitorValidation.validate(req.body);
-  if (error) {
-    return res
-      .status(422)
-      .json({ success: false, msg: error.details[0].message });
+  try {
+    await getMonitorByIdValidation.validateAsync(req.params);
+    await monitorValidation.validateAsync(req.body);
+  } catch (error) {
+    error.status = 422;
+    error.service = SERVICE_NAME;
+    error.message = error.details[0].message;
+    next(error);
+    return;
   }
 
   try {
@@ -180,8 +186,8 @@ const editMonitor = async (req, res, next) => {
       .status(200)
       .json({ success: true, msg: "Monitor edited", data: editedMonitor });
   } catch (error) {
-    logger.error(error.message, { service: SERVICE_NAME });
-    return res.status(500).json({ success: false, msg: error.message });
+    error.service = SERVICE_NAME;
+    next(error);
   }
 };
 

Server/index.js

@@ -7,6 +7,7 @@ const { connectDbAndRunServer } = require("./configs/db");
 require("dotenv").config();
 const logger = require("./utils/logger");
 const { verifyJWT } = require("./middleware/verifyJWT");
+const { handleErrors } = require("./middleware/handleErrors");
 
 // const { sendEmail } = require('./utils/sendEmail')
 
@@ -62,6 +63,12 @@ app.use((req, res, next) => {
 app.use("/api/v1/auth", authRouter);
 app.use("/api/v1/monitors", verifyJWT, monitorRouter);
 
+/**
+ * Error handler middleware
+ * MUST be called after all routes
+ */
+app.use(handleErrors);
+
 // Testing email service
 // app.use('/sendEmail', async (req, res) => {
 //     const response = sendEmail(['veysel.boybay@bluewavelabs.ca'], 'Testing email service', '<h1>Testing Bluewavelabs</h1>');

Server/middleware/handleErrors.js

@@ -0,0 +1,12 @@
+const logger = require("../utils/logger");
+
+const handleErrors = (error, req, res, next) => {
+  const status = error.status || 500;
+  const message = error.message || "Something went wrong";
+  const service = error.errorService || "Unknown service";
+
+  logger.error(error.message, { service: service });
+  res.status(status).json({ success: false, msg: message });
+};
+
+module.exports = { handleErrors };

Server/middleware/verifyJWT.js

@@ -15,8 +15,11 @@ const verifyJWT = (req, res, next) => {
   const token = req.headers["authorization"];
   // Make sure a token is provided
   if (!token) {
-    logger.error("No token provided", { service: SERVICE_NAME });
-    return res.status(401).json({ success: false, msg: "No token provided" });
+    const error = new Error("No token provided");
+    error.status = 401;
+    error.service = SERVICE_NAME;
+    next(error);
+    return;
   }
   // Make sure it is properly formatted
   if (token.startsWith(TOKEN_PREFIX)) {
@@ -32,10 +35,9 @@ const verifyJWT = (req, res, next) => {
       next();
     });
   } else {
-    logger.error("Invalid token format", { service: SERVICE_NAME });
-    return res
-      .status(400)
-      .json({ success: false, msg: "Invalid token format" });
+    error.status = 400;
+    error.service = SERVICE_NAME;
+    next(error);
   }
 };
 

Server/middleware/verifyOwnership.js

@@ -12,28 +12,21 @@ const verifyOwnership = (Model, paramName) => {
         logger.error("Document not found", {
           service: SERVICE_NAME,
         });
-        return res
-          .status(404)
-          .json({ success: false, msg: "Document not found" });
+        const error = new Error("Document not found");
+        error.status = 404;
+        throw error;
       }
 
       // If the userID does not match the document's userID, return a 403 error
       if (userId.toString() !== doc.userId.toString()) {
-        logger.error("Unauthorized access", {
-          service: SERVICE_NAME,
-        });
-
-        return res.status(403).json({
-          success: false,
-          msg: "You are not authorized to perform this action",
-        });
+        const error = new Error("Unauthorized access");
+        error.status = 403;
+        throw error;
       }
       next();
     } catch (error) {
-      logger.error(error.message, {
-        service: SERVICE_NAME,
-      });
-      return res.status(500).json({ success: false, msg: error.message });
+      error.service = SERVICE_NAME;
+      next(error);
     }
   };
 };