Merge pull request #7511 from HabitRPG/paglias/better-logging

Improve logging

test/api/v3/unit/middlewares/errorHandler.test.js

@@ -168,6 +168,8 @@ describe('errorHandler', () => {
       originalUrl: req.originalUrl,
       headers: req.headers,
       body: req.body,
+      httpCode: 400,
+      isHandledError: true,
     });
   });
 });

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,28 @@ 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 that are handled as warnings, 5xx and uncaught errors as serious problems
+      if (!errorData || !errorData.isHandledError || errorData.httpCode >= 500) {
+        logger.error(...loggerArgs);
+      } else {
+        logger.warn(...loggerArgs);
+      }
     } else {
       logger.error(...args);
     }

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,15 @@ 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,
+    httpCode: responseErr.httpCode,
+    isHandledError: responseErr.httpCode < 500,
+  });
+
   let jsonRes = {
     success: false,
     error: responseErr.name,