Merge pull request #8286 from overleaf/em-halt-on-error-backend

Stop on first error backend implementation GitOrigin-RevId: 497b1ed2c13f544760d8ad8d029359db75275389
2025-03-21 15:51:14 +00:00 · 2022-06-06 07:41:36 -04:00 · 2022-06-06 07:41:36 -04:00 · baaf4d4240
commit baaf4d4240
parent bda307fbb5
10 changed files with 228 additions and 177 deletions
--- a/services/clsi/app/js/CompileManager.js
+++ b/services/clsi/app/js/CompileManager.js
@ -198,6 +198,7 @@ function doCompile(request, callback) {
              flags: request.flags,
              environment: env,
              compileGroup: request.compileGroup,
+              stopOnFirstError: request.stopOnFirstError,
            },
            (error, output, stats, timings) => {
              // request was for validation only
--- a/services/clsi/app/js/LatexRunner.js
+++ b/services/clsi/app/js/LatexRunner.js
@ -12,24 +12,25 @@ const TIME_V_METRICS = Object.entries({
  'sys-time': /System time.*: (\d+.\d+)/m,
 })

+const COMPILER_FLAGS = {
+  latex: '-pdfdvi',
+  lualatex: '-lualatex',
+  pdflatex: '-pdf',
+  xelatex: '-xelatex',
+}
+
 function runLatex(projectId, options, callback) {
-  let command
-  let {
+  const {
    directory,
    mainFile,
-    compiler,
-    timeout,
    image,
    environment,
    flags,
    compileGroup,
+    stopOnFirstError,
  } = options
-  if (!compiler) {
-    compiler = 'pdflatex'
-  }
-  if (!timeout) {
-    timeout = 60000
-  } // milliseconds
+  const compiler = options.compiler || 'pdflatex'
+  const timeout = options.timeout || 60000 // milliseconds

  logger.debug(
    {
@ -40,28 +41,20 @@ function runLatex(projectId, options, callback) {
      environment,
      flags,
      compileGroup,
+      stopOnFirstError,
    },
    'starting compile'
  )

-  // We want to run latexmk on the tex file which we will automatically
-  // generate from the Rtex/Rmd/md file.
-  mainFile = mainFile.replace(/\.(Rtex|md|Rmd)$/, '.tex')
-
-  if (compiler === 'pdflatex') {
-    command = _pdflatexCommand(mainFile, flags)
-  } else if (compiler === 'latex') {
-    command = _latexCommand(mainFile, flags)
-  } else if (compiler === 'xelatex') {
-    command = _xelatexCommand(mainFile, flags)
-  } else if (compiler === 'lualatex') {
-    command = _lualatexCommand(mainFile, flags)
-  } else {
-    return callback(new Error(`unknown compiler: ${compiler}`))
-  }
-
-  if (Settings.clsi?.strace) {
-    command = ['strace', '-o', 'strace', '-ff'].concat(command)
+  let command
+  try {
+    command = _buildLatexCommand(mainFile, {
+      compiler,
+      stopOnFirstError,
+      flags,
+    })
+  } catch (err) {
+    return callback(err)
  }

  const id = `${projectId}` // record running project under this id
@ -145,49 +138,55 @@ function killLatex(projectId, callback) {
  }
 }

-function _latexmkBaseCommand(flags) {
-  let args = [
+function _buildLatexCommand(mainFile, opts = {}) {
+  const command = []
+
+  if (Settings.clsi?.strace) {
+    command.push('strace', '-o', 'strace', '-ff')
+  }
+
+  if (Settings.clsi?.latexmkCommandPrefix) {
+    command.push(...Settings.clsi.latexmkCommandPrefix)
+  }
+
+  // Basic command and flags
+  command.push(
    'latexmk',
    '-cd',
-    '-f',
    '-jobname=output',
    '-auxdir=$COMPILE_DIR',
    '-outdir=$COMPILE_DIR',
    '-synctex=1',
-    '-interaction=batchmode',
-  ]
-  if (flags) {
-    args = args.concat(flags)
+    '-interaction=batchmode'
+  )
+
+  // Stop on first error option
+  if (opts.stopOnFirstError) {
+    command.push('-halt-on-error')
+  } else {
+    // Run all passes despite errors
+    command.push('-f')
  }
-  return (Settings.clsi?.latexmkCommandPrefix || []).concat(args)
-}

-function _pdflatexCommand(mainFile, flags) {
-  return _latexmkBaseCommand(flags).concat([
-    '-pdf',
-    Path.join('$COMPILE_DIR', mainFile),
-  ])
-}
+  // Extra flags
+  if (opts.flags) {
+    command.push(...opts.flags)
+  }

-function _latexCommand(mainFile, flags) {
-  return _latexmkBaseCommand(flags).concat([
-    '-pdfdvi',
-    Path.join('$COMPILE_DIR', mainFile),
-  ])
-}
+  // TeX Engine selection
+  const compilerFlag = COMPILER_FLAGS[opts.compiler]
+  if (compilerFlag) {
+    command.push(compilerFlag)
+  } else {
+    throw new Error(`unknown compiler: ${opts.compiler}`)
+  }

-function _xelatexCommand(mainFile, flags) {
-  return _latexmkBaseCommand(flags).concat([
-    '-xelatex',
-    Path.join('$COMPILE_DIR', mainFile),
-  ])
-}
+  // We want to run latexmk on the tex file which we will automatically
+  // generate from the Rtex/Rmd/md file.
+  mainFile = mainFile.replace(/\.(Rtex|md|Rmd)$/, '.tex')
+  command.push(Path.join('$COMPILE_DIR', mainFile))

-function _lualatexCommand(mainFile, flags) {
-  return _latexmkBaseCommand(flags).concat([
-    '-lualatex',
-    Path.join('$COMPILE_DIR', mainFile),
-  ])
+  return command
 }

 module.exports = {
--- a/services/clsi/app/js/RequestParser.js
+++ b/services/clsi/app/js/RequestParser.js
@ -61,6 +61,14 @@ function parse(body, callback) {
      default: false,
      type: 'boolean',
    })
+    response.stopOnFirstError = _parseAttribute(
+      'stopOnFirstError',
+      compile.options.stopOnFirstError,
+      {
+        default: false,
+        type: 'boolean',
+      }
+    )
    response.check = _parseAttribute('check', compile.options.check, {
      type: 'string',
    })
--- a/services/clsi/test/unit/js/CompileManagerTests.js
+++ b/services/clsi/test/unit/js/CompileManagerTests.js
@ -135,6 +135,7 @@ describe('CompileManager', function () {
        imageName: (this.image = 'example.com/image'),
        flags: (this.flags = ['-file-line-error']),
        compileGroup: (this.compileGroup = 'compile-group'),
+        stopOnFirstError: false,
      }
      this.env = {}
    })
@ -187,6 +188,7 @@ describe('CompileManager', function () {
            flags: this.flags,
            environment: this.env,
            compileGroup: this.compileGroup,
+            stopOnFirstError: this.request.stopOnFirstError,
          })
          .should.equal(true)
      })
@ -240,6 +242,7 @@ describe('CompileManager', function () {
              CHKTEX_ULIMIT_OPTIONS: '-t 5 -v 64000',
            },
            compileGroup: this.compileGroup,
+            stopOnFirstError: this.request.stopOnFirstError,
          })
          .should.equal(true)
      })
@ -263,6 +266,7 @@ describe('CompileManager', function () {
            flags: this.flags,
            environment: this.env,
            compileGroup: this.compileGroup,
+            stopOnFirstError: this.request.stopOnFirstError,
          })
          .should.equal(true)
      })
--- a/services/clsi/test/unit/js/LatexRunnerTests.js
+++ b/services/clsi/test/unit/js/LatexRunnerTests.js
@ -1,29 +1,34 @@
 const SandboxedModule = require('sandboxed-module')
 const sinon = require('sinon')
 const { expect } = require('chai')
-const modulePath = require('path').join(
+
+const MODULE_PATH = require('path').join(
  __dirname,
  '../../../app/js/LatexRunner'
 )

 describe('LatexRunner', function () {
  beforeEach(function () {
-    this.LatexRunner = SandboxedModule.require(modulePath, {
+    this.Settings = {
+      docker: {
+        socketPath: '/var/run/docker.sock',
+      },
+    }
+    this.commandRunnerOutput = {
+      stdout: 'this is stdout',
+      stderr: 'this is stderr',
+    }
+    this.CommandRunner = {
+      run: sinon.stub().yields(null, this.commandRunnerOutput),
+    }
+    this.fs = {
+      writeFile: sinon.stub().yields(),
+    }
+    this.LatexRunner = SandboxedModule.require(MODULE_PATH, {
      requires: {
-        '@overleaf/settings': (this.Settings = {
-          docker: {
-            socketPath: '/var/run/docker.sock',
-          },
-        }),
-        './Metrics': {
-          Timer: class Timer {
-            done() {}
-          },
-        },
-        './CommandRunner': (this.CommandRunner = {}),
-        fs: (this.fs = {
-          writeFile: sinon.stub().callsArg(2),
-        }),
+        '@overleaf/settings': this.Settings,
+        './CommandRunner': this.CommandRunner,
+        fs: this.fs,
      },
    })

@ -35,57 +40,70 @@ describe('LatexRunner', function () {
    this.callback = sinon.stub()
    this.project_id = 'project-id-123'
    this.env = { foo: '123' }
+    this.timeout = 42000
+    this.flags = []
+    this.stopOnFirstError = false
+
+    this.call = function (callback) {
+      this.LatexRunner.runLatex(
+        this.project_id,
+        {
+          directory: this.directory,
+          mainFile: this.mainFile,
+          compiler: this.compiler,
+          timeout: this.timeout,
+          image: this.image,
+          environment: this.env,
+          compileGroup: this.compileGroup,
+          flags: this.flags,
+          stopOnFirstError: this.stopOnFirstError,
+        },
+        (error, output, stats, timings) => {
+          this.timings = timings
+          callback(error)
+        }
+      )
+    }
  })

  describe('runLatex', function () {
-    beforeEach(function () {
-      this.CommandRunner.run = sinon.stub().callsArgWith(7, null, {
-        stdout: 'this is stdout',
-        stderr: 'this is stderr',
-      })
-    })
-
    describe('normally', function () {
      beforeEach(function (done) {
-        this.LatexRunner.runLatex(
-          this.project_id,
-          {
-            directory: this.directory,
-            mainFile: this.mainFile,
-            compiler: this.compiler,
-            timeout: (this.timeout = 42000),
-            image: this.image,
-            environment: this.env,
-            compileGroup: this.compileGroup,
-          },
-          (error, output, stats, timings) => {
-            this.timings = timings
-            done(error)
-          }
-        )
+        this.call(done)
      })

      it('should run the latex command', function () {
-        this.CommandRunner.run
-          .calledWith(
-            this.project_id,
-            sinon.match.any,
-            this.directory,
-            this.image,
-            this.timeout,
-            this.env,
-            this.compileGroup
-          )
-          .should.equal(true)
+        this.CommandRunner.run.should.have.been.calledWith(
+          this.project_id,
+          [
+            'latexmk',
+            '-cd',
+            '-jobname=output',
+            '-auxdir=$COMPILE_DIR',
+            '-outdir=$COMPILE_DIR',
+            '-synctex=1',
+            '-interaction=batchmode',
+            '-f',
+            '-pdf',
+            '$COMPILE_DIR/main-file.tex',
+          ],
+          this.directory,
+          this.image,
+          this.timeout,
+          this.env,
+          this.compileGroup
+        )
      })

      it('should record the stdout and stderr', function () {
-        this.fs.writeFile
-          .calledWith(this.directory + '/' + 'output.stdout', 'this is stdout')
-          .should.equal(true)
-        this.fs.writeFile
-          .calledWith(this.directory + '/' + 'output.stderr', 'this is stderr')
-          .should.equal(true)
+        this.fs.writeFile.should.have.been.calledWith(
+          this.directory + '/' + 'output.stdout',
+          'this is stdout'
+        )
+        this.fs.writeFile.should.have.been.calledWith(
+          this.directory + '/' + 'output.stderr',
+          'this is stderr'
+        )
      })

      it('should not record cpu metrics', function () {
@ -95,32 +113,36 @@ describe('LatexRunner', function () {
      })
    })

+    describe('with a different compiler', function () {
+      beforeEach(function (done) {
+        this.compiler = 'lualatex'
+        this.call(done)
+      })
+
+      it('should set the appropriate latexmk flag', function () {
+        this.CommandRunner.run.should.have.been.calledWith(this.project_id, [
+          'latexmk',
+          '-cd',
+          '-jobname=output',
+          '-auxdir=$COMPILE_DIR',
+          '-outdir=$COMPILE_DIR',
+          '-synctex=1',
+          '-interaction=batchmode',
+          '-f',
+          '-lualatex',
+          '$COMPILE_DIR/main-file.tex',
+        ])
+      })
+    })
+
    describe('with time -v', function () {
      beforeEach(function (done) {
-        this.CommandRunner.run = sinon.stub().callsArgWith(7, null, {
-          stdout: 'this is stdout',
-          stderr:
-            '\tCommand being timed: "sh -c timeout 1 yes > /dev/null"\n' +
-            '\tUser time (seconds): 0.28\n' +
-            '\tSystem time (seconds): 0.70\n' +
-            '\tPercent of CPU this job got: 98%\n',
-        })
-        this.LatexRunner.runLatex(
-          this.project_id,
-          {
-            directory: this.directory,
-            mainFile: this.mainFile,
-            compiler: this.compiler,
-            timeout: (this.timeout = 42000),
-            image: this.image,
-            environment: this.env,
-            compileGroup: this.compileGroup,
-          },
-          (error, output, stats, timings) => {
-            this.timings = timings
-            done(error)
-          }
-        )
+        this.commandRunnerOutput.stderr =
+          '\tCommand being timed: "sh -c timeout 1 yes > /dev/null"\n' +
+          '\tUser time (seconds): 0.28\n' +
+          '\tSystem time (seconds): 0.70\n' +
+          '\tPercent of CPU this job got: 98%\n'
+        this.call(done)
      })

      it('should record cpu metrics', function () {
@ -131,18 +153,9 @@ describe('LatexRunner', function () {
    })

    describe('with an .Rtex main file', function () {
-      beforeEach(function () {
-        this.LatexRunner.runLatex(
-          this.project_id,
-          {
-            directory: this.directory,
-            mainFile: 'main-file.Rtex',
-            compiler: this.compiler,
-            image: this.image,
-            timeout: (this.timeout = 42000),
-          },
-          this.callback
-        )
+      beforeEach(function (done) {
+        this.mainFile = 'main-file.Rtex'
+        this.call(done)
      })

      it('should run the latex command on the equivalent .tex file', function () {
@ -153,19 +166,9 @@ describe('LatexRunner', function () {
    })

    describe('with a flags option', function () {
-      beforeEach(function () {
-        this.LatexRunner.runLatex(
-          this.project_id,
-          {
-            directory: this.directory,
-            mainFile: this.mainFile,
-            compiler: this.compiler,
-            image: this.image,
-            timeout: (this.timeout = 42000),
-            flags: ['-shell-restricted', '-halt-on-error'],
-          },
-          this.callback
-        )
+      beforeEach(function (done) {
+        this.flags = ['-shell-restricted', '-halt-on-error']
+        this.call(done)
      })

      it('should include the flags in the command', function () {
@ -178,5 +181,27 @@ describe('LatexRunner', function () {
        flags[1].should.equal('-halt-on-error')
      })
    })
+
+    describe('with the stopOnFirstError option', function () {
+      beforeEach(function (done) {
+        this.stopOnFirstError = true
+        this.call(done)
+      })
+
+      it('should set the appropriate flags', function () {
+        this.CommandRunner.run.should.have.been.calledWith(this.project_id, [
+          'latexmk',
+          '-cd',
+          '-jobname=output',
+          '-auxdir=$COMPILE_DIR',
+          '-outdir=$COMPILE_DIR',
+          '-synctex=1',
+          '-interaction=batchmode',
+          '-halt-on-error',
+          '-pdf',
+          '$COMPILE_DIR/main-file.tex',
+        ])
+      })
+    })
  })
 })
--- a/services/web/app/src/Features/Compile/ClsiManager.js
+++ b/services/web/app/src/Features/Compile/ClsiManager.js
@ -871,7 +871,8 @@ const ClsiManager = {
          compiler: project.compiler,
          timeout: options.timeout,
          imageName: project.imageName,
-          draft: !!options.draft,
+          draft: Boolean(options.draft),
+          stopOnFirstError: Boolean(options.stopOnFirstError),
          check: options.check,
          syncType: options.syncType,
          syncState: options.syncState,
--- a/services/web/app/src/Features/Compile/CompileController.js
+++ b/services/web/app/src/Features/Compile/CompileController.js
@ -54,6 +54,9 @@ module.exports = CompileController = {
    if (req.body.draft) {
      options.draft = req.body.draft
    }
+    if (req.body.stopOnFirstError) {
+      options.stopOnFirstError = req.body.stopOnFirstError
+    }
    if (['validate', 'error', 'silent'].includes(req.body.check)) {
      options.check = req.body.check
    }
--- a/services/web/frontend/js/features/pdf-preview/util/compiler.js
+++ b/services/web/frontend/js/features/pdf-preview/util/compiler.js
@ -41,6 +41,7 @@ export default class DocumentCompiler {
    this.currentDoc = null
    this.error = undefined
    this.timer = 0
+    this.stopOnFirstError = false

    this.debouncedAutoCompile = debounce(
      () => {
@ -82,20 +83,22 @@ export default class DocumentCompiler {

      const t0 = performance.now()

+      const body = {
+        rootDoc_id: this.getRootDocOverrideId(),
+        draft: this.draft,
+        check: 'silent', // NOTE: 'error' and 'validate' are possible, but unused
+        // use incremental compile for all users but revert to a full compile
+        // if there was previously a server error
+        incrementalCompilesEnabled: !this.error,
+      }
+      if (getMeta('ol-showStopOnFirstError')) {
+        body.stopOnFirstError = this.stopOnFirstError
+      }
      const data = await postJSON(
        `/project/${this.projectId}/compile?${params}`,
-        {
-          body: {
-            rootDoc_id: this.getRootDocOverrideId(),
-            draft: this.draft,
-            check: 'silent', // NOTE: 'error' and 'validate' are possible, but unused
-            // use incremental compile for all users but revert to a full compile
-            // if there was previously a server error
-            incrementalCompilesEnabled: !this.error,
-          },
-          signal: this.signal,
-        }
+        { body, signal: this.signal }
      )
+
      const compileTimeClientE2E = performance.now() - t0
      const { firstRenderDone } = trackPdfDownload(data, compileTimeClientE2E)
      this.setFirstRenderDone(() => firstRenderDone)
--- a/services/web/frontend/js/shared/context/local-compile-context.js
+++ b/services/web/frontend/js/shared/context/local-compile-context.js
@ -224,6 +224,11 @@ export function LocalCompileProvider({ children }) {
    compiler.draft = draft
  }, [compiler, draft])

+  // keep stop on first error setting in sync with the compiler
+  useEffect(() => {
+    compiler.stopOnFirstError = stopOnFirstError
+  }, [compiler, stopOnFirstError])
+
  // pass the "uncompiled" value up into the scope for use outside this context provider
  useEffect(() => {
    setUncompiled(changedAt > 0)
--- a/services/web/test/unit/src/Compile/ClsiManagerTests.js
+++ b/services/web/test/unit/src/Compile/ClsiManagerTests.js
@ -623,6 +623,7 @@ describe('ClsiManager', function () {
              enablePdfCaching: false,
              flags: undefined,
              metricsMethod: 'standard',
+              stopOnFirstError: false,
            }, // "01234567890abcdef"
            rootResourcePath: 'main.tex',
            resources: [
@ -718,6 +719,7 @@ describe('ClsiManager', function () {
              enablePdfCaching: false,
              flags: undefined,
              metricsMethod: 'priority',
+              stopOnFirstError: false,
            },
            rootResourcePath: 'main.tex',
            resources: [