Post-decaf cleanup of app.js

services/filestore/app.js

@@ -1,89 +1,40 @@
-/*
- * decaffeinate suggestions:
- * DS102: Remove unnecessary code created because of implicit returns
- * DS207: Consider shorter variations of null checks
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
-/* eslint-disable node/no-deprecated-api */
 const Metrics = require('metrics-sharelatex')
+const logger = require('logger-sharelatex')
+
 Metrics.initialize('filestore')
-const express = require('express')
-const bodyParser = require('body-parser')
-let logger = require('logger-sharelatex')
 logger.initialize('filestore')
+
 const settings = require('settings-sharelatex')
+const express = require('express')
+const bodyParser = require('body-parser')
+
 const fileController = require('./app/js/FileController')
 const bucketController = require('./app/js/BucketController')
 const keyBuilder = require('./app/js/KeyBuilder')
 const healthCheckController = require('./app/js/HealthCheckController')
-const domain = require('domain')
-let appIsOk = true
+const ExceptionHandler = require('./app/js/ExceptionHandler')
+const exceptionHandler = new ExceptionHandler()
+
 const app = express()
+app.exceptionHandler = exceptionHandler
 
-if ((settings.sentry != null ? settings.sentry.dsn : undefined) != null) {
+if (settings.sentry && settings.sentry.dsn) {
   logger.initializeErrorReporting(settings.sentry.dsn)
 }
 
 Metrics.open_sockets.monitor(logger)
-if (Metrics.event_loop != null) {
+Metrics.memory.monitor(logger)
+if (Metrics.event_loop) {
   Metrics.event_loop.monitor(logger)
 }
-Metrics.memory.monitor(logger)
 
 app.use(Metrics.http.monitor(logger))
-
 app.use(function(req, res, next) {
   Metrics.inc('http-request')
-  return next()
-})
-
-app.use(function(req, res, next) {
-  const requestDomain = domain.create()
-  requestDomain.add(req)
-  requestDomain.add(res)
-  requestDomain.on('error', function(err) {
-    try {
-      // request a shutdown to prevent memory leaks
-      beginShutdown()
-      if (!res.headerSent) {
-        res.send(500, 'uncaught exception')
-      }
-      logger = require('logger-sharelatex')
-      req = {
-        body: req.body,
-        headers: req.headers,
-        url: req.url,
-        key: req.key,
-        statusCode: req.statusCode
-      }
-      err = {
-        message: err.message,
-        stack: err.stack,
-        name: err.name,
-        type: err.type,
-        arguments: err.arguments
-      }
-      return logger.err(
-        { err, req, res },
-        'uncaught exception thrown on request'
-      )
-    } catch (exception) {
-      return logger.err(
-        { err: exception },
-        'exception in request domain handler'
-      )
-    }
-  })
-  return requestDomain.run(next)
+  next()
 })
 
-app.use(function(req, res, next) {
-  if (!appIsOk) {
-    // when shutting down, close any HTTP keep-alive connections
-    res.set('Connection', 'close')
-  }
-  return next()
-})
+exceptionHandler.addMiddleware(app)
 
 Metrics.injectMetricsRoute(app)
 
@@ -108,7 +59,7 @@ app.put(
   bodyParser.json(),
   fileController.copyFile
 )
-app.del(
+app.delete(
   '/project/:project_id/file/:file_id',
   keyBuilder.userFileKeyMiddleware,
   fileController.deleteFile
@@ -156,7 +107,7 @@ app.put(
   bodyParser.json(),
   fileController.copyFile
 )
-app.del(
+app.delete(
   '/project/:project_id/public/:public_file_id',
   keyBuilder.publicFileKeyMiddleware,
   fileController.deleteFile
@@ -183,68 +134,50 @@ app.get('/heapdump', (req, res, next) =>
 )
 
 app.post('/shutdown', function(req, res) {
-  appIsOk = false
-  return res.send()
+  exceptionHandler.setNotOk()
+  res.sendStatus(200)
 })
 
 app.get('/status', function(req, res) {
-  if (appIsOk) {
-    return res.send('filestore sharelatex up')
+  if (exceptionHandler.appIsOk()) {
+    res.send('filestore sharelatex up')
   } else {
     logger.log('app is not ok - shutting down')
-    return res.send('server is being shut down', 500)
+    res.send('server is being shut down').status(500)
   }
 })
 
 app.get('/health_check', healthCheckController.check)
 
-app.get('*', (req, res) => res.send(404))
-
-var beginShutdown = function() {
-  if (appIsOk) {
-    appIsOk = false
-    // hard-terminate this process if graceful shutdown fails
-    const killTimer = setTimeout(() => process.exit(1), 120 * 1000)
-    if (typeof killTimer.unref === 'function') {
-      killTimer.unref()
-    } // prevent timer from keeping process alive
-    server.close(function() {
-      logger.log('closed all connections')
-      Metrics.close()
-      return typeof process.disconnect === 'function'
-        ? process.disconnect()
-        : undefined
-    })
-    return logger.log('server will stop accepting connections')
-  }
-}
+app.get('*', (req, res) => res.sendStatus(404))
 
 const port = settings.internal.filestore.port || 3009
 const host = '0.0.0.0'
 
 if (!module.parent) {
   // Called directly
-  var server = app.listen(port, host, error => {
+  const server = app.listen(port, host, error => {
     if (error) {
       logger.error('Error starting Filestore', error)
       throw error
     }
     logger.info(`Filestore starting up, listening on ${host}:${port}`)
   })
+  exceptionHandler.server = server
 }
 
 module.exports = app
 
 process.on('SIGTERM', function() {
   logger.log('filestore got SIGTERM, shutting down gracefully')
-  return beginShutdown()
+  exceptionHandler.beginShutdown()
 })
 
-if (global.gc != null) {
+if (global.gc) {
   const oneMinute = 60 * 1000
   const gcTimer = setInterval(function() {
     global.gc()
-    return logger.log(process.memoryUsage(), 'global.gc')
+    logger.log(process.memoryUsage(), 'global.gc')
   }, 3 * oneMinute)
   gcTimer.unref()
 }

services/filestore/app/js/ExceptionHandler.js

@@ -0,0 +1,97 @@
+const Metrics = require('metrics-sharelatex')
+const logger = require('logger-sharelatex')
+
+// TODO: domain has been deprecated for some time - do we need it and is there a better way?
+
+// eslint-disable-next-line node/no-deprecated-api
+const domain = require('domain')
+
+const TWO_MINUTES = 120 * 1000
+
+class ExceptionHandler {
+  constructor() {
+    this._appIsOk = true
+  }
+
+  beginShutdown() {
+    if (this._appIsOk) {
+      this._appIsOk = false
+
+      // hard-terminate this process if graceful shutdown fails
+      const killTimer = setTimeout(() => process.exit(1), TWO_MINUTES)
+
+      if (typeof killTimer.unref === 'function') {
+        killTimer.unref()
+      } // prevent timer from keeping process alive
+
+      this.server.close(function() {
+        logger.log('closed all connections')
+        Metrics.close()
+        if (typeof process.disconnect === 'function') {
+          process.disconnect()
+        }
+      })
+      logger.log('server will stop accepting connections')
+    }
+  }
+
+  addMiddleware(app) {
+    app.use(this.middleware.bind(this))
+  }
+
+  appIsOk() {
+    return this._appIsOk
+  }
+
+  setNotOk() {
+    this._appIsOk = false
+  }
+
+  middleware(req, res, next) {
+    const rescueLogger = require('logger-sharelatex')
+    const requestDomain = domain.create()
+    requestDomain.add(req)
+    requestDomain.add(res)
+    requestDomain.on('error', err => {
+      try {
+        // request a shutdown to prevent memory leaks
+        this.beginShutdown()
+        if (!res.headerSent) {
+          res.send('uncaught exception').status(500)
+        }
+        req = {
+          body: req.body,
+          headers: req.headers,
+          url: req.url,
+          key: req.key,
+          statusCode: req.statusCode
+        }
+        err = {
+          message: err.message,
+          stack: err.stack,
+          name: err.name,
+          type: err.type,
+          arguments: err.arguments
+        }
+        rescueLogger.err(
+          { err, req, res },
+          'uncaught exception thrown on request'
+        )
+      } catch (exception) {
+        rescueLogger.err(
+          { err: exception },
+          'exception in request domain handler'
+        )
+      }
+    })
+
+    if (!this._appIsOk) {
+      // when shutting down, close any HTTP keep-alive connections
+      res.set('Connection', 'close')
+    }
+
+    requestDomain.run(next)
+  }
+}
+
+module.exports = ExceptionHandler

services/filestore/test/acceptance/js/FilestoreApp.js

@@ -44,6 +44,7 @@ class FilestoreApp {
           resolve()
         }
       )
+      this.app.exceptionHandler.server = this.server
     })
 
     if (Settings.filestore.backend === 's3') {