From c6192dd24b3a50c80baf9837c6e4205f41f9e36c Mon Sep 17 00:00:00 2001
From: Matteo Pagliazzi <matteopagliazzi@gmail.com>
Date: Thu, 26 May 2016 23:10:38 +0200
Subject: [PATCH] feat(logging): improve logging removing not useful data and
 distinguishing between user and server errors

---
 website/server/libs/api-v3/logger.js          | 26 +++++++++++++++++--
 .../server/middlewares/api-v3/errorHandler.js | 14 +++++-----
 2 files changed, 32 insertions(+), 8 deletions(-)

diff --git a/website/server/libs/api-v3/logger.js b/website/server/libs/api-v3/logger.js
index a5a46fec87..4260e72a48 100644
--- a/website/server/libs/api-v3/logger.js
+++ b/website/server/libs/api-v3/logger.js
@@ -2,6 +2,9 @@
 import winston from 'winston';
 import nconf from 'nconf';
 import _ from 'lodash';
+import {
+  CustomError,
+} from './errors';
 
 const IS_PROD = nconf.get('IS_PROD');
 const IS_TEST = nconf.get('IS_TEST');
@@ -42,8 +45,27 @@ let loggerInterface = {
       // pass the error stack as the first parameter to logger.error
       let stack = err.stack || err.message || err;
 
-      if (_.isPlainObject(errorData) && !errorData.fullError) errorData.fullError = err;
-      logger.error(stack, errorData, ...otherArgs);
+      if (_.isPlainObject(errorData) && !errorData.fullError) {
+        // If the error object has interesting data (not only httpCode, message and name from the CustomError class)
+        // add it to the logs
+        if (err instanceof CustomError) {
+          let errWithoutCommonProps = _.omit(err, ['name', 'httpCode', 'message']);
+
+          if (Object.keys(errWithoutCommonProps).length > 0) {
+            errorData.fullError = errWithoutCommonProps;
+          }
+        } else {
+          errorData.fullError = err;
+        }
+      }
+
+      let loggerArgs = [stack, errorData, ...otherArgs];
+      // Treat 4xx errors as warnings, 5xx and uncaught errors as serious problems
+      if (!errorData || !errorData.statusCode || errorData.statusCode > 499) {
+        logger.error(...loggerArgs);
+      } else {
+        logger.warn(stack, errorData, ...otherArgs);
+      }
     } else {
       logger.error(...args);
     }
diff --git a/website/server/middlewares/api-v3/errorHandler.js b/website/server/middlewares/api-v3/errorHandler.js
index 11d75042e3..891f0b36af 100644
--- a/website/server/middlewares/api-v3/errorHandler.js
+++ b/website/server/middlewares/api-v3/errorHandler.js
@@ -12,12 +12,6 @@ import {
 } from 'lodash';
 
 module.exports = function errorHandler (err, req, res, next) { // eslint-disable-line no-unused-vars
-  logger.error(err, {
-    originalUrl: req.originalUrl,
-    headers: omit(req.headers, ['x-api-key']),
-    body: req.body,
-  });
-
   // In case of a CustomError class, use it's data
   // Otherwise try to identify the type of error (mongoose validation, mongodb unique, ...)
   // If we can't identify it, respond with a generic 500 error
@@ -70,6 +64,14 @@ module.exports = function errorHandler (err, req, res, next) { // eslint-disable
     responseErr = new InternalServerError();
   }
 
+  // log the error
+  logger.error(err, {
+    originalUrl: req.originalUrl,
+    headers: omit(req.headers, ['x-api-key', 'cookie']), // don't send sensitive information that only adds noise
+    body: req.body,
+    statusCode: responseErr.httpCode,
+  });
+
   let jsonRes = {
     success: false,
     error: responseErr.name,