Merge pull request #18474 from overleaf/ar-return-build-id-from-clsi-after-compile

[clsi] Return buildId after compiles

GitOrigin-RevId: 872048f4fea8f5a00b817e29bd26a444d179a45f

services/clsi/app/js/CompileController.js

@@ -30,7 +30,7 @@ function compile(req, res, next) {
           return next(error)
         }
         CompileManager.doCompileWithLock(request, (error, result) => {
-          let { outputFiles, stats, timings } = result || {}
+          let { buildId, outputFiles, stats, timings } = result || {}
           let code, status
           if (outputFiles == null) {
             outputFiles = []
@@ -98,6 +98,7 @@ function compile(req, res, next) {
 
           if (error) {
             outputFiles = error.outputFiles || []
+            buildId = error.buildId
           }
 
           timer.done()
@@ -107,6 +108,7 @@ function compile(req, res, next) {
               error: error?.message || error,
               stats,
               timings,
+              buildId,
               outputUrlPrefix: Settings.apis.clsi.outputUrlPrefix,
               outputFiles: outputFiles.map(file => ({
                 url:

services/clsi/app/js/CompileManager.js

@@ -208,7 +208,7 @@ async function doCompile(request) {
       Metrics.inc('compiles-timeout', 1, request.metricsOpts)
     }
 
-    const { outputFiles, allEntries } = await _saveOutputFiles({
+    const { outputFiles, allEntries, buildId } = await _saveOutputFiles({
       request,
       compileDir,
       resourceList,
@@ -216,7 +216,7 @@ async function doCompile(request) {
       timings,
     })
     error.outputFiles = outputFiles // return output files so user can check logs
-
+    error.buildId = buildId
     // Clear project if this compile was abruptly terminated
     if (error.terminated || error.timedout) {
       await clearProjectWithListing(
@@ -280,7 +280,7 @@ async function doCompile(request) {
   // Emit compile time.
   timings.compile = ts
 
-  const { outputFiles } = await _saveOutputFiles({
+  const { outputFiles, buildId } = await _saveOutputFiles({
     request,
     compileDir,
     resourceList,
@@ -296,7 +296,7 @@ async function doCompile(request) {
     emitPdfStats(stats, timings, request)
   }
 
-  return { outputFiles, stats, timings }
+  return { outputFiles, stats, timings, buildId }
 }
 
 async function _saveOutputFiles({
@@ -316,20 +316,24 @@ async function _saveOutputFiles({
   let { outputFiles, allEntries } =
     await OutputFileFinder.promises.findOutputFiles(resourceList, compileDir)
 
+  let buildId
+
   try {
-    outputFiles = await OutputCacheManager.promises.saveOutputFiles(
+    const saveResult = await OutputCacheManager.promises.saveOutputFiles(
       { request, stats, timings },
       outputFiles,
       compileDir,
       outputDir
     )
+    buildId = saveResult.buildId
+    outputFiles = saveResult.outputFiles
   } catch (err) {
     const { project_id: projectId, user_id: userId } = request
     logger.err({ projectId, userId, err }, 'failed to save output files')
   }
 
   timings.output = timer.done()
-  return { outputFiles, allEntries }
+  return { outputFiles, allEntries, buildId }
 }
 
 async function stopCompile(projectId, userId) {

services/clsi/app/js/OutputCacheManager.js

@@ -164,7 +164,7 @@ module.exports = OutputCacheManager = {
             outputDir,
             stats,
             (err, outputFiles) => {
-              if (err) return callback(err, outputFiles)
+              if (err) return callback(err, { outputFiles, buildId })
 
               const enablePdfCaching = request.enablePdfCaching
               const enablePdfCachingDark =
@@ -173,7 +173,7 @@ module.exports = OutputCacheManager = {
                 !Settings.enablePdfCaching ||
                 (!enablePdfCaching && !enablePdfCachingDark)
               ) {
-                return callback(null, outputFiles)
+                return callback(null, { outputFiles, buildId })
               }
 
               OutputCacheManager.saveStreamsInContentDir(
@@ -191,9 +191,9 @@ module.exports = OutputCacheManager = {
                       { err, outputDir, stats, timings },
                       'pdf caching failed'
                     )
-                    return callback(null, outputFiles)
+                    return callback(null, { outputFiles, buildId })
                   }
-                  callback(err, outputFiles)
+                  callback(err, { outputFiles, buildId })
                 }
               )
             }

services/clsi/test/unit/js/CompileControllerTests.js

@@ -50,6 +50,7 @@ function tryImageNameValidation(method, imageNameField) {
 
 describe('CompileController', function () {
   beforeEach(function () {
+    this.buildId = 'build-id-123'
     this.CompileController = SandboxedModule.require(modulePath, {
       requires: {
         './CompileManager': (this.CompileManager = {}),
@@ -118,6 +119,7 @@ describe('CompileController', function () {
           outputFiles: this.output_files,
           stats: this.stats,
           timings: this.timings,
+          buildId: this.buildId,
         })
         this.CompileController.compile(this.req, this.res)
       })
@@ -147,6 +149,7 @@ describe('CompileController', function () {
               error: null,
               stats: this.stats,
               timings: this.timings,
+              buildId: this.buildId,
               outputUrlPrefix: '/zone/b',
               outputFiles: this.output_files.map(file => ({
                 url: `${this.Settings.apis.clsi.url}/project/${this.project_id}/build/${file.build}/output/${file.path}`,
@@ -165,6 +168,7 @@ describe('CompileController', function () {
           outputFiles: this.output_files,
           stats: this.stats,
           timings: this.timings,
+          buildId: this.buildId,
         })
         this.CompileController.compile(this.req, this.res)
       })
@@ -178,6 +182,7 @@ describe('CompileController', function () {
               error: null,
               stats: this.stats,
               timings: this.timings,
+              buildId: this.buildId,
               outputUrlPrefix: '',
               outputFiles: this.output_files.map(file => ({
                 url: `${this.Settings.apis.clsi.url}/project/${this.project_id}/build/${file.build}/output/${file.path}`,
@@ -207,6 +212,7 @@ describe('CompileController', function () {
           outputFiles: this.output_files,
           stats: this.stats,
           timings: this.timings,
+          buildId: this.buildId,
         })
         this.CompileController.compile(this.req, this.res)
       })
@@ -221,6 +227,7 @@ describe('CompileController', function () {
               stats: this.stats,
               timings: this.timings,
               outputUrlPrefix: '/zone/b',
+              buildId: this.buildId,
               outputFiles: this.output_files.map(file => ({
                 url: `${this.Settings.apis.clsi.url}/project/${this.project_id}/build/${file.build}/output/${file.path}`,
                 ...file,
@@ -250,6 +257,7 @@ describe('CompileController', function () {
           outputFiles: this.output_files,
           stats: this.stats,
           timings: this.timings,
+          buildId: this.buildId,
         })
         this.CompileController.compile(this.req, this.res)
       })
@@ -262,6 +270,7 @@ describe('CompileController', function () {
               status: 'failure',
               error: null,
               stats: this.stats,
+              buildId: this.buildId,
               timings: this.timings,
               outputUrlPrefix: '/zone/b',
               outputFiles: this.output_files.map(file => ({
@@ -276,9 +285,11 @@ describe('CompileController', function () {
 
     describe('with an error', function () {
       beforeEach(function () {
+        const error = new Error((this.message = 'error message'))
+        error.buildId = this.buildId
         this.CompileManager.doCompileWithLock = sinon
           .stub()
-          .callsArgWith(1, new Error((this.message = 'error message')), null)
+          .callsArgWith(1, error, null)
         this.CompileController.compile(this.req, this.res)
       })
 
@@ -291,6 +302,7 @@ describe('CompileController', function () {
               error: this.message,
               outputUrlPrefix: '/zone/b',
               outputFiles: [],
+              buildId: this.buildId,
               // JSON.stringify will omit these
               stats: undefined,
               timings: undefined,
@@ -320,6 +332,7 @@ describe('CompileController', function () {
               outputUrlPrefix: '/zone/b',
               outputFiles: [],
               // JSON.stringify will omit these
+              buildId: undefined,
               stats: undefined,
               timings: undefined,
             },
@@ -346,6 +359,7 @@ describe('CompileController', function () {
               outputUrlPrefix: '/zone/b',
               outputFiles: [],
               // JSON.stringify will omit these
+              buildId: undefined,
               stats: undefined,
               timings: undefined,
             },

services/clsi/test/unit/js/CompileManagerTests.js

@@ -35,6 +35,7 @@ describe('CompileManager', function () {
         build: 1234,
       },
     ]
+    this.buildId = 'build-id-123'
     this.commandOutput = 'Dummy output'
     this.compileBaseDir = '/compile/dir'
     this.outputBaseDir = '/output/dir'
@@ -61,7 +62,9 @@ describe('CompileManager', function () {
     }
     this.OutputCacheManager = {
       promises: {
-        saveOutputFiles: sinon.stub().resolves(this.buildFiles),
+        saveOutputFiles: sinon
+          .stub()
+          .resolves({ outputFiles: this.buildFiles, buildId: this.buildId }),
       },
     }
     this.Settings = {