Create a ServerError class.

The problem this pr is trying to solve:

When an error occurs on the server, a message should
be returned to the client, and a message should be logged.

Currently, noting is often logged.

Byt adding a ServerError class, we can embed a ParseError into
the ServerError.  The ServerError contains the message for the log
and the ParseError contains the error for the client.

In addition, this pr will stop calling the default express error handler
which causes two problems: 1. it writes to console instead of log file
2. the output is completely useless! :)

Author: Arthur Cinader

spec/CloudCodeLogger.spec.js

@@ -194,7 +194,7 @@ 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[1];
+              const log = logs[2];
               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!"}/);

spec/FilesController.spec.js

@@ -1,7 +1,17 @@
-var GridStoreAdapter = require("../src/Adapters/Files/GridStoreAdapter").GridStoreAdapter;
-var Config = require("../src/Config");
-var FilesController = require('../src/Controllers/FilesController').default;
+const LoggerController = require('../src/Controllers/LoggerController').LoggerController;
+const WinstonLoggerAdapter = require('../src/Adapters/Logger/WinstonLoggerAdapter').WinstonLoggerAdapter;
+const GridStoreAdapter = require("../src/Adapters/Files/GridStoreAdapter").GridStoreAdapter;
+const Config = require("../src/Config");
+const FilesController = require('../src/Controllers/FilesController').default;
 
+const mockAdapter = {
+  createFile: () => {
+    return Parse.Promise.reject(new Error('it failed'));
+  },
+  deleteFile: () => { },
+  getFileData: () => { },
+  getFileLocation: () => { }
+}
 
 // Small additional tests to improve overall coverage
 describe("FilesController",() =>{
@@ -26,5 +36,25 @@ describe("FilesController",() =>{
     expect(anObject.aFile.url).toEqual("http://an.url");
 
     done();
-  })
+  });
+
+  it('should create a server log on failure', done => {
+    const logController = new LoggerController(new WinstonLoggerAdapter());
+
+    reconfigureServer({ filesAdapter: mockAdapter })
+      .then(() => {
+        const file = new Parse.File('foo', { base64: "xxxx" });
+        return file.save(null, { useMasterKey: true });
+      })
+      .then(() => done.fail('should not succeed'), () => logController.getLogs({ from: Date.now() - 500, size: 1000 }))
+      .then((logs) => {
+        const log = logs.pop();
+        expect(log.level).toBe('error');
+        expect(log.code).toBe(130);
+        expect(log.message).toBe('Could not store file.');
+        expect(log.serverLogError).toBe('it failed');
+        expect(log.stack).not.toBe(undefined);
+        done();
+      });
+  });
 });

src/ParseServer.js

@@ -394,4 +394,6 @@ function addParseCloud() {
   global.Parse = Parse;
 }
 
+ParseServer.Error = require('./ServerError').default;
+
 export default ParseServer;

src/Routers/FilesRouter.js

@@ -1,9 +1,10 @@
-import express             from 'express';
-import BodyParser          from 'body-parser';
-import * as Middlewares    from '../middlewares';
-import Parse               from 'parse/node';
-import Config              from '../Config';
-import mime                from 'mime';
+import express from 'express';
+import BodyParser from 'body-parser';
+import * as Middlewares from '../middlewares';
+import Parse from 'parse/node';
+import Config from '../Config';
+import mime from 'mime';
+import ParseServer from '../ParseServer';
 
 export class FilesRouter {
 
@@ -87,8 +88,9 @@ export class FilesRouter {
       res.status(201);
       res.set('Location', result.url);
       res.json(result);
-    }).catch(() => {
-      next(new Parse.Error(Parse.Error.FILE_SAVE_ERROR, 'Could not store file.'));
+    }).catch((e) => {
+      const serverError = new ParseServer.Error(Parse.Error.FILE_SAVE_ERROR, 'Could not store file.', e.message, e.stack);
+      next(serverError);
     });
   }
 

src/ServerError.js

@@ -0,0 +1,26 @@
+import Parse from 'parse';
+
+/**
+ * Constructs a new ServerError object with the given message and source.
+ *
+ * The message is an error that is intended for the server logs
+ * while the source message is intended for the server logs to aid a developer
+ *
+ * @class ServerError
+ * @constructor
+ * @param {ErrorCode} code the Parse.Error code
+ * @param {String} message will be sent to clients
+ * @param {String} serverErrorLog additional information
+ *                  about the error for logging on the server
+ * @param {String} stack the stack trace
+ */
+export default class ServerError extends Parse.Error {
+  serverLogError;
+  stack;
+
+  constructor(code, message, serverLogError, stack) {
+    super(code, message);
+    this.serverLogError = serverLogError;
+    this.stack = stack;
+  }
+}

src/middlewares.js

@@ -1,9 +1,10 @@
-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 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 ParseServer from './ParseServer';
 
 // Checks that the request is authorized for this app and checks user
 // auth too.
@@ -233,10 +234,8 @@ export function allowMethodOverride(req, res, next) {
 }
 
 export function handleParseErrors(err, req, res, next) {
-  // TODO: Add logging as those errors won't make it to the PromiseRouter
-  if (err instanceof Parse.Error) {
-    var httpStatus;
-
+  if (err instanceof Parse.Error || err instanceof ParseServer.Error) {
+    let httpStatus;
     // TODO: fill out this mapping
     switch (err.code) {
     case Parse.Error.INTERNAL_SERVER_ERROR:
@@ -250,17 +249,23 @@ export function handleParseErrors(err, req, res, next) {
     }
 
     res.status(httpStatus);
-    res.json({code: err.code, error: err.message});
+    res.json({ code: err.code, error: err.message });
+    log.error(err.message, err);
   } else if (err.status && err.message) {
+    log.error(err.message);
     res.status(err.status);
-    res.json({error: err.message});
+    res.json({ error: err.message });
+    next(err);
   } else {
     log.error('Uncaught internal server error.', err, err.stack);
     res.status(500);
-    res.json({code: Parse.Error.INTERNAL_SERVER_ERROR,
-      message: 'Internal server error.'});
+    res.json({
+      code: Parse.Error.INTERNAL_SERVER_ERROR,
+      message: 'Internal server error.'
+    });
+    next(err);
   }
-  next(err);
+
 }
 
 export function enforceMasterKeyAccess(req, res, next) {