diff --git a/libraries/logger/.eslintrc b/libraries/logger/.eslintrc index b0db54a22f..3efdae4f58 100644 --- a/libraries/logger/.eslintrc +++ b/libraries/logger/.eslintrc @@ -19,12 +19,6 @@ "settings": { }, "rules": { - "max-len": ["error", { - "ignoreUrls": true, - // Ignore long describe/it test blocks, long import/require statements - "ignorePattern": "(^\\s*(it|describe)\\s*\\(['\"]|^import\\s*.*\\s*from\\s*['\"]|^.*\\s*=\\s*require\\(['\"])" - }], - // Add some mocha specific rules "mocha/handle-done-callback": "error", "mocha/no-exclusive-tests": "error", diff --git a/libraries/logger/logging-manager.js b/libraries/logger/logging-manager.js index 794f23eccb..541d612137 100644 --- a/libraries/logger/logging-manager.js +++ b/libraries/logger/logging-manager.js @@ -3,9 +3,10 @@ const request = require('request') const OError = require('@overleaf/o-error') // bunyan error serializer -const errSerializer = function (err) { - if (!err || !err.stack) - return err; +const errSerializer = function(err) { + if (!err || !err.stack) { + return err + } return { message: err.message, name: err.name, @@ -13,10 +14,10 @@ const errSerializer = function (err) { info: OError.getFullInfo(err), code: err.code, signal: err.signal - }; -}; + } +} -const Logger = module.exports = { +const Logger = (module.exports = { initialize(name) { this.isProduction = (process.env['NODE_ENV'] || '').toLowerCase() === 'production' @@ -31,14 +32,13 @@ const Logger = module.exports = { } ] if (this.ringBufferSize > 0) { - this.ringBuffer = new bunyan.RingBuffer({limit: this.ringBufferSize}) + this.ringBuffer = new bunyan.RingBuffer({ limit: this.ringBufferSize }) loggerStreams.push({ level: 'trace', type: 'raw', stream: this.ringBuffer }) - } - else { + } else { this.ringBuffer = null } this.logger = bunyan.createLogger({ @@ -69,9 +69,7 @@ const Logger = module.exports = { headers: { 'Metadata-Flavor': 'Google' }, - uri: `http://metadata.google.internal/computeMetadata/v1/project/attributes/${ - this.loggerName - }-setLogLevelEndTime` + uri: `http://metadata.google.internal/computeMetadata/v1/project/attributes/${this.loggerName}-setLogLevelEndTime` } request(options, (err, response, body) => { if (err) { @@ -86,9 +84,9 @@ const Logger = module.exports = { }) }, - initializeErrorReporting(sentry_dsn, options) { + initializeErrorReporting(sentryDsn, options) { const raven = require('raven') - this.raven = new raven.Client(sentry_dsn, options) + this.raven = new raven.Client(sentryDsn, options) this.lastErrorTimeStamp = 0 // for rate limiting on sentry reporting this.lastErrorCount = 0 }, @@ -154,25 +152,21 @@ const Logger = module.exports = { if (error.path) { error.message = error.message.replace(` '${error.path}'`, '') } - } catch (error1) {} - // send the error to sentry - try { + + // send the error to sentry this.raven.captureException(error, { tags, extra, level }) + // put a flag on the errors to avoid reporting them multiple times - return (() => { - const result = [] - for (key in attributes) { - value = attributes[key] - if (value instanceof Error) { - result.push((value.reportedToSentry = true)) - } else { - result.push(undefined) - } + const result = [] + for (key in attributes) { + value = attributes[key] + if (value instanceof Error) { + value.reportedToSentry = true } return result - })() - } catch (error2) { - return + } + } catch (err) { + // ignore Raven errors } } }, @@ -191,7 +185,7 @@ const Logger = module.exports = { error(attributes, message, ...args) { if (this.ringBuffer !== null && Array.isArray(this.ringBuffer.records)) { - attributes.logBuffer = this.ringBuffer.records.filter(function (record) { + attributes.logBuffer = this.ringBuffer.records.filter(function(record) { return record.level !== 50 }) } @@ -245,6 +239,6 @@ const Logger = module.exports = { return callback() } } -} +}) Logger.initialize('default-sharelatex') diff --git a/libraries/logger/package.json b/libraries/logger/package.json index 6c18e516d8..09515adc53 100644 --- a/libraries/logger/package.json +++ b/libraries/logger/package.json @@ -32,7 +32,8 @@ "eslint-plugin-node": "^6.0.0", "eslint-plugin-promise": "^3.6.0", "eslint-plugin-standard": "^3.0.1", - "mocha": "5.2.0", + "mocha": "^5.2.0", + "prettier": "^1.18.2", "sandboxed-module": "2.0.3", "sinon": "7.2.3", "sinon-chai": "3.3.0" diff --git a/libraries/logger/test/unit/loggingManagerTests.js b/libraries/logger/test/unit/loggingManagerTests.js index cb4d2ca473..4a55717ec6 100644 --- a/libraries/logger/test/unit/loggingManagerTests.js +++ b/libraries/logger/test/unit/loggingManagerTests.js @@ -7,6 +7,7 @@ const sinonChai = require('sinon-chai') chai.use(sinonChai) chai.should() +const expect = chai.expect const modulePath = path.join(__dirname, '../../logging-manager.js') @@ -15,7 +16,7 @@ describe('LoggingManager', function() { this.start = Date.now() this.clock = sinon.useFakeTimers(this.start) this.captureException = sinon.stub() - this.mockBunyanLogger = { + this.bunyanLogger = { debug: sinon.stub(), error: sinon.stub(), fatal: sinon.stub(), @@ -23,25 +24,28 @@ describe('LoggingManager', function() { level: sinon.stub(), warn: sinon.stub() } - this.mockRavenClient = { + this.ravenClient = { captureException: this.captureException, once: sinon.stub().yields() } + this.Bunyan = { + createLogger: sinon.stub().returns(this.bunyanLogger), + RingBuffer: bunyan.RingBuffer, + stdSerializers: { + req: sinon.stub(), + res: sinon.stub() + } + } + this.Raven = { + Client: sinon.stub().returns(this.ravenClient) + } + this.Request = sinon.stub() this.LoggingManager = SandboxedModule.require(modulePath, { globals: { console, process }, requires: { - bunyan: (this.Bunyan = { - createLogger: sinon.stub().returns(this.mockBunyanLogger), - RingBuffer: bunyan.RingBuffer, - stdSerializers: { - req: sinon.stub(), - res: sinon.stub() - } - }), - raven: (this.Raven = { - Client: sinon.stub().returns(this.mockRavenClient) - }), - request: (this.Request = sinon.stub()) + bunyan: this.Bunyan, + raven: this.Raven, + request: this.Request } }) this.loggerName = 'test' @@ -133,37 +137,37 @@ describe('LoggingManager', function() { it('should log debug', function() { this.logger.debug(this.logArgs) - this.mockBunyanLogger.debug.should.have.been.calledWith(this.logArgs) + this.bunyanLogger.debug.should.have.been.calledWith(this.logArgs) }) it('should log error', function() { this.logger.error(this.logArgs) - this.mockBunyanLogger.error.should.have.been.calledWith(this.logArgs) + this.bunyanLogger.error.should.have.been.calledWith(this.logArgs) }) it('should log fatal', function() { this.logger.fatal(this.logArgs) - this.mockBunyanLogger.fatal.should.have.been.calledWith(this.logArgs) + this.bunyanLogger.fatal.should.have.been.calledWith(this.logArgs) }) it('should log info', function() { this.logger.info(this.logArgs) - this.mockBunyanLogger.info.should.have.been.calledWith(this.logArgs) + this.bunyanLogger.info.should.have.been.calledWith(this.logArgs) }) it('should log warn', function() { this.logger.warn(this.logArgs) - this.mockBunyanLogger.warn.should.have.been.calledWith(this.logArgs) + this.bunyanLogger.warn.should.have.been.calledWith(this.logArgs) }) it('should log err', function() { this.logger.err(this.logArgs) - this.mockBunyanLogger.error.should.have.been.calledWith(this.logArgs) + this.bunyanLogger.error.should.have.been.calledWith(this.logArgs) }) it('should log log', function() { this.logger.log(this.logArgs) - this.mockBunyanLogger.info.should.have.been.calledWith(this.logArgs) + this.bunyanLogger.info.should.have.been.calledWith(this.logArgs) }) }) @@ -253,9 +257,7 @@ describe('LoggingManager', function() { headers: { 'Metadata-Flavor': 'Google' }, - uri: `http://metadata.google.internal/computeMetadata/v1/project/attributes/${ - this.loggerName - }-setLogLevelEndTime` + uri: `http://metadata.google.internal/computeMetadata/v1/project/attributes/${this.loggerName}-setLogLevelEndTime` } this.Request.should.have.been.calledWithMatch(options) }) @@ -267,7 +269,7 @@ describe('LoggingManager', function() { }) it('should only set default level', function() { - this.mockBunyanLogger.level.should.have.been.calledOnce.and.calledWith( + this.bunyanLogger.level.should.have.been.calledOnce.and.calledWith( 'debug' ) }) @@ -280,7 +282,7 @@ describe('LoggingManager', function() { }) it('should only set default level', function() { - this.mockBunyanLogger.level.should.have.been.calledOnce.and.calledWith( + this.bunyanLogger.level.should.have.been.calledOnce.and.calledWith( 'debug' ) }) @@ -293,22 +295,22 @@ describe('LoggingManager', function() { }) it('should only set default level', function() { - this.mockBunyanLogger.level.should.have.been.calledOnce.and.calledWith( + this.bunyanLogger.level.should.have.been.calledOnce.and.calledWith( 'debug' ) }) }) - describe('when time value returned that is less than current time', function() { + describe('when time value returned that is more than current time', function() { describe('when level is already set', function() { beforeEach(function() { - this.mockBunyanLogger.level.returns(10) + this.bunyanLogger.level.returns(10) this.Request.yields(null, { statusCode: 200 }, this.start + 1000) this.logger.checkLogLevel() }) it('should set trace level', function() { - this.mockBunyanLogger.level.should.have.been.calledOnce.and.calledWith( + this.bunyanLogger.level.should.have.been.calledOnce.and.calledWith( 'trace' ) }) @@ -316,13 +318,13 @@ describe('LoggingManager', function() { describe('when level is not already set', function() { beforeEach(function() { - this.mockBunyanLogger.level.returns(20) + this.bunyanLogger.level.returns(20) this.Request.yields(null, { statusCode: 200 }, this.start + 1000) this.logger.checkLogLevel() }) it('should set trace level', function() { - this.mockBunyanLogger.level.should.have.been.calledOnce.and.calledWith( + this.bunyanLogger.level.should.have.been.calledOnce.and.calledWith( 'trace' ) }) @@ -333,16 +335,9 @@ describe('LoggingManager', function() { describe('ringbuffer', function() { beforeEach(function() { this.logBufferMock = [ - { - msg: 'log 1' - }, - { - msg: 'log 2' - }, - { - level: 50, - msg: 'error' - } + { msg: 'log 1' }, + { msg: 'log 2' }, + { level: 50, msg: 'error' } ] }) @@ -359,13 +354,9 @@ describe('LoggingManager', function() { }) it('should include buffered logs in error log and filter out error logs in buffer', function() { - this.mockBunyanLogger.error.lastCall.args[0].logBuffer.should.deep.equal([ - { - msg: 'log 1' - }, - { - msg: 'log 2' - }, + this.bunyanLogger.error.lastCall.args[0].logBuffer.should.deep.equal([ + { msg: 'log 1' }, + { msg: 'log 2' } ]) }) }) @@ -382,8 +373,7 @@ describe('LoggingManager', function() { }) it('should not include buffered logs in error log', function() { - chai.expect(this.mockBunyanLogger.error.lastCall.args[0].logBuffer).be - .undefined + expect(this.bunyanLogger.error.lastCall.args[0].logBuffer).be.undefined }) }) })