Merge pull request #4342 from overleaf/jpa-fix-clsi-server-id-handling

[misc] get current clsi server id directly from compile response

GitOrigin-RevId: 403e7ca35270a8937a0066fe2c8daf52ed71531d

services/web/app/src/Features/Compile/ClsiCookieManager.js

@@ -99,7 +99,7 @@ module.exports = function (backendGroup) {
         return rclient.expire(
           this.buildKey(project_id),
           Settings.clsiCookie.ttl,
-          callback
+          err => callback(err, undefined)
         )
       }
       if (rclient_secondary != null) {
@@ -134,10 +134,10 @@ module.exports = function (backendGroup) {
 
     getCookieJar(project_id, callback) {
       if (callback == null) {
-        callback = function (err, jar) {}
+        callback = function (err, jar, clsiServerId) {}
       }
       if (!clsiCookiesEnabled) {
-        return callback(null, request.jar())
+        return callback(null, request.jar(), undefined)
       }
       return this._getServerId(project_id, (err, serverId) => {
         if (err != null) {
@@ -151,7 +151,7 @@ module.exports = function (backendGroup) {
         )
         const jar = request.jar()
         jar.setCookie(serverCookie, Settings.apis.clsi.url)
-        return callback(null, jar)
+        return callback(null, jar, serverId)
       })
     },
   }

services/web/app/src/Features/Compile/ClsiManager.js

@@ -237,7 +237,7 @@ const ClsiManager = {
           userId,
           req,
           options.compileGroup,
-          (err, response) => {
+          (err, response, clsiServerId) => {
             if (err != null) {
               return callback(
                 OError.tag(err, 'error sending request to clsi', {
@@ -246,31 +246,24 @@ const ClsiManager = {
                 })
               )
             }
-            ClsiCookieManager._getServerId(projectId, (err, clsiServerId) => {
-              if (err != null) {
-                return callback(
-                  OError.tag(err, 'error getting server id', { projectId })
-                )
-              }
-              const outputFiles = ClsiManager._parseOutputFiles(
-                projectId,
-                response && response.compile && response.compile.outputFiles
-              )
-              const compile = (response && response.compile) || {}
-              const status = compile.status
-              const stats = compile.stats
-              const timings = compile.timings
-              const validationProblems = undefined
-              callback(
-                null,
-                status,
-                outputFiles,
-                clsiServerId,
-                validationProblems,
-                stats,
-                timings
-              )
-            })
+            const outputFiles = ClsiManager._parseOutputFiles(
+              projectId,
+              response && response.compile && response.compile.outputFiles
+            )
+            const compile = (response && response.compile) || {}
+            const status = compile.status
+            const stats = compile.stats
+            const timings = compile.timings
+            const validationProblems = undefined
+            callback(
+              null,
+              status,
+              outputFiles,
+              clsiServerId,
+              validationProblems,
+              stats,
+              timings
+            )
           }
         )
       }
@@ -299,43 +292,59 @@ const ClsiManager = {
       {
         currentBackend(cb) {
           const startTime = new Date()
-          ClsiCookieManager.getCookieJar(projectId, (err, jar) => {
-            if (err != null) {
-              return callback(
-                OError.tag(err, 'error getting cookie jar for CLSI request', {
-                  projectId,
-                })
-              )
-            }
-            opts.jar = jar
-            const timer = new Metrics.Timer('compile.currentBackend')
-            request(opts, (err, response, body) => {
+          ClsiCookieManager.getCookieJar(
+            projectId,
+            (err, jar, clsiServerId) => {
               if (err != null) {
                 return callback(
-                  OError.tag(err, 'error making request to CLSI', { projectId })
+                  OError.tag(err, 'error getting cookie jar for CLSI request', {
+                    projectId,
+                  })
                 )
               }
-              timer.done()
-              Metrics.inc(
-                `compile.currentBackend.response.${response.statusCode}`
-              )
-              ClsiCookieManager.setServerId(projectId, response, err => {
+              opts.jar = jar
+              const timer = new Metrics.Timer('compile.currentBackend')
+              request(opts, (err, response, body) => {
                 if (err != null) {
-                  callback(
-                    OError.tag(err, 'error setting server id', { projectId })
+                  return callback(
+                    OError.tag(err, 'error making request to CLSI', {
+                      projectId,
+                    })
                   )
-                } else {
-                  // return as soon as the standard compile has returned
-                  callback(null, response, body)
                 }
-                cb(err, {
+                timer.done()
+                Metrics.inc(
+                  `compile.currentBackend.response.${response.statusCode}`
+                )
+                ClsiCookieManager.setServerId(
+                  projectId,
                   response,
-                  body,
-                  finishTime: new Date() - startTime,
-                })
+                  (err, newClsiServerId) => {
+                    if (err != null) {
+                      callback(
+                        OError.tag(err, 'error setting server id', {
+                          projectId,
+                        })
+                      )
+                    } else {
+                      // return as soon as the standard compile has returned
+                      callback(
+                        null,
+                        response,
+                        body,
+                        newClsiServerId || clsiServerId
+                      )
+                    }
+                    cb(err, {
+                      response,
+                      body,
+                      finishTime: new Date() - startTime,
+                    })
+                  }
+                )
               })
-            })
-          })
+            }
+          )
         },
         newBackend(cb) {
           const startTime = new Date()
@@ -460,40 +469,47 @@ const ClsiManager = {
       json: req,
       method: 'POST',
     }
-    ClsiManager._makeRequest(projectId, opts, (err, response, body) => {
-      if (err != null) {
-        return callback(
-          new OError('failed to make request to CLSI', {
-            projectId,
-            userId,
-            compileOptions: req.compile.options,
-            rootResourcePath: req.compile.rootResourcePath,
-          })
-        )
+    ClsiManager._makeRequest(
+      projectId,
+      opts,
+      (err, response, body, clsiServerId) => {
+        if (err != null) {
+          return callback(
+            new OError('failed to make request to CLSI', {
+              projectId,
+              userId,
+              compileOptions: req.compile.options,
+              rootResourcePath: req.compile.rootResourcePath,
+            })
+          )
+        }
+        if (response.statusCode >= 200 && response.statusCode < 300) {
+          callback(null, body, clsiServerId)
+        } else if (response.statusCode === 413) {
+          callback(null, { compile: { status: 'project-too-large' } })
+        } else if (response.statusCode === 409) {
+          callback(null, { compile: { status: 'conflict' } })
+        } else if (response.statusCode === 423) {
+          callback(null, { compile: { status: 'compile-in-progress' } })
+        } else if (response.statusCode === 503) {
+          callback(null, { compile: { status: 'unavailable' } })
+        } else {
+          callback(
+            new OError(
+              `CLSI returned non-success code: ${response.statusCode}`,
+              {
+                projectId,
+                userId,
+                compileOptions: req.compile.options,
+                rootResourcePath: req.compile.rootResourcePath,
+                clsiResponse: body,
+                statusCode: response.statusCode,
+              }
+            )
+          )
+        }
       }
-      if (response.statusCode >= 200 && response.statusCode < 300) {
-        callback(null, body)
-      } else if (response.statusCode === 413) {
-        callback(null, { compile: { status: 'project-too-large' } })
-      } else if (response.statusCode === 409) {
-        callback(null, { compile: { status: 'conflict' } })
-      } else if (response.statusCode === 423) {
-        callback(null, { compile: { status: 'compile-in-progress' } })
-      } else if (response.statusCode === 503) {
-        callback(null, { compile: { status: 'unavailable' } })
-      } else {
-        callback(
-          new OError(`CLSI returned non-success code: ${response.statusCode}`, {
-            projectId,
-            userId,
-            compileOptions: req.compile.options,
-            rootResourcePath: req.compile.rootResourcePath,
-            clsiResponse: body,
-            statusCode: response.statusCode,
-          })
-        )
-      }
-    })
+    )
   },
 
   _parseOutputFiles(projectId, rawOutputFiles = []) {

services/web/test/unit/src/Compile/ClsiManagerTests.js

@@ -148,30 +148,34 @@ describe('ClsiManager', function () {
 
     describe('with ranges on the pdf and stats/timings details', function () {
       beforeEach(function () {
-        this.ClsiManager._postToClsi = sinon.stub().yields(null, {
-          compile: {
-            status: 'success',
-            stats: { fooStat: 1 },
-            timings: { barTiming: 2 },
-            outputFiles: [
-              {
-                url: `${this.settings.apis.clsi.url}/project/${this.project_id}/user/${this.user_id}/build/1234/output/output.pdf`,
-                path: 'output.pdf',
-                type: 'pdf',
-                build: 1234,
-                contentId: '123-321',
-                ranges: [{ start: 1, end: 42, hash: 'foo' }],
-                size: 42,
-              },
-              {
-                url: `${this.settings.apis.clsi.url}/project/${this.project_id}/user/${this.user_id}/build/1234/output/output.log`,
-                path: 'output.log',
-                type: 'log',
-                build: 1234,
-              },
-            ],
+        this.ClsiManager._postToClsi = sinon.stub().yields(
+          null,
+          {
+            compile: {
+              status: 'success',
+              stats: { fooStat: 1 },
+              timings: { barTiming: 2 },
+              outputFiles: [
+                {
+                  url: `${this.settings.apis.clsi.url}/project/${this.project_id}/user/${this.user_id}/build/1234/output/output.pdf`,
+                  path: 'output.pdf',
+                  type: 'pdf',
+                  build: 1234,
+                  contentId: '123-321',
+                  ranges: [{ start: 1, end: 42, hash: 'foo' }],
+                  size: 42,
+                },
+                {
+                  url: `${this.settings.apis.clsi.url}/project/${this.project_id}/user/${this.user_id}/build/1234/output/output.log`,
+                  path: 'output.log',
+                  type: 'log',
+                  build: 1234,
+                },
+              ],
+            },
           },
-        })
+          'clsi-server-id-43'
+        )
         this.ClsiCookieManager._getServerId.yields(null, 'clsi-server-id-42')
         this.ClsiManager.sendRequest(
           this.project_id,
@@ -208,7 +212,7 @@ describe('ClsiManager', function () {
           null,
           'success',
           outputFiles,
-          'clsi-server-id-42',
+          'clsi-server-id-43',
           validationError,
           { fooStat: 1 },
           { barTiming: 2 }