[clsi] gracefully handle fast exit of synctex/wordcount containers (#29505)

* [clsi] gracefully handle fast exit of synctex/wordcount containers

* [clsi] do not change container options in-place for logging

GitOrigin-RevId: 0b685310a3c72f8f46125fefaa30c1ddb19e7b07

services/clsi/app/js/DockerRunner.js

@@ -176,29 +176,35 @@ const DockerRunner = {
           return callback(error)
         }
 
-        DockerRunner.waitForContainer(name, timeout, (error, exitCode) => {
-          if (error != null) {
-            return callback(error)
+        DockerRunner.waitForContainer(
+          name,
+          timeout,
+          options,
+          (error, exitCode) => {
+            if (error != null) {
+              return callback(error)
+            }
+            if (exitCode === 137) {
+              // exit status from kill -9
+              const err = new Error('terminated')
+              err.terminated = true
+              return callback(err)
+            }
+            if (exitCode === 1) {
+              // exit status from chktex
+              const err = new Error('exited')
+              err.code = exitCode
+              return callback(err)
+            }
+            containerReturned = true
+            logger.debug(
+              // The seccomp policy is very large. Avoid logging it. _.omit deep clones.
+              { exitCode, options: _.omit(options, 'HostConfig.SecurityOpt') },
+              'docker container has exited'
+            )
+            callbackIfFinished()
           }
-          if (exitCode === 137) {
-            // exit status from kill -9
-            const err = new Error('terminated')
-            err.terminated = true
-            return callback(err)
-          }
-          if (exitCode === 1) {
-            // exit status from chktex
-            const err = new Error('exited')
-            err.code = exitCode
-            return callback(err)
-          }
-          containerReturned = true
-          if (options != null && options.HostConfig != null) {
-            options.HostConfig.SecurityOpt = null
-          }
-          logger.debug({ exitCode, options }, 'docker container has exited')
-          callbackIfFinished()
-        })
+        )
       }
     )
   },
@@ -429,7 +435,7 @@ const DockerRunner = {
     })
   },
 
-  waitForContainer(containerId, timeout, _callback) {
+  waitForContainer(containerId, timeout, options, _callback) {
     const callback = _.once(_callback)
 
     const container = dockerode.getContainer(containerId)
@@ -445,6 +451,14 @@ const DockerRunner = {
 
     logger.debug({ containerId }, 'waiting for docker container')
     container.wait((error, res) => {
+      if (error?.statusCode === 404 && options.HostConfig.AutoRemove) {
+        logger.debug(
+          { containerId },
+          'auto-destroy container destroyed before starting to wait'
+        )
+        clearTimeout(timeoutId)
+        return callback(null, 0)
+      }
       if (error != null) {
         clearTimeout(timeoutId)
         logger.warn({ err: error, containerId }, 'error waiting for container')

services/clsi/test/unit/js/DockerRunnerTests.js

@@ -514,7 +514,7 @@ describe('DockerRunner', function () {
       sinon.spy(this.DockerRunner, 'startContainer')
       this.DockerRunner.waitForContainer = sinon
         .stub()
-        .callsArgWith(2, null, (this.exitCode = 42))
+        .callsArgWith(3, null, (this.exitCode = 42))
       return this.DockerRunner._runAndWaitForContainer(
         this.options,
         this.volumes,
@@ -675,6 +675,7 @@ describe('DockerRunner', function () {
         return this.DockerRunner.waitForContainer(
           this.containerId,
           this.timeout,
+          {},
           this.callback
         )
       })
@@ -691,6 +692,49 @@ describe('DockerRunner', function () {
       })
     })
 
+    describe('when the container is removed before waiting', function () {
+      const err = new Error('not found')
+      err.statusCode = 404
+      beforeEach(function () {
+        this.container.wait = sinon.stub().yields(err)
+      })
+
+      describe('AutoRemove not set', function () {
+        beforeEach(function () {
+          this.DockerRunner.waitForContainer(
+            this.containerId,
+            this.timeout,
+            { HostConfig: {} },
+            this.callback
+          )
+        })
+        it('should wait for the container', function () {
+          this.getContainer.calledWith(this.containerId).should.equal(true)
+          this.container.wait.called.should.equal(true)
+        })
+        it('should call the callback with the error', function () {
+          this.callback.calledWith(err).should.equal(true)
+        })
+      })
+      describe('AutoRemove=true', function () {
+        beforeEach(function () {
+          this.DockerRunner.waitForContainer(
+            this.containerId,
+            this.timeout,
+            { HostConfig: { AutoRemove: true } },
+            this.callback
+          )
+        })
+        it('should wait for the container', function () {
+          this.getContainer.calledWith(this.containerId).should.equal(true)
+          this.container.wait.called.should.equal(true)
+        })
+        it('should call the callback with exit code 0', function () {
+          this.callback.calledWith(null, 0).should.equal(true)
+        })
+      })
+    })
+
     return describe('when the container does not return before the timeout', function () {
       beforeEach(function (done) {
         this.container.wait = function (callback) {
@@ -703,6 +747,7 @@ describe('DockerRunner', function () {
         return this.DockerRunner.waitForContainer(
           this.containerId,
           this.timeout,
+          {},
           (...args) => {
             this.callback(...Array.from(args || []))
             return done()