From b6ef55783eefdd7d3d34c5906d05faf1c3c667ee Mon Sep 17 00:00:00 2001
From: Joel Biddle <joel.sven.biddle@gmail.com>
Date: Mon, 9 Oct 2023 13:37:16 +0000
Subject: [PATCH] Fix: add stack trace errors in logging for prod

---
 backend/.eslintrc                             |  1 +
 backend/src/middleware/requestErrorHandler.ts | 54 +++++++++----------
 backend/src/utils/requestError.ts             | 27 +++++++++-
 3 files changed, 50 insertions(+), 32 deletions(-)

diff --git a/backend/.eslintrc b/backend/.eslintrc
index 6aa49ee31..31bcc259b 100644
--- a/backend/.eslintrc
+++ b/backend/.eslintrc
@@ -26,6 +26,7 @@
     ],
     "@typescript-eslint/no-unused-vars": "off",
     "unused-imports/no-unused-imports": "error",
+    "@typescript-eslint/no-extra-semi": "off", // added to be able to push
     "unused-imports/no-unused-vars": [
       "warn",
       {
diff --git a/backend/src/middleware/requestErrorHandler.ts b/backend/src/middleware/requestErrorHandler.ts
index 2869a0f94..8804a9934 100644
--- a/backend/src/middleware/requestErrorHandler.ts
+++ b/backend/src/middleware/requestErrorHandler.ts
@@ -3,7 +3,7 @@ import { ErrorRequestHandler } from "express";
 import { TokenExpiredError } from "jsonwebtoken";
 import { InternalServerError, UnauthorizedRequestError } from "../utils/errors";
 import { getLogger } from "../utils/logger";
-import RequestError, { LogLevel } from "../utils/requestError";
+import RequestError, { LogLevel, mapToWinstonLogLevel } from "../utils/requestError";
 import { getNodeEnv } from "../config";
 
 export const requestErrorHandler: ErrorRequestHandler = async (
@@ -14,37 +14,31 @@ export const requestErrorHandler: ErrorRequestHandler = async (
 ) => {
   if (res.headersSent) return next();
 
-  if (await getNodeEnv() !== "production") {
-    /* eslint-disable no-console */
-    console.error(error);
-  }
+  const logAndCaptureException = async (error: RequestError, logLevel: LogLevel) => {
+    // log stack trace & error message to the console using Winston
+    (await getLogger("backend-main")).log(mapToWinstonLogLevel(logLevel), `${error.stack}\n${error.message}`);
 
-  //TODO: Find better way to type check for error. In current setting you need to cast type to get the functions and variables from RequestError
-  if (error instanceof TokenExpiredError) {
-    error = UnauthorizedRequestError({ stack: error.stack, message: "Token expired" });
-  } else if (!(error instanceof RequestError)) {
-    error = InternalServerError({
-      context: { exception: error.message },
-      stack: error.stack,
-    });
-    (await getLogger("backend-main")).log(
-      (<RequestError>error).levelName.toLowerCase(),
-      (<RequestError>error).message
-    );
-  }
+    //* Set Sentry user identification if req.user is populated
+    if (req.user !== undefined && req.user !== null) {
+      Sentry.setUser({ email: (req.user as any).email });
+    }
 
-  //* Set Sentry user identification if req.user is populated
-  if (req.user !== undefined && req.user !== null) {
-    Sentry.setUser({ email: (req.user as any).email });
-  }
-  //* Only sent error to Sentry if LogLevel is one of the following level 'ERROR', 'EMERGENCY' or 'CRITICAL'
-  //* with this we will eliminate false-positive errors like 'BadRequestError', 'UnauthorizedRequestError' and so on
-  if (
-    [LogLevel.ERROR, LogLevel.EMERGENCY, LogLevel.CRITICAL].includes(
-      (<RequestError>error).level
-    )
-  ) {
-    Sentry.captureException(error);
+    // eliminate false-positive errors being sent to Sentry
+    if ("level" in error && [LogLevel.ERROR, LogLevel.EMERGENCY, LogLevel.CRITICAL].includes(error.level)) {
+      Sentry.captureException(error);
+    }
+  };
+
+  if (error instanceof RequestError || error instanceof TokenExpiredError) {
+    if (error instanceof TokenExpiredError) {
+      error = UnauthorizedRequestError({ stack: error.stack, message: "Token expired" }) as RequestError;
+    }
+
+    logAndCaptureException((<RequestError>error), LogLevel.INFO);
+  } else {
+    // For unexpected errors, throw a 500 error & ensure these are sent to Sentry in prod
+    error = InternalServerError({ context: { exception: error.message }, stack: error.stack }) as RequestError;
+    logAndCaptureException((<RequestError>error), (await getNodeEnv() === "production") ? LogLevel.ERROR : LogLevel.DEBUG);
   }
 
   res
diff --git a/backend/src/utils/requestError.ts b/backend/src/utils/requestError.ts
index 0c4e093b2..c83f30e52 100644
--- a/backend/src/utils/requestError.ts
+++ b/backend/src/utils/requestError.ts
@@ -1,3 +1,4 @@
+/* eslint-disable @typescript-eslint/no-extra-semi */
 import { Request } from "express"
 import { getVerboseErrorOutput } from "../config";
 
@@ -12,6 +13,27 @@ export enum LogLevel {
     EMERGENCY = 600,
 }
 
+export const mapToWinstonLogLevel = (customLogLevel: LogLevel): string => {
+  switch (customLogLevel) {
+    case LogLevel.DEBUG:
+      return "debug";
+    case LogLevel.INFO:
+      return "info";
+    case LogLevel.NOTICE:
+      return "notice";
+    case LogLevel.WARNING:
+      return "warn";
+    case LogLevel.ERROR:
+      return "error";
+    case LogLevel.CRITICAL:
+      return "crit";
+    case LogLevel.ALERT:
+      return "alert";
+    case LogLevel.EMERGENCY:
+      return "emerg";
+  }
+}
+
 export type RequestErrorContext =  {
 	logLevel?: LogLevel,
 	statusCode: number,
@@ -87,7 +109,8 @@ export default class RequestError extends Error{
         }, this.context)
 
         //* Omit sensitive information from context that can leak internal workings of this program if user is not developer
-        if(!(await getVerboseErrorOutput())){
+        const verboseErrorOutput = await getVerboseErrorOutput();
+        if (verboseErrorOutput !== undefined) {
             _context = this._omit(_context, [
                 "stacktrace",
                 "exception",
@@ -110,4 +133,4 @@ export default class RequestError extends Error{
         return formatObject
 
     }
-}
\ No newline at end of file
+}