Removes second log when responding with a regular error

Author: flovilmart

spec/CloudCodeLogger.spec.js

@@ -1,6 +1,7 @@
 const LoggerController = require('../lib/Controllers/LoggerController').LoggerController;
 const WinstonLoggerAdapter = require('../lib/Adapters/Logger/WinstonLoggerAdapter').WinstonLoggerAdapter;
 const fs = require('fs');
+const Config = require('../lib/Config');
 
 const loremFile = __dirname + '/support/lorem.txt';
 
@@ -23,34 +24,33 @@ describe("Cloud Code Logger", () => {
   // Note that helpers takes care of logout.
   // see helpers.js:afterEach
 
-  it("should expose log to functions", done => {
-    const logController = new LoggerController(new WinstonLoggerAdapter());
+  it("should expose log to functions", () => {
+    const config = Config.get('test');
+    const spy = spyOn(config.loggerController, 'log').and.callThrough();
 
     Parse.Cloud.define("loggerTest", (req, res) => {
       req.log.info('logTest', 'info log', { info: 'some log' });
       req.log.error('logTest', 'error log', { error: 'there was an error' });
       res.success({});
     });
 
-    Parse.Cloud.run('loggerTest').then(() => {
-      return logController.getLogs({ from: Date.now() - 500, size: 1000 });
-    }).then((res) => {
-      expect(res.length).not.toBe(0);
-      const lastLogs = res.slice(0, 3);
-      const cloudFunctionMessage = lastLogs[0];
-      const errorMessage = lastLogs[1];
-      const infoMessage = lastLogs[2];
-      expect(cloudFunctionMessage.level).toBe('info');
-      expect(cloudFunctionMessage.params).toEqual({});
-      expect(cloudFunctionMessage.message).toMatch(/Ran cloud function loggerTest for user [^ ]* with:\n {2}Input: {}\n {2}Result: {}/);
-      expect(cloudFunctionMessage.functionName).toEqual('loggerTest');
-      expect(errorMessage.level).toBe('error');
-      expect(errorMessage.error).toBe('there was an error');
-      expect(errorMessage.message).toBe('logTest error log');
-      expect(infoMessage.level).toBe('info');
-      expect(infoMessage.info).toBe('some log');
-      expect(infoMessage.message).toBe('logTest info log');
-      done();
+    return Parse.Cloud.run('loggerTest').then(() => {
+      expect(spy).toHaveBeenCalledTimes(3);
+      const cloudFunctionMessage = spy.calls.all()[2];
+      const errorMessage = spy.calls.all()[1];
+      const infoMessage = spy.calls.all()[0];
+      expect(cloudFunctionMessage.args[0]).toBe('info');
+      expect(cloudFunctionMessage.args[1][1].params).toEqual({});
+      expect(cloudFunctionMessage.args[1][0]).toMatch(/Ran cloud function loggerTest for user [^ ]* with:\n {2}Input: {}\n {2}Result: {}/);
+      expect(cloudFunctionMessage.args[1][1].functionName).toEqual('loggerTest');
+      expect(errorMessage.args[0]).toBe('error');
+      expect(errorMessage.args[1][2].error).toBe('there was an error');
+      expect(errorMessage.args[1][0]).toBe('logTest');
+      expect(errorMessage.args[1][1]).toBe('error log');
+      expect(infoMessage.args[0]).toBe('info');
+      expect(infoMessage.args[1][2].info).toBe('some log');
+      expect(infoMessage.args[1][0]).toBe('logTest');
+      expect(infoMessage.args[1][1]).toBe('info log');
     });
   });
 
@@ -194,7 +194,8 @@ describe("Cloud Code Logger", () => {
     Parse.Cloud.run('aFunction', { foo: 'bar' })
       .then(null, () => logController.getLogs({ from: Date.now() - 500, size: 1000 }))
       .then(logs => {
-        const log = logs[2];
+        expect(logs[0].message).toBe('it failed!');
+        const log = logs[1];
         expect(log.level).toEqual('error');
         expect(log.message).toMatch(
           /Failed running cloud function aFunction for user [^ ]* with:\n {2}Input: {"foo":"bar"}\n {2}Error: {"code":141,"message":"it failed!"}/);
@@ -243,4 +244,18 @@ describe("Cloud Code Logger", () => {
       })
       .then(null, e => done.fail(e));
   });
+
+  it('should only log once for object not found', async () => {
+    const config = Config.get('test');
+    const spy = spyOn(config.loggerController, 'error').and.callThrough();
+    try {
+      const object = new Parse.Object('Object');
+      object.id = 'invalid'
+      await object.fetch();
+    } catch(e) { /**/ }
+    expect(spy).toHaveBeenCalled();
+    expect(spy.calls.count()).toBe(1);
+    const { args } = spy.calls.mostRecent();
+    expect(args[0]).toBe('Object not found.');
+  });
 });

spec/LogsRouter.spec.js

@@ -87,8 +87,8 @@ describe('LogsRouter', () => {
         }, (error, response, body) => {
           expect(response.statusCode).toEqual(200);
           // 4th entry is our actual GET request
-          expect(body[3].url).toEqual('/1/login?username=test&password=********');
-          expect(body[3].message).toEqual('REQUEST for [GET] /1/login?username=test&password=********: {}');
+          expect(body[2].url).toEqual('/1/login?username=test&password=********');
+          expect(body[2].message).toEqual('REQUEST for [GET] /1/login?username=test&password=********: {}');
           done();
         });
       });
@@ -114,8 +114,8 @@ describe('LogsRouter', () => {
         }, (error, response, body) => {
           expect(response.statusCode).toEqual(200);
           // 4th entry is our actual GET request
-          expect(body[3].url).toEqual('/1/login?username=test&password=********');
-          expect(body[3].message).toEqual('REQUEST for [GET] /1/login?username=test&password=********: {}');
+          expect(body[2].url).toEqual('/1/login?username=test&password=********');
+          expect(body[2].message).toEqual('REQUEST for [GET] /1/login?username=test&password=********: {}');
           done();
         });
       });
@@ -144,8 +144,8 @@ describe('LogsRouter', () => {
         }, (error, response, body) => {
           expect(response.statusCode).toEqual(200);
           // 4th entry is our actual GET request
-          expect(body[3].url).toEqual('/1/login');
-          expect(body[3].message).toEqual('REQUEST for [POST] /1/login: {}');
+          expect(body[2].url).toEqual('/1/login');
+          expect(body[2].message).toEqual('REQUEST for [POST] /1/login: {}');
           done();
         });
       });

src/PromiseRouter.js

@@ -181,7 +181,7 @@ function makeExpressHandler(appId, promiseHandler) {
           })
         }
         res.json(result.response);
-      }, (e) => {
+      }, (error) => next(error)).catch((e) => {
         log.error(`Error generating response. ${inspect(e)}`, {error: e});
         next(e);
       });

src/middlewares.js

@@ -1,9 +1,9 @@
 import AppCache from './cache';
-import log from './logger';
 import Parse from 'parse/node';
 import auth from './Auth';
 import Config from './Config';
 import ClientSDK from './ClientSDK';
+import defaultLogger from './logger';
 
 // Checks that the request is authorized for this app and checks user
 // auth too.
@@ -179,7 +179,7 @@ export function handleParseHeaders(req, res, next) {
       }
       else {
         // TODO: Determine the correct error scenario.
-        log.error('error getting auth for sessionToken', error);
+        req.config.loggerController.error('error getting auth for sessionToken', error);
         throw new Parse.Error(Parse.Error.UNKNOWN_ERROR, error);
       }
     });
@@ -267,6 +267,7 @@ export function allowMethodOverride(req, res, next) {
 }
 
 export function handleParseErrors(err, req, res, next) {
+  const log = (req.config && req.config.loggerController) || defaultLogger;
   if (err instanceof Parse.Error) {
     let httpStatus;
     // TODO: fill out this mapping