From a85b2b34f5ebe429dddd1f84a14a9d1777ea1435 Mon Sep 17 00:00:00 2001 From: Jakob Ackermann Date: Wed, 3 Sep 2025 15:22:37 +0200 Subject: [PATCH] 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 --- .../app/src/Features/Compile/ClsiManager.js | 51 +++++++++---- services/web/app/src/router.mjs | 34 +++++---- .../test/unit/src/Compile/ClsiManagerTests.js | 73 +++++++++++++++++++ 3 files changed, 131 insertions(+), 27 deletions(-) diff --git a/services/web/app/src/Features/Compile/ClsiManager.js b/services/web/app/src/Features/Compile/ClsiManager.js index 4798c24331..2bdf0091a9 100644 --- a/services/web/app/src/Features/Compile/ClsiManager.js +++ b/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) { diff --git a/services/web/app/src/router.mjs b/services/web/app/src/router.mjs index 9c747cbe2d..4e33532f7e 100644 --- a/services/web/app/src/router.mjs +++ b/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) } } ) diff --git a/services/web/test/unit/src/Compile/ClsiManagerTests.js b/services/web/test/unit/src/Compile/ClsiManagerTests.js index d57e8b2f13..0b4a19adb9 100644 --- a/services/web/test/unit/src/Compile/ClsiManagerTests.js +++ b/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` + ) + ) + }) + }) }) })