From 17c14b119261da16e5b6b23e0bd19add5c08071c Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Tue, 2 Jun 2020 09:18:38 +0100 Subject: [PATCH 1/4] fix formatting with make format_fix --- app/js/DockerRunner.js | 2 +- app/js/LatexRunner.js | 11 ++++++----- test/unit/js/LatexRunnerTests.js | 14 ++++---------- test/unit/js/ResourceWriterTests.js | 6 +++--- 4 files changed, 14 insertions(+), 19 deletions(-) diff --git a/app/js/DockerRunner.js b/app/js/DockerRunner.js index e7cb2a7..c50087d 100644 --- a/app/js/DockerRunner.js +++ b/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() diff --git a/app/js/LatexRunner.js b/app/js/LatexRunner.js index fe737c7..c3edb29 100644 --- a/app/js/LatexRunner.js +++ b/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.stderr"), output.stderr, () => { + _writeFile(Path.join(directory, 'output.stdout'), output.stdout, () => { + _writeFile(Path.join(directory, 'output.stderr'), output.stderr, () => { callback() }) }) diff --git a/test/unit/js/LatexRunnerTests.js b/test/unit/js/LatexRunnerTests.js index 15902e9..42e816d 100644 --- a/test/unit/js/LatexRunnerTests.js +++ b/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) }) } @@ -87,18 +87,12 @@ describe('LatexRunner', function() { .should.equal(true) }) - it('should record the stdout and stderr', function () { + it('should record the stdout and stderr', function() { this.fs.writeFile - .calledWith( - this.directory + '/' + 'output.stdout', - "this is stdout" - ) + .calledWith(this.directory + '/' + 'output.stdout', 'this is stdout') .should.equal(true) this.fs.writeFile - .calledWith( - this.directory + '/' + 'output.stderr', - "this is stderr" - ) + .calledWith(this.directory + '/' + 'output.stderr', 'this is stderr') .should.equal(true) }) }) diff --git a/test/unit/js/ResourceWriterTests.js b/test/unit/js/ResourceWriterTests.js index 1080765..36a9513 100644 --- a/test/unit/js/ResourceWriterTests.js +++ b/test/unit/js/ResourceWriterTests.js @@ -262,19 +262,19 @@ describe('ResourceWriter', function() { .should.equal(true) }) - it('should delete the stdout log file', function () { + it('should delete the stdout log file', function() { return this.ResourceWriter._deleteFileIfNotDirectory .calledWith(path.join(this.basePath, 'output.stdout')) .should.equal(true) }) - it('should delete the stderr log file', function () { + it('should delete the stderr log file', function() { return this.ResourceWriter._deleteFileIfNotDirectory .calledWith(path.join(this.basePath, 'output.stderr')) .should.equal(true) }) - it('should delete the extra files', function () { + it('should delete the extra files', function() { return this.ResourceWriter._deleteFileIfNotDirectory .calledWith(path.join(this.basePath, 'extra/file.tex')) .should.equal(true) From 440ec5553e233c3cd951f1b2db73225b960c2fb2 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Tue, 2 Jun 2020 09:19:39 +0100 Subject: [PATCH 2/4] fix unreachable code lint error --- test/unit/js/LatexRunnerTests.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/js/LatexRunnerTests.js b/test/unit/js/LatexRunnerTests.js index 42e816d..d9aa398 100644 --- a/test/unit/js/LatexRunnerTests.js +++ b/test/unit/js/LatexRunnerTests.js @@ -74,7 +74,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, From 2211ebcefb897e11f4c78d5492554c9a242241b1 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Tue, 2 Jun 2020 09:51:34 +0100 Subject: [PATCH 3/4] fix eslint errors --- app.js | 66 ++++++++++++++++----------------- app/js/UrlFetcher.js | 1 + config/settings.defaults.js | 20 +++++----- test/unit/js/UrlFetcherTests.js | 2 +- 4 files changed, 44 insertions(+), 45 deletions(-) diff --git a/app.js b/app.js index add5f21..319799a 100644 --- a/app.js +++ b/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) { - if (project_id != null ? project_id.match(/^[a-zA-Z0-9_-]+$/) : undefined) { +app.param('project_id', function(req, res, next, projectId) { + 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) { - if (user_id != null ? user_id.match(/^[0-9a-f]{24}$/) : undefined) { +app.param('user_id', function(req, res, next, userId) { + 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 - ? build_id.match(OutputCacheManager.BUILD_REGEX) - : undefined + buildId != null ? buildId.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,15 +205,15 @@ if (Settings.processLifespanLimitMs) { }, Settings.processLifespanLimitMs) } +function runSmokeTest() { + if (Settings.processTooOld) return + logger.log('running smoke tests') + smokeTest.triggerRun(err => { + if (err) logger.error({ err }, 'smoke tests failed') + setTimeout(runSmokeTest, 30 * 1000) + }) +} if (Settings.smokeTest) { - function runSmokeTest() { - if (Settings.processTooOld) return - logger.log('running smoke tests') - smokeTest.triggerRun(err => { - if (err) logger.error({ err }, 'smoke tests failed') - setTimeout(runSmokeTest, 30 * 1000) - }) - } runSmokeTest() } @@ -308,29 +306,31 @@ const host = x1 => x1.host ) || 'localhost' -const load_tcp_port = Settings.internal.load_balancer_agent.load_port -const load_http_port = Settings.internal.load_balancer_agent.local_port +const loadTcpPort = Settings.internal.load_balancer_agent.load_port +const loadHttpPort = Settings.internal.load_balancer_agent.local_port if (!module.parent) { // Called directly - app.listen(port, host, error => - logger.info(`CLSI starting up, listening on ${host}:${port}`) - ) - - loadTcpServer.listen(load_tcp_port, host, function(error) { - if (error != null) { - throw 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}`) } - 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( - `Load http agent listening on load port ${load_http_port}` - ) + return logger.info(`Load tcp agent listening on load port ${loadTcpPort}`) + }) + + 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 diff --git a/app/js/UrlFetcher.js b/app/js/UrlFetcher.js index 6c7d83a..f9f993f 100644 --- a/app/js/UrlFetcher.js +++ b/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') diff --git a/config/settings.defaults.js b/config/settings.defaults.js index ba63e24..51cb624 100644 --- a/config/settings.defaults.js +++ b/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'), - clsiCacheDir: Path.resolve(__dirname + '/../cache'), - synctexBaseDir(project_id) { - return Path.join(this.compilesDir, project_id) + compilesDir: Path.resolve(__dirname, '../compiles'), + clsiCacheDir: Path.resolve(__dirname, '../cache'), + synctexBaseDir(projectId) { + 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( - __dirname + '/../seccomp/clsi-profile.json' - ) + seccompProfilePath = Path.resolve(__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}` ) } diff --git a/test/unit/js/UrlFetcherTests.js b/test/unit/js/UrlFetcherTests.js index 2479209..57bee3a 100644 --- a/test/unit/js/UrlFetcherTests.js +++ b/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) From bf2430f1fc17254285556f3d9f1e81a6b2cacc4b Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Tue, 2 Jun 2020 11:12:57 +0100 Subject: [PATCH 4/4] fix broken unit test --- test/unit/js/LatexRunnerTests.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/unit/js/LatexRunnerTests.js b/test/unit/js/LatexRunnerTests.js index d9aa398..b112b17 100644 --- a/test/unit/js/LatexRunnerTests.js +++ b/test/unit/js/LatexRunnerTests.js @@ -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() {