Merge pull request #171 from overleaf/bg-fix-format

fix format and lint checks

services/clsi/app.js

@@ -5,7 +5,7 @@
  * DS207: Consider shorter variations of null checks
  * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
  */
-let tenMinutes
+const tenMinutes = 10 * 60 * 1000
 const Metrics = require('metrics-sharelatex')
 Metrics.initialize('clsi')
 
@@ -49,31 +49,29 @@ app.use(function(req, res, next) {
   return next()
 })
 
-app.param('project_id', function(req, res, next, project_id) {
+app.param('project_id', function(req, res, next, projectId) {
-  if (project_id != null ? project_id.match(/^[a-zA-Z0-9_-]+$/) : undefined) {
+  if (projectId != null ? projectId.match(/^[a-zA-Z0-9_-]+$/) : undefined) {
     return next()
   } else {
     return next(new Error('invalid project id'))
   }
 })
 
-app.param('user_id', function(req, res, next, user_id) {
+app.param('user_id', function(req, res, next, userId) {
-  if (user_id != null ? user_id.match(/^[0-9a-f]{24}$/) : undefined) {
+  if (userId != null ? userId.match(/^[0-9a-f]{24}$/) : undefined) {
     return next()
   } else {
     return next(new Error('invalid user id'))
   }
 })
 
-app.param('build_id', function(req, res, next, build_id) {
+app.param('build_id', function(req, res, next, buildId) {
   if (
-    build_id != null
+    buildId != null ? buildId.match(OutputCacheManager.BUILD_REGEX) : undefined
-      ? build_id.match(OutputCacheManager.BUILD_REGEX)
-      : undefined
   ) {
     return next()
   } else {
-    return next(new Error(`invalid build id ${build_id}`))
+    return next(new Error(`invalid build id ${buildId}`))
   }
 })
 
@@ -207,7 +205,6 @@ if (Settings.processLifespanLimitMs) {
   }, Settings.processLifespanLimitMs)
 }
 
-if (Settings.smokeTest) {
 function runSmokeTest() {
   if (Settings.processTooOld) return
   logger.log('running smoke tests')
@@ -216,6 +213,7 @@ if (Settings.smokeTest) {
     setTimeout(runSmokeTest, 30 * 1000)
   })
 }
+if (Settings.smokeTest) {
   runSmokeTest()
 }
 
@@ -308,29 +306,31 @@ const host =
     x1 => x1.host
   ) || 'localhost'
 
-const load_tcp_port = Settings.internal.load_balancer_agent.load_port
+const loadTcpPort = Settings.internal.load_balancer_agent.load_port
-const load_http_port = Settings.internal.load_balancer_agent.local_port
+const loadHttpPort = Settings.internal.load_balancer_agent.local_port
 
 if (!module.parent) {
   // Called directly
-  app.listen(port, host, error =>
+  app.listen(port, host, error => {
+    if (error) {
+      logger.fatal({ error }, `Error starting CLSI on ${host}:${port}`)
+    } else {
       logger.info(`CLSI starting up, listening on ${host}:${port}`)
-  )
-
-  loadTcpServer.listen(load_tcp_port, host, function(error) {
-    if (error != null) {
-      throw error
     }
-    return logger.info(`Load tcp agent listening on load port ${load_tcp_port}`)
   })
 
-  loadHttpServer.listen(load_http_port, host, function(error) {
+  loadTcpServer.listen(loadTcpPort, host, function(error) {
     if (error != null) {
       throw error
     }
-    return logger.info(
+    return logger.info(`Load tcp agent listening on load port ${loadTcpPort}`)
-      `Load http agent listening on load port ${load_http_port}`
+  })
-    )
+
+  loadHttpServer.listen(loadHttpPort, host, function(error) {
+    if (error != null) {
+      throw error
+    }
+    return logger.info(`Load http agent listening on load port ${loadHttpPort}`)
   })
 }
 
@@ -339,7 +339,7 @@ module.exports = app
 setInterval(() => {
   ProjectPersistenceManager.refreshExpiryTimeout()
   ProjectPersistenceManager.clearExpiredProjects()
-}, (tenMinutes = 10 * 60 * 1000))
+}, tenMinutes)
 
 function __guard__(value, transform) {
   return typeof value !== 'undefined' && value !== null

services/clsi/app/js/DockerRunner.js

@@ -412,7 +412,7 @@ module.exports = DockerRunner = {
           })
         }
       )
-    var inspectContainer = (isRetry) =>
+    var inspectContainer = isRetry =>
       container.inspect(function(error, stats) {
         if ((error != null ? error.statusCode : undefined) === 404) {
           return createAndStartContainer()

services/clsi/app/js/LatexRunner.js

@@ -132,7 +132,8 @@ module.exports = LatexRunner = {
         LatexRunner.writeLogOutput(project_id, directory, output, () => {
           return callback(error, output, stats, timings)
         })
-      }))
+      }
+    ))
   },
 
   writeLogOutput(project_id, directory, output, callback) {
@@ -142,9 +143,9 @@ module.exports = LatexRunner = {
     // internal method for writing non-empty log files
     function _writeFile(file, content, cb) {
       if (content && content.length > 0) {
-        fs.writeFile(file, content, (err) => {
+        fs.writeFile(file, content, err => {
           if (err) {
-            logger.error({ project_id, file }, "error writing log file") // don't fail on error
+            logger.error({ project_id, file }, 'error writing log file') // don't fail on error
           }
           cb()
         })
@@ -153,8 +154,8 @@ module.exports = LatexRunner = {
       }
     }
     // write stdout and stderr, ignoring errors
-    _writeFile(Path.join(directory, "output.stdout"), output.stdout, () => {
+    _writeFile(Path.join(directory, 'output.stdout'), output.stdout, () => {
-      _writeFile(Path.join(directory, "output.stderr"), output.stderr, () => {
+      _writeFile(Path.join(directory, 'output.stderr'), output.stderr, () => {
         callback()
       })
     })

services/clsi/app/js/UrlFetcher.js

@@ -12,6 +12,7 @@
  * DS207: Consider shorter variations of null checks
  * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
  */
+let UrlFetcher
 const request = require('request').defaults({ jar: false })
 const fs = require('fs')
 const logger = require('logger-sharelatex')

services/clsi/config/settings.defaults.js

@@ -9,7 +9,7 @@ module.exports = {
       username: 'clsi',
       dialect: 'sqlite',
       storage:
-        process.env.SQLITE_PATH || Path.resolve(__dirname + '/../db/db.sqlite'),
+        process.env.SQLITE_PATH || Path.resolve(__dirname, '../db/db.sqlite'),
       pool: {
         max: 1,
         min: 1
@@ -26,10 +26,10 @@ module.exports = {
     parseInt(process.env.PROCESS_LIFE_SPAN_LIMIT_MS) || 60 * 60 * 24 * 1000 * 2,
 
   path: {
-    compilesDir: Path.resolve(__dirname + '/../compiles'),
+    compilesDir: Path.resolve(__dirname, '../compiles'),
-    clsiCacheDir: Path.resolve(__dirname + '/../cache'),
+    clsiCacheDir: Path.resolve(__dirname, '../cache'),
-    synctexBaseDir(project_id) {
+    synctexBaseDir(projectId) {
-      return Path.join(this.compilesDir, project_id)
+      return Path.join(this.compilesDir, projectId)
     }
   },
 
@@ -63,7 +63,7 @@ module.exports = {
 }
 
 if (process.env.DOCKER_RUNNER) {
-  let seccomp_profile_path
+  let seccompProfilePath
   module.exports.clsi = {
     dockerRunner: process.env.DOCKER_RUNNER === 'true',
     docker: {
@@ -81,16 +81,14 @@ if (process.env.DOCKER_RUNNER) {
   }
 
   try {
-    seccomp_profile_path = Path.resolve(
+    seccompProfilePath = Path.resolve(__dirname, '../seccomp/clsi-profile.json')
-      __dirname + '/../seccomp/clsi-profile.json'
-    )
     module.exports.clsi.docker.seccomp_profile = JSON.stringify(
-      JSON.parse(require('fs').readFileSync(seccomp_profile_path))
+      JSON.parse(require('fs').readFileSync(seccompProfilePath))
     )
   } catch (error) {
     console.log(
       error,
-      `could not load seccom profile from ${seccomp_profile_path}`
+      `could not load seccom profile from ${seccompProfilePath}`
     )
   }
 

services/clsi/test/unit/js/LatexRunnerTests.js

@@ -38,7 +38,7 @@ describe('LatexRunner', function() {
           })
         },
         './CommandRunner': (this.CommandRunner = {}),
-        'fs': (this.fs = {
+        fs: (this.fs = {
           writeFile: sinon.stub().callsArg(2)
         })
       }
@@ -55,7 +55,10 @@ describe('LatexRunner', function() {
 
   return describe('runLatex', function() {
     beforeEach(function() {
-      return (this.CommandRunner.run = sinon.stub().callsArg(6))
+      return (this.CommandRunner.run = sinon.stub().callsArgWith(6, null, {
+        stdout: 'this is stdout',
+        stderr: 'this is stderr'
+      }))
     })
 
     describe('normally', function() {
@@ -74,7 +77,7 @@ describe('LatexRunner', function() {
         )
       })
 
-      return it('should run the latex command', function() {
+      it('should run the latex command', function() {
         return this.CommandRunner.run
           .calledWith(
             this.project_id,
@@ -89,16 +92,10 @@ describe('LatexRunner', function() {
 
       it('should record the stdout and stderr', function() {
         this.fs.writeFile
-          .calledWith(
+          .calledWith(this.directory + '/' + 'output.stdout', 'this is stdout')
-            this.directory + '/' + 'output.stdout',
-            "this is stdout"
-          )
           .should.equal(true)
         this.fs.writeFile
-          .calledWith(
+          .calledWith(this.directory + '/' + 'output.stderr', 'this is stderr')
-            this.directory + '/' + 'output.stderr',
-            "this is stderr"
-          )
           .should.equal(true)
       })
     })

services/clsi/test/unit/js/UrlFetcherTests.js

@@ -48,7 +48,7 @@ describe('UrlFetcher', function() {
     })
 
     it('should call pipeUrlToFile multiple times on error', function(done) {
-      error = new Error("couldn't download file")
+      const error = new Error("couldn't download file")
       this.UrlFetcher.pipeUrlToFile.callsArgWith(2, error)
       this.UrlFetcher.pipeUrlToFileWithRetry(this.url, this.path, err => {
         expect(err).to.equal(error)