Small cleanup, satisfy linter, add prettier

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",

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) {
+const errSerializer = function(err) {
-  if (!err || !err.stack)
+  if (!err || !err.stack) {
-    return err;
+    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/${
+      uri: `http://metadata.google.internal/computeMetadata/v1/project/attributes/${this.loggerName}-setLogLevelEndTime`
-        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
+        // send the error to sentry
-      try {
         this.raven.captureException(error, { tags, extra, level })
+
         // put a flag on the errors to avoid reporting them multiple times
-        return (() => {
+        const result = []
-          const result = []
+        for (key in attributes) {
-          for (key in attributes) {
+          value = attributes[key]
-            value = attributes[key]
+          if (value instanceof Error) {
-            if (value instanceof Error) {
+            value.reportedToSentry = true
-              result.push((value.reportedToSentry = true))
-            } else {
-              result.push(undefined)
-            }
           }
           return result
-        })()
+        }
-      } catch (error2) {
+      } catch (err) {
-        return
+        // 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')

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"

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 = {
+        bunyan: this.Bunyan,
-          createLogger: sinon.stub().returns(this.mockBunyanLogger),
+        raven: this.Raven,
-          RingBuffer: bunyan.RingBuffer,
+        request: this.Request
-          stdSerializers: {
-            req: sinon.stub(),
-            res: sinon.stub()
-          }
-        }),
-        raven: (this.Raven = {
-          Client: sinon.stub().returns(this.mockRavenClient)
-        }),
-        request: (this.Request = sinon.stub())
       }
     })
     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/${
+        uri: `http://metadata.google.internal/computeMetadata/v1/project/attributes/${this.loggerName}-setLogLevelEndTime`
-          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 1'
+        { msg: 'log 2' },
-        },
+        { level: 50, msg: 'error' }
-        {
-          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([
+        this.bunyanLogger.error.lastCall.args[0].logBuffer.should.deep.equal([
-          {
+          { msg: 'log 1' },
-            msg: 'log 1'
+          { msg: 'log 2' }
-          },
-          {
-            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
+        expect(this.bunyanLogger.error.lastCall.args[0].logBuffer).be.undefined
-          .undefined
       })
     })
   })