Merge pull request #28233 from overleaf/jpa-clsi-lb-clear

[web] clear clsi server id/output files in both clsi-lb backends

GitOrigin-RevId: b73ccc2017800d9abbb8f571efeb34f51c9f96c1

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

@@ -34,6 +34,17 @@ const CLSI_COOKIES_ENABLED = (Settings.clsiCookie?.key ?? '') !== ''
 // The timeout in services/clsi/app.js is 10 minutes, so we'll be on the safe side with 12 minutes
 const COMPILE_REQUEST_TIMEOUT_MS = 12 * 60 * 1000
 
+async function clearClsiServerId(projectId, userId) {
+  const jobs = [ClsiCookieManager.promises.clearServerId(projectId, userId)]
+  if (Settings.apis.clsi_new?.url) {
+    // Mirror resetting the clsiserverid in both backends.
+    jobs.push(
+      NewBackendCloudClsiCookieManager.promises.clearServerId(projectId, userId)
+    )
+  }
+  await Promise.all(jobs)
+}
+
 function collectMetricsOnBlgFiles(outputFiles) {
   let topLevel = 0
   let nested = 0
@@ -160,14 +171,15 @@ async function deleteAuxFiles(projectId, userId, options, clsiserverid) {
     try {
       await DocumentUpdaterHandler.promises.clearProjectState(projectId)
     } finally {
-      await ClsiCookieManager.promises.clearServerId(projectId, userId)
+      // always clear the clsi server id, even if prior actions failed
+      await clearClsiServerId(projectId, userId)
     }
   }
 }
 
-async function _sendBuiltRequest(projectId, userId, req, options, callback) {
+async function _sendBuiltRequest(projectId, userId, req, options) {
   if (options.forceNewClsiServer) {
-    await ClsiCookieManager.promises.clearServerId(projectId, userId)
+    await clearClsiServerId(projectId, userId)
   }
   const validationProblems =
     await ClsiFormatChecker.promises.checkRecoursesForProblems(
@@ -221,13 +233,12 @@ async function _makeRequestWithClsiServerId(
 ) {
   if (clsiserverid) {
     // ignore cookies and newBackend, go straight to the clsi node
-    url.searchParams.set('compileGroup', compileGroup)
-    url.searchParams.set('compileBackendClass', compileBackendClass)
-    url.searchParams.set('clsiserverid', clsiserverid)
+    const urlWithId = new URL(url)
+    urlWithId.searchParams.set('clsiserverid', clsiserverid)
 
     let body
     try {
-      body = await fetchString(url, opts)
+      body = await fetchString(urlWithId, opts)
     } catch (err) {
       throw OError.tag(err, 'error making request to CLSI', {
         userId,
@@ -242,6 +253,17 @@ async function _makeRequestWithClsiServerId(
       // some responses are empty. Ignore JSON parsing errors.
     }
 
+    _makeNewBackendRequest(
+      projectId,
+      userId,
+      compileGroup,
+      compileBackendClass,
+      url,
+      opts
+    ).catch(err => {
+      logger.warn({ err }, 'Error making request to new CLSI backend')
+    })
+
     return { body: json }
   } else {
     return await _makeRequest(
@@ -372,9 +394,9 @@ async function _makeNewBackendRequest(
   if (Settings.apis.clsi_new?.url == null) {
     return null
   }
-  url = url
-    .toString()
-    .replace(Settings.apis.clsi.url, Settings.apis.clsi_new.url)
+  url = new URL(
+    url.toString().replace(Settings.apis.clsi.url, Settings.apis.clsi_new.url)
+  )
 
   const clsiServerId =
     await NewBackendCloudClsiCookieManager.promises.getServerId(
@@ -383,9 +405,12 @@ async function _makeNewBackendRequest(
       compileGroup,
       compileBackendClass
     )
-  opts.headers = {
-    Accept: 'application/json',
-    'Content-Type': 'application/json',
+  opts = {
+    ...opts,
+    headers: {
+      Accept: 'application/json',
+      'Content-Type': 'application/json',
+    },
   }
 
   if (CLSI_COOKIES_ENABLED) {

services/web/app/src/router.mjs

@@ -26,7 +26,6 @@ import TutorialController from './Features/Tutorial/TutorialController.mjs'
 import DocumentController from './Features/Documents/DocumentController.mjs'
 import CompileManager from './Features/Compile/CompileManager.js'
 import CompileController from './Features/Compile/CompileController.js'
-import ClsiCookieManagerFactory from './Features/Compile/ClsiCookieManager.js'
 import HealthCheckController from './Features/HealthCheck/HealthCheckController.mjs'
 import ProjectDownloadsController from './Features/Downloads/ProjectDownloadsController.mjs'
 import FileStoreController from './Features/FileStore/FileStoreController.mjs'
@@ -69,9 +68,6 @@ import SocketDiagnostics from './Features/SocketDiagnostics/SocketDiagnostics.mj
 import ClsiCacheController from './Features/Compile/ClsiCacheController.js'
 import AsyncLocalStorage from './infrastructure/AsyncLocalStorage.js'
 
-const ClsiCookieManager = ClsiCookieManagerFactory(
-  Settings.apis.clsi != null ? Settings.apis.clsi.backendGroupName : undefined
-)
 const { renderUnsupportedBrowserPage, unsupportedBrowserMiddleware } =
   UnsupportedBrowserMiddleware
 
@@ -1184,33 +1180,43 @@ async function initialize(webRouter, privateApiRouter, publicApiRouter) {
       const projectId = req.params.Project_id
       // use a valid user id for testing
       const testUserId = '123456789012345678901234'
-      const sendRes = _.once(function (statusCode, message) {
+      const sendRes = _.once(function (statusCode, message, clsiServerId) {
         res.status(statusCode)
         plainTextResponse(res, message)
-        ClsiCookieManager.promises
-          .clearServerId(projectId, testUserId)
+        // Force every compile to a new server and do not leave cruft behind.
+        CompileManager.promises
+          .deleteAuxFiles(projectId, testUserId, clsiServerId)
           .catch(() => {})
-      }) // force every compile to a new server
-      // set a timeout
+      })
       let handler = setTimeout(function () {
+        CompileManager.promises
+          .stopCompile(projectId, testUserId)
+          .catch(() => {})
         sendRes(500, 'Compiler timed out')
         handler = null
       }, 10000)
-      // run the compile
       CompileManager.compile(
         projectId,
         testUserId,
         {},
-        function (error, status) {
+        function (error, status, _outputFiles, clsiServerId) {
           if (handler) {
             clearTimeout(handler)
           }
           if (error) {
-            sendRes(500, `Compiler returned error ${error.message}`)
+            sendRes(
+              500,
+              `Compiler returned error ${error.message}`,
+              clsiServerId
+            )
           } else if (status === 'success') {
-            sendRes(200, 'Compiler returned in less than 10 seconds')
+            sendRes(
+              200,
+              'Compiler returned in less than 10 seconds',
+              clsiServerId
+            )
           } else {
-            sendRes(500, `Compiler returned failure ${status}`)
+            sendRes(500, `Compiler returned failure ${status}`, clsiServerId)
           }
         }
       )

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

@@ -970,6 +970,47 @@ describe('ClsiManager', function () {
           .called
       })
     })
+
+    describe('when a new backend is configured', function () {
+      beforeEach(async function () {
+        this.Settings.apis.clsi_new = { url: 'https://compiles.somewhere.test' }
+        await this.ClsiManager.promises.deleteAuxFiles(
+          this.project._id,
+          this.user_id,
+          { compileBackendClass: 'n2d', compileGroup: 'standard' },
+          'node-1'
+        )
+        // wait for the background task to finish
+        await setTimeout(0)
+      })
+
+      it('should forward delete request', function () {
+        expect(this.FetchUtils.fetchString).to.have.been.calledWith(
+          sinon.match(
+            url =>
+              url.host === CLSI_HOST &&
+              url.pathname ===
+                `/project/${this.project._id}/user/${this.user_id}` &&
+              url.searchParams.get('compileBackendClass') === 'n2d' &&
+              url.searchParams.get('compileGroup') === 'standard' &&
+              url.searchParams.get('clsiserverid') === 'node-1'
+          ),
+          { method: 'DELETE' }
+        )
+        expect(this.FetchUtils.fetchStringWithResponse).to.have.been.calledWith(
+          sinon.match(
+            url =>
+              url.host === 'compiles.somewhere.test' &&
+              url.pathname ===
+                `/project/${this.project._id}/user/${this.user_id}` &&
+              url.searchParams.get('compileBackendClass') === 'n2d' &&
+              url.searchParams.get('compileGroup') === 'standard' &&
+              !url.searchParams.has('clsiserverid')
+          ),
+          sinon.match({ method: 'DELETE' })
+        )
+      })
+    })
   })
 
   describe('wordCount', function () {
@@ -1032,6 +1073,38 @@ describe('ClsiManager', function () {
           .called
       })
     })
+
+    describe('when a new backend is configured', function () {
+      beforeEach(async function () {
+        this.Settings.apis.clsi_new = { url: 'https://compiles.somewhere.test' }
+        await this.ClsiManager.promises.wordCount(
+          this.project._id,
+          this.user_id,
+          false,
+          { compileBackendClass: 'n2d', compileGroup: 'standard' },
+          'node-1'
+        )
+        // wait for the background task to finish
+        await setTimeout(0)
+      })
+
+      it('should forward wordcount request', function () {
+        expect(this.FetchUtils.fetchString).to.have.been.calledWith(
+          sinon.match(
+            url =>
+              url.toString() ===
+              `http://clsi.example.com/project/${this.project._id}/user/${this.user_id}/wordcount?compileBackendClass=n2d&compileGroup=standard&file=main.tex&image=mock-image-name&clsiserverid=node-1`
+          )
+        )
+        expect(this.FetchUtils.fetchStringWithResponse).to.have.been.calledWith(
+          sinon.match(
+            url =>
+              url.toString() ===
+              `${this.Settings.apis.clsi_new.url}/project/${this.project._id}/user/${this.user_id}/wordcount?compileBackendClass=n2d&compileGroup=standard&file=main.tex&image=mock-image-name`
+          )
+        )
+      })
+    })
   })
 })