From 74b480fc55bd5475cf4c7e39aa724f110967f708 Mon Sep 17 00:00:00 2001 From: Simon Detheridge Date: Fri, 3 Jan 2020 10:06:19 +0000 Subject: [PATCH] Post-decaf cleanup of app.js --- services/filestore/app.js | 34 ++++++- services/filestore/app/js/ExceptionHandler.js | 97 +++++++++++++++++++ .../test/acceptance/js/FilestoreApp.js | 1 + 3 files changed, 131 insertions(+), 1 deletion(-) create mode 100644 services/filestore/app/js/ExceptionHandler.js diff --git a/services/filestore/app.js b/services/filestore/app.js index 232c5b24bc..6bf68f6a34 100644 --- a/services/filestore/app.js +++ b/services/filestore/app.js @@ -12,8 +12,11 @@ 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 ExceptionHandler = require('./app/js/ExceptionHandler') +const exceptionHandler = new ExceptionHandler() const app = express() +app.exceptionHandler = exceptionHandler if (settings.sentry && settings.sentry.dsn) { logger.initializeErrorReporting(settings.sentry.dsn) @@ -31,6 +34,8 @@ app.use(function(req, res, next) { next() }) +exceptionHandler.addMiddleware(app) + Metrics.injectMetricsRoute(app) app.head( @@ -128,12 +133,24 @@ app.get('/heapdump', (req, res, next) => ) ) +app.post('/shutdown', function(req, res) { + exceptionHandler.setNotOk() + res.sendStatus(200) +}) + app.get('/status', function(req, res) { - res.send('filestore sharelatex up') + if (exceptionHandler.appIsOk()) { + res.send('filestore sharelatex up') + } else { + logger.log('app is not ok - shutting down') + res.send('server is being shut down').status(500) + } }) app.get('/health_check', healthCheckController.check) +app.get('*', (req, res) => res.sendStatus(404)) + const port = settings.internal.filestore.port || 3009 const host = '0.0.0.0' @@ -146,6 +163,21 @@ if (!module.parent) { } 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') + exceptionHandler.beginShutdown() +}) + +if (global.gc) { + const oneMinute = 60 * 1000 + const gcTimer = setInterval(function() { + global.gc() + logger.log(process.memoryUsage(), 'global.gc') + }, 3 * oneMinute) + gcTimer.unref() +} diff --git a/services/filestore/app/js/ExceptionHandler.js b/services/filestore/app/js/ExceptionHandler.js new file mode 100644 index 0000000000..122e78805c --- /dev/null +++ b/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 diff --git a/services/filestore/test/acceptance/js/FilestoreApp.js b/services/filestore/test/acceptance/js/FilestoreApp.js index 718d53bcf8..32ec7a5adf 100644 --- a/services/filestore/test/acceptance/js/FilestoreApp.js +++ b/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') {