From 843757fcf56f8a333d6839058a5fcd12600f5f01 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?H=C3=BCseyin=20Berke=20B=C3=BCt=C3=BCn?=
 <8071263+Zamion101@users.noreply.github.com>
Date: Thu, 22 Dec 2022 17:51:53 +0100
Subject: [PATCH] fix: Wrong error handler middleware position

Error handler is a middleware that captures errors from throw and
next(...), so in order to be able to handle it needs to come after
all of the routing and other middlewares. Previously it was before
routuing logic and wasn't working, now it works as intended.

Also added check for LogLevel to Sentry for not sending false-positive
errors like `BadRequestError`, `UnauthorizedRequestError` and etc.
---
 backend/src/app.ts | 52 +++++++++++++++++++++++++++++-----------------
 1 file changed, 33 insertions(+), 19 deletions(-)

diff --git a/backend/src/app.ts b/backend/src/app.ts
index c043eb344..d1c13e2a5 100644
--- a/backend/src/app.ts
+++ b/backend/src/app.ts
@@ -30,8 +30,8 @@ import {
   integrationAuth as integrationAuthRouter
 } from './routes';
 import { getLogger } from './utils/logger';
-import RequestError from './utils/requestError';
-import { InternalServerError } from './utils/errors';
+import RequestError, { LogLevel } from './utils/requestError';
+import { InternalServerError, RouteNotFoundError } from './utils/errors';
 
 export const app = express();
 
@@ -53,23 +53,6 @@ if (NODE_ENV === 'production') {
   app.use(helmet());
 }
 
-//* Error Handling Middleware
-app.use((error: RequestError|Error, req: Request, res: Response, next: NextFunction)=>{
-  if(res.headersSent) return next();
-  
-  if(!(error instanceof RequestError)){
-      error = InternalServerError({context: {exception: error.message}, stack: error.stack})
-      getLogger('backend-main').log((<RequestError>error).levelName.toLowerCase(), (<RequestError>error).message)
-  }
-  //* Sentry Error Capture
-  if(req.user !== undefined || req.user !== null)
-    Sentry.setUser({ email: req.user.email })
-  Sentry.captureException(error)
-  
-  res.status((<RequestError>error).statusCode).json((<RequestError>error).format(req))
-  next()
-})
-
 
 // routers
 app.use('/api/v1/signup', signupRouter);
@@ -90,6 +73,37 @@ app.use('/api/v1/stripe', stripeRouter);
 app.use('/api/v1/integration', integrationRouter);
 app.use('/api/v1/integration-auth', integrationAuthRouter);
 
+
+//* Handle unrouted requests and respond with proper error message as well as status code
+app.use((req, res, next)=>{
+  if(res.headersSent) return next();
+  next(RouteNotFoundError({message: `The requested source '(${req.method})${req.url}' was not found`}))
+})
+
+//* Error Handling Middleware (must be after all routing logic)
+app.use((error: RequestError|Error, req: Request, res: Response, next: NextFunction)=>{
+  if(res.headersSent) return next();
+  //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 RequestError)){
+      error = InternalServerError({context: {exception: error.message}, stack: error.stack})
+      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.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)
+  }
+  
+  res.status((<RequestError>error).statusCode).json((<RequestError>error).format(req))
+  next()
+})
+
+
 export const server = app.listen(PORT, () => {
   getLogger("backend-main").info(`Server started listening at port ${PORT}`)
 });