Merge branch 'master' into bg-add-compile-groups

This commit is contained in:
Brian Gough 2020-06-17 11:58:26 +01:00
commit bdebef91eb
6 changed files with 171 additions and 158 deletions

View file

@ -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

View file

@ -431,30 +431,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))
})
},
@ -467,53 +455,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)
@ -538,25 +512,30 @@ module.exports = CompileManager = {
const timeout = 60 * 1000 // increased to allow for large projects
const compileName = getCompileName(project_id, user_id)
const compileGroup = 'synctex'
return CommandRunner.run(
compileName,
command,
directory,
Settings.clsi != null ? Settings.clsi.docker.image : undefined,
timeout,
{},
compileGroup,
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,
{},
compileGroup,
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) {

View file

@ -26,7 +26,6 @@ const LockManager = require('./DockerLockManager')
const fs = require('fs')
const Path = require('path')
const _ = require('lodash')
const metrics = require('metrics-sharelatex')
logger.info('using docker runner')
@ -446,28 +445,19 @@ module.exports = DockerRunner = {
})
}
)
var inspectContainer = isRetry =>
container.inspect(function(error, stats) {
if ((error != null ? error.statusCode : undefined) === 404) {
return createAndStartContainer()
} else if (error != null) {
if (error.message.match(/EPIPE/)) {
if (!isRetry) {
metrics.inc('container-inspect-epipe-retry')
return inspectContainer(true)
}
metrics.inc('container-inspect-epipe-error')
}
logger.err(
{ container_name: name, error },
'unable to inspect container to start'
)
return callback(error)
} else {
return startExistingContainer()
}
})
inspectContainer(false)
return container.inspect(function(error, stats) {
if ((error != null ? error.statusCode : undefined) === 404) {
return createAndStartContainer()
} else if (error != null) {
logger.err(
{ container_name: name, error },
'unable to inspect container to start'
)
return callback(error)
} else {
return startExistingContainer()
}
})
},
attachToContainer(containerId, attachStreamHandler, attachStartCallback) {

View file

@ -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()
}
)
})
})
})
})

View file

@ -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)
}
)
},

View file

@ -36,7 +36,6 @@ describe('DockerRunner', function() {
'logger-sharelatex': (this.logger = {
log: sinon.stub(),
error: sinon.stub(),
err: sinon.stub(),
info: sinon.stub(),
warn: sinon.stub()
}),
@ -453,63 +452,6 @@ describe('DockerRunner', function() {
})
})
describe('when inspect always fails with EPIPE error', function() {
beforeEach(function() {
this.error = new Error('write EPIPE')
this.container.inspect = sinon.stub().yields(this.error)
this.container.start = sinon.stub().yields()
this.DockerRunner.startContainer(
this.options,
this.volumes,
() => {},
this.callback
)
})
it('should retry once', function() {
sinon.assert.callOrder(
this.container.inspect,
this.container.inspect,
this.callback
)
})
it('should call back with error', function() {
sinon.assert.calledWith(this.callback, this.error)
})
})
describe('when inspect fails once with EPIPE error', function() {
beforeEach(function() {
this.container.inspect = sinon.stub()
this.container.inspect.onFirstCall().yields(new Error('write EPIPE'))
this.container.inspect.onSecondCall().yields()
this.container.start = sinon.stub().yields()
this.DockerRunner.startContainer(
this.options,
this.volumes,
() => {},
this.callback
)
})
it('should retry once and start container', function() {
sinon.assert.callOrder(
this.container.inspect,
this.container.inspect,
this.DockerRunner.attachToContainer,
this.container.start,
this.callback
)
})
it('should call back without error', function() {
sinon.assert.calledWith(this.callback, null)
})
})
describe('when the container does not exist', function() {
beforeEach(function() {
const exists = false