From dab6e9aa8e9c285d95903e7b897a67727106c624 Mon Sep 17 00:00:00 2001 From: Eric Mc Sween Date: Tue, 22 Oct 2019 15:30:14 -0400 Subject: [PATCH] Send output files on timeout The unconventional use of callbacks to return both an error and data after compilation created a subtle bug where the output files were dropped by the LockManager in case of an error such as a timeout. This prevented the frontend to show error logs when a timeout occurs, creating confusion among users. We now attach the output files to the error so that they reach the controller and are sent back to the web service. --- .../clsi/app/coffee/CompileController.coffee | 19 ++++++++++--------- .../clsi/app/coffee/CompileManager.coffee | 5 +++-- services/clsi/app/coffee/DockerRunner.coffee | 10 +++++----- .../acceptance/coffee/TimeoutTests.coffee | 5 ++++- 4 files changed, 22 insertions(+), 17 deletions(-) diff --git a/services/clsi/app/coffee/CompileController.coffee b/services/clsi/app/coffee/CompileController.coffee index 73a7fd1ae5..4952d84568 100644 --- a/services/clsi/app/coffee/CompileController.coffee +++ b/services/clsi/app/coffee/CompileController.coffee @@ -26,21 +26,19 @@ module.exports = CompileController = status = "terminated" else if error?.validate status = "validation-#{error.validate}" + else if error?.timedout + status = "timedout" + logger.log err: error, project_id: request.project_id, "timeout running compile" else if error? - if error.timedout - status = "timedout" - logger.log err: error, project_id: request.project_id, "timeout running compile" - else - status = "error" - code = 500 - logger.warn err: error, project_id: request.project_id, "error running compile" - + status = "error" + code = 500 + logger.warn err: error, project_id: request.project_id, "error running compile" else status = "failure" for file in outputFiles if file.path?.match(/output\.pdf$/) status = "success" - + if status == "failure" logger.warn project_id: request.project_id, outputFiles:outputFiles, "project failed to compile successfully, no output.pdf generated" @@ -49,6 +47,9 @@ module.exports = CompileController = if file.path is "core" logger.error project_id:request.project_id, req:req, outputFiles:outputFiles, "core file found in output" + if error? + outputFiles = error.outputFiles || [] + timer.done() res.status(code or 200).send { compile: diff --git a/services/clsi/app/coffee/CompileManager.coffee b/services/clsi/app/coffee/CompileManager.coffee index 65b78aaff8..792beb8d16 100644 --- a/services/clsi/app/coffee/CompileManager.coffee +++ b/services/clsi/app/coffee/CompileManager.coffee @@ -106,10 +106,11 @@ module.exports = CompileManager = error = new Error("compilation") error.validate = "fail" # compile was killed by user, was a validation, or a compile which failed validation - if error?.terminated or error?.validate + if error?.terminated or error?.validate or error?.timedout OutputFileFinder.findOutputFiles resourceList, compileDir, (err, outputFiles) -> return callback(err) if err? - callback(error, outputFiles) # return output files so user can check logs + error.outputFiles = outputFiles # return output files so user can check logs + callback(error) return # compile completed normally return callback(error) if error? diff --git a/services/clsi/app/coffee/DockerRunner.coffee b/services/clsi/app/coffee/DockerRunner.coffee index 3c2ed9c581..6ea929f249 100644 --- a/services/clsi/app/coffee/DockerRunner.coffee +++ b/services/clsi/app/coffee/DockerRunner.coffee @@ -78,7 +78,7 @@ module.exports = DockerRunner = _callback(args...) # Only call the callback once _callback = () -> - + name = options.name streamEnded = false @@ -115,7 +115,7 @@ module.exports = DockerRunner = _getContainerOptions: (command, image, volumes, timeout, environment) -> timeoutInSeconds = timeout / 1000 - + dockerVolumes = {} for hostVol, dockerVol of volumes dockerVolumes[dockerVol] = {} @@ -148,7 +148,7 @@ module.exports = DockerRunner = "Ulimits": [{'Name': 'cpu', 'Soft': timeoutInSeconds+5, 'Hard': timeoutInSeconds+10}] "CapDrop": "ALL" "SecurityOpt": ["no-new-privileges"] - + if Settings.path?.synctexBinHostPath? options["HostConfig"]["Binds"].push("#{Settings.path.synctexBinHostPath}:/opt/synctex:ro") @@ -276,7 +276,7 @@ module.exports = DockerRunner = logger.log container_id: containerId, "timeout reached, killing container" container.kill(() ->) , timeout - + logger.log container_id: containerId, "waiting for docker container" container.wait (error, res) -> if error? @@ -355,4 +355,4 @@ module.exports = DockerRunner = , oneHour = 60 * 60 * 1000 , randomDelay -DockerRunner.startContainerMonitor() \ No newline at end of file +DockerRunner.startContainerMonitor() diff --git a/services/clsi/test/acceptance/coffee/TimeoutTests.coffee b/services/clsi/test/acceptance/coffee/TimeoutTests.coffee index c3acd8f9f8..b274dd54ff 100644 --- a/services/clsi/test/acceptance/coffee/TimeoutTests.coffee +++ b/services/clsi/test/acceptance/coffee/TimeoutTests.coffee @@ -15,7 +15,7 @@ describe "Timed out compile", -> \\documentclass{article} \\begin{document} \\def\\x{Hello!\\par\\x} - \\x + \\x \\end{document} ''' ] @@ -29,3 +29,6 @@ describe "Timed out compile", -> it "should return a timedout status", -> @body.compile.status.should.equal "timedout" + it "should return the log output file name", -> + outputFilePaths = @body.compile.outputFiles.map((x) => x.path) + outputFilePaths.should.include('output.log')