Winston 3 upgrade (#5496)

* ⚡ Release 3.1.3 (#5267) * ⚡ Release 3.1.3 * Update CHANGELOG.md * ⬆️ Bump winston and winston-daily-rotate-file Bumps [winston](https://github.com/winstonjs/winston) and [winston-daily-rotate-file](https://github.com/winstonjs/winston-daily-rotate-file). These dependencies needed to be updated together. Updates `winston` from 2.4.4 to 3.1.0 - [Release notes](https://github.com/winstonjs/winston/releases) - [Changelog](https://github.com/winstonjs/winston/blob/master/CHANGELOG.md) - [Commits](https://github.com/winstonjs/winston/compare/2.4.4...3.1.0) Updates `winston-daily-rotate-file` from 1.7.2 to 3.5.1 - [Release notes](https://github.com/winstonjs/winston-daily-rotate-file/releases) - [Commits](https://github.com/winstonjs/winston-daily-rotate-file/compare/v1.7.2...v3.5.1) Signed-off-by: dependabot[bot] <support@dependabot.com> * Rewrote WinstonLogger to work with winston 3.x api * Changed winston logger test to use winston-transport * Added winston-transport dependency * Close and remove transports before adding them again * Changed to strict equal * Override adapter name * Updated and added getLogs tests * Bump winston and winston-daily-rotate-file Bumps [winston](https://github.com/winstonjs/winston) and [winston-daily-rotate-file](https://github.com/winstonjs/winston-daily-rotate-file). These dependencies needed to be updated together. Updates `winston` from 2.4.4 to 3.2.0 - [Release notes](https://github.com/winstonjs/winston/releases) - [Changelog](https://github.com/winstonjs/winston/blob/master/CHANGELOG.md) - [Commits](https://github.com/winstonjs/winston/compare/2.4.4...3.2.0) Updates `winston-daily-rotate-file` from 1.7.2 to 3.6.0 - [Release notes](https://github.com/winstonjs/winston-daily-rotate-file/releases) - [Commits](https://github.com/winstonjs/winston-daily-rotate-file/compare/v1.7.2...v3.6.0) Signed-off-by: dependabot[bot] <support@dependabot.com> * Fixed tests, updated parse logging * Fixed tests, better error logging * Fix failing tests * Updates as per review
2019-04-15 09:03:33 +10:00
parent 943134812e
commit 6ffc41345f
14 changed files with 1018 additions and 1010 deletions
--- a/package-lock.json
+++ b/package-lock.json
--- a/package.json
+++ b/package.json
@@ -40,8 +40,8 @@
    "semver": "6.0.0",
    "tv4": "1.3.0",
    "uuid": "3.3.2",
-    "winston": "2.4.4",
-    "winston-daily-rotate-file": "1.7.2",
+    "winston": "3.2.1",
+    "winston-daily-rotate-file": "3.8.0",
    "ws": "6.2.1"
  },
  "devDependencies": {
--- a/spec/AudienceRouter.spec.js
+++ b/spec/AudienceRouter.spec.js
@@ -189,52 +189,60 @@ describe('AudiencesRouter', () => {
      });
  });

-  it_exclude_dbs(['postgres'])('query installations with limit = 0 and count = 1', done => {
-    const config = Config.get('test');
-    const androidAudienceRequest = {
-      name: 'Android Users',
-      query: '{ "test": "android" }',
-    };
-    const iosAudienceRequest = {
-      name: 'Iphone Users',
-      query: '{ "test": "ios" }',
-    };
-    const request = {
-      config: config,
-      auth: auth.master(config),
-      body: {},
-      query: {
-        limit: 0,
-        count: 1,
-      },
-      info: {},
-    };
+  it_exclude_dbs(['postgres'])(
+    'query installations with limit = 0 and count = 1',
+    done => {
+      const config = Config.get('test');
+      const androidAudienceRequest = {
+        name: 'Android Users',
+        query: '{ "test": "android" }',
+      };
+      const iosAudienceRequest = {
+        name: 'Iphone Users',
+        query: '{ "test": "ios" }',
+      };
+      const request = {
+        config: config,
+        auth: auth.master(config),
+        body: {},
+        query: {
+          limit: 0,
+          count: 1,
+        },
+        info: {},
+      };

-    const router = new AudiencesRouter();
-    rest
-      .create(config, auth.nobody(config), '_Audience', androidAudienceRequest)
-      .then(() => {
-        return rest.create(
+      const router = new AudiencesRouter();
+      rest
+        .create(
          config,
          auth.nobody(config),
          '_Audience',
-          iosAudienceRequest
-        );
-      })
-      .then(() => {
-        return router.handleFind(request);
-      })
-      .then(res => {
-        const response = res.response;
-        expect(response.results.length).toEqual(0);
-        expect(response.count).toEqual(2);
-        done();
-      })
-      .catch(err => {
-        fail(JSON.stringify(err));
-        done();
-      });
-  });
+          androidAudienceRequest
+        )
+        .then(() => {
+          return rest.create(
+            config,
+            auth.nobody(config),
+            '_Audience',
+            iosAudienceRequest
+          );
+        })
+        .then(() => {
+          return router.handleFind(request);
+        })
+        .then(res => {
+          const response = res.response;
+          expect(response.results.length).toEqual(0);
+          expect(response.count).toEqual(2);
+          done();
+        })
+        .catch(err => {
+          fail(JSON.stringify(err));
+          done();
+        });
+    }
+  );

  it('should create, read, update and delete audiences throw api', done => {
    Parse._request(
--- a/spec/CloudCodeLogger.spec.js
+++ b/spec/CloudCodeLogger.spec.js
@@ -203,7 +203,9 @@ describe('Cloud Code Logger', () => {
      .catch(() => {})
      .then(() => {
        const logs = spy.calls.all().reverse();
-        expect(logs[0].args[1]).toBe('it failed!');
+        expect(logs[0].args[1]).toBe('Parse error: ');
+        expect(logs[0].args[2].message).toBe('it failed!');
+
        const log = logs[1].args;
        expect(log[0]).toEqual('error');
        expect(log[1]).toMatch(
@@ -268,6 +270,7 @@ describe('Cloud Code Logger', () => {
    expect(spy).toHaveBeenCalled();
    expect(spy.calls.count()).toBe(1);
    const { args } = spy.calls.mostRecent();
-    expect(args[0]).toBe('Object not found.');
+    expect(args[0]).toBe('Parse error: ');
+    expect(args[1].message).toBe('Object not found.');
  });
 });
--- a/spec/FilesController.spec.js
+++ b/spec/FilesController.spec.js
@@ -9,7 +9,7 @@ const FilesController = require('../lib/Controllers/FilesController').default;

 const mockAdapter = {
  createFile: () => {
-    return Promise.reject(new Error('it failed'));
+    return Promise.reject(new Error('it failed with xyz'));
  },
  deleteFile: () => {},
  getFileData: () => {},
@@ -59,13 +59,18 @@ describe('FilesController', () => {
      .then(logs => {
        // we get two logs here: 1. the source of the failure to save the file
        // and 2 the message that will be sent back to the client.
-        const log1 = logs.pop();
+
+        const log1 = logs.find(
+          x => x.message === 'Error creating a file: it failed with xyz'
+        );
        expect(log1.level).toBe('error');
-        expect(log1.message).toBe('it failed');
-        const log2 = logs.pop();
+
+        const log2 = logs.find(
+          x => x.message === 'Parse error: Could not store file: yolo.txt.'
+        );
        expect(log2.level).toBe('error');
        expect(log2.code).toBe(130);
-        expect(log2.message).toBe('Could not store file.');
+
        done();
      });
  });
--- a/spec/Logger.spec.js
+++ b/spec/Logger.spec.js
@@ -1,31 +1,30 @@
 const logging = require('../lib/Adapters/Logger/WinstonLogger');
-const winston = require('winston');
+const Transport = require('winston-transport');

-class TestTransport extends winston.Transport {
-  log(level, msg, meta, callback) {
+class TestTransport extends Transport {
+  log(info, callback) {
    callback(null, true);
  }
 }

-describe('Logger', () => {
+describe('WinstonLogger', () => {
  it('should add transport', () => {
-    const testTransport = new TestTransport({
-      name: 'test',
-    });
+    const testTransport = new TestTransport();
    spyOn(testTransport, 'log');
    logging.addTransport(testTransport);
-    expect(Object.keys(logging.logger.transports).length).toBe(4);
+    expect(logging.logger.transports.length).toBe(4);
    logging.logger.info('hi');
    expect(testTransport.log).toHaveBeenCalled();
+    logging.logger.error('error');
+    expect(testTransport.log).toHaveBeenCalled();
    logging.removeTransport(testTransport);
-    expect(Object.keys(logging.logger.transports).length).toBe(3);
+    expect(logging.logger.transports.length).toBe(3);
  });

  it('should have files transports', done => {
    reconfigureServer().then(() => {
      const transports = logging.logger.transports;
-      const transportKeys = Object.keys(transports);
-      expect(transportKeys.length).toBe(3);
+      expect(transports.length).toBe(3);
      done();
    });
  });
@@ -35,8 +34,7 @@ describe('Logger', () => {
      logsFolder: null,
    }).then(() => {
      const transports = logging.logger.transports;
-      const transportKeys = Object.keys(transports);
-      expect(transportKeys.length).toBe(1);
+      expect(transports.length).toBe(1);
      done();
    });
  });
--- a/spec/LoggerController.spec.js
+++ b/spec/LoggerController.spec.js
@@ -44,7 +44,7 @@ describe('LoggerController', () => {
    done();
  });

-  it('can process an ascending query without throwing', done => {
+  it('can parse an ascending query without throwing', done => {
    // Make mock request
    const query = {
      from: '2016-01-01Z00:00:00',
@@ -65,11 +65,59 @@ describe('LoggerController', () => {
    done();
  });

+  it('can process an ascending query without throwing', done => {
+    // Make mock request
+    const query = {
+      from: '2016-01-01Z00:00:00',
+      until: '2016-01-01Z00:00:00',
+      size: 5,
+      order: 'asc',
+      level: 'error',
+    };
+
+    const loggerController = new LoggerController(new WinstonLoggerAdapter());
+
+    expect(() => {
+      loggerController
+        .getLogs(query)
+        .then(function(res) {
+          expect(res.length).not.toBe(0);
+          done();
+        })
+        .catch(err => {
+          jfail(err);
+          fail('should not fail');
+          done();
+        });
+    }).not.toThrow();
+  });
+
+  it('can parse a descending query without throwing', done => {
+    // Make mock request
+    const query = {
+      from: '2016-01-01Z00:00:00',
+      until: '2016-01-01Z00:00:00',
+      size: 5,
+      order: 'desc',
+      level: 'error',
+    };
+
+    const result = LoggerController.parseOptions(query);
+
+    expect(result.from.getTime()).toEqual(1451606400000);
+    expect(result.until.getTime()).toEqual(1451606400000);
+    expect(result.size).toEqual(5);
+    expect(result.order).toEqual('desc');
+    expect(result.level).toEqual('error');
+
+    done();
+  });
+
  it('can process a descending query without throwing', done => {
    // Make mock request
    const query = {
-      from: '2016-01-01',
-      until: '2016-01-30',
+      from: '2016-01-01Z00:00:00',
+      until: '2016-01-01Z00:00:00',
      size: 5,
      order: 'desc',
      level: 'error',
@@ -81,7 +129,7 @@ describe('LoggerController', () => {
      loggerController
        .getLogs(query)
        .then(function(res) {
-          expect(res.length).toBe(0);
+          expect(res.length).not.toBe(0);
          done();
        })
        .catch(err => {
--- a/spec/MongoStorageAdapter.spec.js
+++ b/spec/MongoStorageAdapter.spec.js
@@ -6,6 +6,11 @@ const { MongoClient } = require('mongodb');
 const databaseURI =
  'mongodb://localhost:27017/parseServerMongoAdapterTestDatabase';

+const fakeClient = {
+  s: { options: { dbName: null } },
+  db: () => null,
+};
+
 // These tests are specific to the mongo storage adapter + mongo storage format
 // and will eventually be moved into their own repo
 describe_only_db('mongo')('MongoStorageAdapter', () => {
@@ -16,7 +21,7 @@ describe_only_db('mongo')('MongoStorageAdapter', () => {
  });

  it('auto-escapes symbols in auth information', () => {
-    spyOn(MongoClient, 'connect').and.returnValue(Promise.resolve(null));
+    spyOn(MongoClient, 'connect').and.returnValue(Promise.resolve(fakeClient));
    new MongoStorageAdapter({
      uri:
        'mongodb://user!with@+ symbols:password!with@+ symbols@localhost:1234/parse',
@@ -28,7 +33,7 @@ describe_only_db('mongo')('MongoStorageAdapter', () => {
  });

  it("doesn't double escape already URI-encoded information", () => {
-    spyOn(MongoClient, 'connect').and.returnValue(Promise.resolve(null));
+    spyOn(MongoClient, 'connect').and.returnValue(Promise.resolve(fakeClient));
    new MongoStorageAdapter({
      uri:
        'mongodb://user!with%40%2B%20symbols:password!with%40%2B%20symbols@localhost:1234/parse',
@@ -41,7 +46,7 @@ describe_only_db('mongo')('MongoStorageAdapter', () => {

  // https://github.com/parse-community/parse-server/pull/148#issuecomment-180407057
  it('preserves replica sets', () => {
-    spyOn(MongoClient, 'connect').and.returnValue(Promise.resolve(null));
+    spyOn(MongoClient, 'connect').and.returnValue(Promise.resolve(fakeClient));
    new MongoStorageAdapter({
      uri:
        'mongodb://test:testpass@ds056315-a0.mongolab.com:59325,ds059315-a1.mongolab.com:59315/testDBname?replicaSet=rs-ds059415',
--- a/spec/PushController.spec.js
+++ b/spec/PushController.spec.js
@@ -1512,7 +1512,10 @@ describe('PushController', () => {
          date: noTimezone,
          isLocalTime: true,
        })
-      ).toBe(`2017-09-06T${expectedHour}:14:01.048`, 'No timezone');
+      ).toBe(
+        `2017-09-06T${expectedHour.toString().padStart(2, '0')}:14:01.048`,
+        'No timezone'
+      );
      expect(
        PushController.formatPushTime({
          date: new Date('2017-09-06'),
@@ -1569,7 +1572,7 @@ describe('PushController', () => {
        .then(pushStatus => {
          expect(pushStatus.get('status')).toBe('scheduled');
          expect(pushStatus.get('pushTime')).toBe(
-            `2017-09-06T${expectedHour}:14:01.048`
+            `2017-09-06T${expectedHour.toString().padStart(2, '0')}:14:01.048`
          );
        })
        .then(done, done.fail);
--- a/spec/WinstonLoggerAdapter.spec.js
+++ b/spec/WinstonLoggerAdapter.spec.js
@@ -7,59 +7,64 @@ const request = require('../lib/request');
 describe('info logs', () => {
  it('Verify INFO logs', done => {
    const winstonLoggerAdapter = new WinstonLoggerAdapter();
-    winstonLoggerAdapter.log('info', 'testing info logs', () => {
-      winstonLoggerAdapter.query(
-        {
-          from: new Date(Date.now() - 500),
-          size: 100,
-          level: 'info',
-        },
-        results => {
-          if (results.length == 0) {
-            fail('The adapter should return non-empty results');
-          } else {
-            expect(results[0].message).toEqual('testing info logs');
-          }
-          // Check the error log
-          // Regression #2639
-          winstonLoggerAdapter.query(
-            {
-              from: new Date(Date.now() - 200),
-              size: 100,
-              level: 'error',
-            },
-            results => {
-              expect(results.length).toEqual(0);
-              done();
-            }
+    winstonLoggerAdapter.log('info', 'testing info logs with 1234');
+    winstonLoggerAdapter.query(
+      {
+        from: new Date(Date.now() - 500),
+        size: 100,
+        level: 'info',
+        order: 'desc',
+      },
+      results => {
+        if (results.length == 0) {
+          fail('The adapter should return non-empty results');
+        } else {
+          const log = results.find(
+            x => x.message === 'testing info logs with 1234'
          );
+          expect(log.level).toEqual('info');
        }
-      );
-    });
+        // Check the error log
+        // Regression #2639
+        winstonLoggerAdapter.query(
+          {
+            from: new Date(Date.now() - 200),
+            size: 100,
+            level: 'error',
+          },
+          errors => {
+            const log = errors.find(
+              x => x.message === 'testing info logs with 1234'
+            );
+            expect(log).toBeUndefined();
+            done();
+          }
+        );
+      }
+    );
  });
 });

 describe('error logs', () => {
  it('Verify ERROR logs', done => {
    const winstonLoggerAdapter = new WinstonLoggerAdapter();
-    winstonLoggerAdapter.log('error', 'testing error logs', () => {
-      winstonLoggerAdapter.query(
-        {
-          from: new Date(Date.now() - 500),
-          size: 100,
-          level: 'error',
-        },
-        results => {
-          if (results.length == 0) {
-            fail('The adapter should return non-empty results');
-            done();
-          } else {
-            expect(results[0].message).toEqual('testing error logs');
-            done();
-          }
+    winstonLoggerAdapter.log('error', 'testing error logs');
+    winstonLoggerAdapter.query(
+      {
+        from: new Date(Date.now() - 500),
+        size: 100,
+        level: 'error',
+      },
+      results => {
+        if (results.length == 0) {
+          fail('The adapter should return non-empty results');
+          done();
+        } else {
+          expect(results[0].message).toEqual('testing error logs');
+          done();
        }
-      );
-    });
+      }
+    );
  });
 });

--- a/src/Adapters/Logger/WinstonLogger.js
+++ b/src/Adapters/Logger/WinstonLogger.js
@@ -5,59 +5,58 @@ import DailyRotateFile from 'winston-daily-rotate-file';
 import _ from 'lodash';
 import defaults from '../../defaults';

-const logger = new winston.Logger();
-const additionalTransports = [];
+const logger = winston.createLogger();

-function updateTransports(options) {
-  const transports = Object.assign({}, logger.transports);
+function configureTransports(options) {
+  const transports = [];
  if (options) {
    const silent = options.silent;
    delete options.silent;
-    if (_.isNull(options.dirname)) {
-      delete transports['parse-server'];
-      delete transports['parse-server-error'];
-    } else if (!_.isUndefined(options.dirname)) {
-      transports['parse-server'] = new DailyRotateFile(
+
+    if (!_.isNil(options.dirname)) {
+      const parseServer = new DailyRotateFile(
        Object.assign(
-          {},
          {
            filename: 'parse-server.info',
-            name: 'parse-server',
+            json: true,
          },
          options,
          { timestamp: true }
        )
      );
-      transports['parse-server-error'] = new DailyRotateFile(
+      parseServer.name = 'parse-server';
+      transports.push(parseServer);
+
+      const parseServerError = new DailyRotateFile(
        Object.assign(
-          {},
          {
            filename: 'parse-server.err',
-            name: 'parse-server-error',
+            json: true,
          },
          options,
          { level: 'error', timestamp: true }
        )
      );
+      parseServerError.name = 'parse-server-error';
+      transports.push(parseServerError);
    }

-    transports.console = new winston.transports.Console(
-      Object.assign(
-        {
-          colorize: true,
-          name: 'console',
-          silent,
-        },
-        options
+    transports.push(
+      new winston.transports.Console(
+        Object.assign(
+          {
+            colorize: true,
+            name: 'console',
+            silent,
+          },
+          options
+        )
      )
    );
  }
-  // Mount the additional transports
-  additionalTransports.forEach(transport => {
-    transports[transport.name] = transport;
-  });
+
  logger.configure({
-    transports: _.values(transports),
+    transports,
  });
 }

@@ -93,25 +92,27 @@ export function configureLogger({
    options.json = true;
    options.stringify = true;
  }
-  updateTransports(options);
+  configureTransports(options);
 }

 export function addTransport(transport) {
-  additionalTransports.push(transport);
-  updateTransports();
+  // we will remove the existing transport
+  // before replacing it with a new one
+  removeTransport(transport.name);
+
+  logger.add(transport);
 }

 export function removeTransport(transport) {
-  const transportName =
-    typeof transport == 'string' ? transport : transport.name;
-  const transports = Object.assign({}, logger.transports);
-  delete transports[transportName];
-  logger.configure({
-    transports: _.values(transports),
-  });
-  _.remove(additionalTransports, transport => {
-    return transport.name === transportName;
+  const matchingTransport = logger.transports.find(t1 => {
+    return typeof transport === 'string'
+      ? t1.name === transport
+      : t1 === transport;
  });
+
+  if (matchingTransport) {
+    logger.remove(matchingTransport);
+  }
 }

 export { logger };
--- a/src/Adapters/Logger/WinstonLoggerAdapter.js
+++ b/src/Adapters/Logger/WinstonLoggerAdapter.js
@@ -48,7 +48,8 @@ export class WinstonLoggerAdapter extends LoggerAdapter {
          callback(err);
          return reject(err);
        }
-        if (level == 'error') {
+
+        if (level === 'error') {
          callback(res['parse-server-error']);
          resolve(res['parse-server-error']);
        } else {
--- a/src/Routers/FilesRouter.js
+++ b/src/Routers/FilesRouter.js
@@ -111,9 +111,12 @@ export class FilesRouter {
        res.json(result);
      })
      .catch(e => {
-        logger.error(e.message, e);
+        logger.error('Error creating a file: ', e);
        next(
-          new Parse.Error(Parse.Error.FILE_SAVE_ERROR, 'Could not store file.')
+          new Parse.Error(
+            Parse.Error.FILE_SAVE_ERROR,
+            `Could not store file: ${filename}.`
+          )
        );
      });
  }
--- a/src/middlewares.js
+++ b/src/middlewares.js
@@ -325,7 +325,7 @@ export function handleParseErrors(err, req, res, next) {

    res.status(httpStatus);
    res.json({ code: err.code, error: err.message });
-    log.error(err.message, err);
+    log.error('Parse error: ', err);
    if (req.config && req.config.enableExpressErrorHandler) {
      next(err);
    }