From 5368630754fbef79d3a3da540a1a3faa1c9fb384 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Fri, 12 Jun 2020 15:15:27 +0100 Subject: [PATCH 1/4] check output file exists before running synctex --- services/clsi/app/js/CompileManager.js | 119 ++++++++++--------------- 1 file changed, 49 insertions(+), 70 deletions(-) diff --git a/services/clsi/app/js/CompileManager.js b/services/clsi/app/js/CompileManager.js index 614c49aaf7..4b0cff6323 100644 --- a/services/clsi/app/js/CompileManager.js +++ b/services/clsi/app/js/CompileManager.js @@ -430,30 +430,18 @@ module.exports = CompileManager = { const compileDir = getCompileDir(project_id, user_id) const synctex_path = `${base_dir}/output.pdf` const command = ['code', synctex_path, file_path, line, column] - return fse.ensureDir(compileDir, function(error) { + CompileManager._runSynctex(project_id, user_id, command, function( + error, + stdout + ) { if (error != null) { - logger.err( - { error, project_id, user_id, file_name }, - 'error ensuring dir for sync from code' - ) return callback(error) } - return CompileManager._runSynctex(project_id, user_id, command, function( - error, - stdout - ) { - if (error != null) { - return callback(error) - } - logger.log( - { project_id, user_id, file_name, line, column, command, stdout }, - 'synctex code output' - ) - return callback( - null, - CompileManager._parseSynctexFromCodeOutput(stdout) - ) - }) + logger.log( + { project_id, user_id, file_name, line, column, command, stdout }, + 'synctex code output' + ) + return callback(null, CompileManager._parseSynctexFromCodeOutput(stdout)) }) }, @@ -466,53 +454,39 @@ module.exports = CompileManager = { const base_dir = Settings.path.synctexBaseDir(compileName) const synctex_path = `${base_dir}/output.pdf` const command = ['pdf', synctex_path, page, h, v] - return fse.ensureDir(compileDir, function(error) { + CompileManager._runSynctex(project_id, user_id, command, function( + error, + stdout + ) { if (error != null) { - logger.err( - { error, project_id, user_id, file_name }, - 'error ensuring dir for sync to code' - ) return callback(error) } - return CompileManager._runSynctex(project_id, user_id, command, function( - error, - stdout - ) { - if (error != null) { - return callback(error) - } - logger.log( - { project_id, user_id, page, h, v, stdout }, - 'synctex pdf output' - ) - return callback( - null, - CompileManager._parseSynctexFromPdfOutput(stdout, base_dir) - ) - }) + logger.log( + { project_id, user_id, page, h, v, stdout }, + 'synctex pdf output' + ) + return callback( + null, + CompileManager._parseSynctexFromPdfOutput(stdout, base_dir) + ) }) }, - _checkFileExists(path, callback) { + _checkFileExists(dir, filename, callback) { if (callback == null) { callback = function(error) {} } - const synctexDir = Path.dirname(path) - const synctexFile = Path.join(synctexDir, 'output.synctex.gz') - return fs.stat(synctexDir, function(error, stats) { + const file = Path.join(dir, filename) + return fs.stat(dir, function(error, stats) { if ((error != null ? error.code : undefined) === 'ENOENT') { - return callback( - new Errors.NotFoundError('called synctex with no output directory') - ) + return callback(new Errors.NotFoundError('no output directory')) } if (error != null) { return callback(error) } - return fs.stat(synctexFile, function(error, stats) { + return fs.stat(file, function(error, stats) { if ((error != null ? error.code : undefined) === 'ENOENT') { - return callback( - new Errors.NotFoundError('called synctex with no output file') - ) + return callback(new Errors.NotFoundError('no output file')) } if (error != null) { return callback(error) @@ -536,24 +510,29 @@ module.exports = CompileManager = { const directory = getCompileDir(project_id, user_id) const timeout = 60 * 1000 // increased to allow for large projects const compileName = getCompileName(project_id, user_id) - return CommandRunner.run( - compileName, - command, - directory, - Settings.clsi != null ? Settings.clsi.docker.image : undefined, - timeout, - {}, - function(error, output) { - if (error != null) { - logger.err( - { err: error, command, project_id, user_id }, - 'error running synctex' - ) - return callback(error) - } - return callback(null, output.stdout) + CompileManager._checkFileExists(directory, 'output.synctex.gz', error => { + if (error) { + return callback(error) } - ) + return CommandRunner.run( + compileName, + command, + directory, + Settings.clsi != null ? Settings.clsi.docker.image : undefined, + timeout, + {}, + function(error, output) { + if (error != null) { + logger.err( + { err: error, command, project_id, user_id }, + 'error running synctex' + ) + return callback(error) + } + return callback(null, output.stdout) + } + ) + }) }, _parseSynctexFromCodeOutput(output) { From f99023320df362ec82ce7bb12f51e37f59610636 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Fri, 12 Jun 2020 15:15:51 +0100 Subject: [PATCH 2/4] use json parsing in request --- services/clsi/test/acceptance/js/helpers/Client.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/services/clsi/test/acceptance/js/helpers/Client.js b/services/clsi/test/acceptance/js/helpers/Client.js index 9f430e3535..d65941671a 100644 --- a/services/clsi/test/acceptance/js/helpers/Client.js +++ b/services/clsi/test/acceptance/js/helpers/Client.js @@ -81,13 +81,14 @@ module.exports = Client = { file, line, column - } + }, + json: true }, (error, response, body) => { if (error != null) { return callback(error) } - return callback(null, JSON.parse(body)) + return callback(null, body) } ) }, @@ -103,13 +104,14 @@ module.exports = Client = { page, h, v - } + }, + json: true }, (error, response, body) => { if (error != null) { return callback(error) } - return callback(null, JSON.parse(body)) + return callback(null, body) } ) }, From 262ea01911b375d2c25099aa2e99fff647171bb7 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Fri, 12 Jun 2020 15:16:14 +0100 Subject: [PATCH 3/4] add acceptance test for synctex when project/file does not exist --- .../clsi/test/acceptance/js/SynctexTests.js | 102 +++++++++++++++++- 1 file changed, 101 insertions(+), 1 deletion(-) diff --git a/services/clsi/test/acceptance/js/SynctexTests.js b/services/clsi/test/acceptance/js/SynctexTests.js index 4860c6040c..1140f3fade 100644 --- a/services/clsi/test/acceptance/js/SynctexTests.js +++ b/services/clsi/test/acceptance/js/SynctexTests.js @@ -69,7 +69,7 @@ Hello world }) }) - return describe('from pdf to code', function() { + describe('from pdf to code', function() { return it('should return the correct location', function(done) { return Client.syncFromPdf( this.project_id, @@ -88,4 +88,104 @@ Hello world ) }) }) + + describe('when the project directory is not available', function() { + before(function() { + this.other_project_id = Client.randomId() + }) + describe('from code to pdf', function() { + it('should return a 404 response', function(done) { + return Client.syncFromCode( + this.other_project_id, + 'main.tex', + 3, + 5, + (error, body) => { + if (error != null) { + throw error + } + expect(body).to.equal('Not Found') + return done() + } + ) + }) + }) + describe('from pdf to code', function() { + it('should return a 404 response', function(done) { + return Client.syncFromPdf( + this.other_project_id, + 1, + 100, + 200, + (error, body) => { + if (error != null) { + throw error + } + expect(body).to.equal('Not Found') + return done() + } + ) + }) + }) + }) + + describe('when the synctex file is not available', function() { + before(function(done) { + this.broken_project_id = Client.randomId() + const content = 'this is not valid tex' // not a valid tex file + this.request = { + resources: [ + { + path: 'main.tex', + content + } + ] + } + Client.compile( + this.broken_project_id, + this.request, + (error, res, body) => { + this.error = error + this.res = res + this.body = body + return done() + } + ) + }) + + describe('from code to pdf', function() { + it('should return a 404 response', function(done) { + return Client.syncFromCode( + this.broken_project_id, + 'main.tex', + 3, + 5, + (error, body) => { + if (error != null) { + throw error + } + expect(body).to.equal('Not Found') + return done() + } + ) + }) + }) + describe('from pdf to code', function() { + it('should return a 404 response', function(done) { + return Client.syncFromPdf( + this.broken_project_id, + 1, + 100, + 200, + (error, body) => { + if (error != null) { + throw error + } + expect(body).to.equal('Not Found') + return done() + } + ) + }) + }) + }) }) From 0914908676fb16e169a562a2535a091f53784553 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Mon, 15 Jun 2020 11:06:54 +0100 Subject: [PATCH 4/4] downgrade NotFoundError log-level --- services/clsi/app.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/clsi/app.js b/services/clsi/app.js index b22e0a016f..ff5d70e7bd 100644 --- a/services/clsi/app.js +++ b/services/clsi/app.js @@ -228,7 +228,7 @@ app.get('/smoke_test_force', (req, res) => smokeTest.sendNewResult(res)) app.use(function(error, req, res, next) { if (error instanceof Errors.NotFoundError) { - logger.warn({ err: error, url: req.url }, 'not found error') + logger.log({ err: error, url: req.url }, 'not found error') return res.sendStatus(404) } else if (error.code === 'EPIPE') { // inspect container returns EPIPE when shutting down