From 1409e320101501e8b7054ea35c17ced641cee609 Mon Sep 17 00:00:00 2001
From: andrew rumble <andrew.rumble@overleaf.com>
Date: Tue, 18 Jun 2024 09:17:15 +0100
Subject: [PATCH] Move logging into ArchiveManager

GitOrigin-RevId: 71a28a0215c5f1a6975c9e2fcdcd476513df1cbc
---
 services/clsi/app/js/OutputController.js                    | 6 ------
 services/clsi/app/js/OutputFileArchiveManager.js            | 5 +++++
 services/clsi/test/unit/js/OutputFileArchiveManagerTests.js | 5 +++++
 3 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/services/clsi/app/js/OutputController.js b/services/clsi/app/js/OutputController.js
index a1518bd97a..8ac6fd7a28 100644
--- a/services/clsi/app/js/OutputController.js
+++ b/services/clsi/app/js/OutputController.js
@@ -1,4 +1,3 @@
-const logger = require('@overleaf/logger')
 const OutputFileArchiveManager = require('./OutputFileArchiveManager')
 const { expressify } = require('@overleaf/promise-utils')
 
@@ -8,7 +7,6 @@ async function createOutputZip(req, res) {
     user_id: userId,
     build_id: buildId,
   } = req.params
-  logger.debug({ projectId, userId, buildId }, 'Will create zip file')
 
   const archive = await OutputFileArchiveManager.archiveFilesForBuild(
     projectId,
@@ -16,10 +14,6 @@ async function createOutputZip(req, res) {
     buildId
   )
 
-  archive.on('error', err => {
-    logger.warn({ err }, 'error emitted when creating output files archive')
-  })
-
   res.attachment('output.zip')
   res.setHeader('X-Content-Type-Options', 'nosniff')
   archive.pipe(res)
diff --git a/services/clsi/app/js/OutputFileArchiveManager.js b/services/clsi/app/js/OutputFileArchiveManager.js
index 45e9f33efa..98d6b8dddd 100644
--- a/services/clsi/app/js/OutputFileArchiveManager.js
+++ b/services/clsi/app/js/OutputFileArchiveManager.js
@@ -5,6 +5,7 @@ const Settings = require('@overleaf/settings')
 const { open } = require('node:fs/promises')
 const path = require('path')
 const { NotFoundError } = require('./Errors')
+const logger = require('@overleaf/logger')
 
 // NOTE: Updating this list requires a corresponding change in
 // * services/web/frontend/js/features/pdf-preview/util/file-list.js
@@ -22,10 +23,14 @@ function getContentDir(projectId, userId) {
 
 module.exports = {
   async archiveFilesForBuild(projectId, userId, build) {
+    logger.debug({ projectId, userId, build }, 'Will create zip file')
     const validFiles = await this._getAllOutputFiles(projectId, userId, build)
 
     const archive = archiver('zip')
 
+    archive.on('error', err => {
+      logger.warn({ err }, 'error emitted when creating output files archive')
+    })
     const missingFiles = []
 
     for (const file of validFiles) {
diff --git a/services/clsi/test/unit/js/OutputFileArchiveManagerTests.js b/services/clsi/test/unit/js/OutputFileArchiveManagerTests.js
index c2a7047a76..6eeb3637c6 100644
--- a/services/clsi/test/unit/js/OutputFileArchiveManagerTests.js
+++ b/services/clsi/test/unit/js/OutputFileArchiveManagerTests.js
@@ -32,6 +32,7 @@ describe('OutputFileArchiveManager', function () {
     this.archive = {
       append: sinon.stub(),
       finalize: sinon.stub(),
+      on: sinon.stub(),
     }
 
     this.archiver = sinon.stub().returns(this.archive)
@@ -80,6 +81,10 @@ describe('OutputFileArchiveManager', function () {
       sinon.assert.calledWith(this.archiver, 'zip')
     })
 
+    it('listens to errors from the archive', function () {
+      sinon.assert.calledWith(this.archive.on, 'error', sinon.match.func)
+    })
+
     it('adds all the output files to the archive', function () {
       expect(this.archive.append.callCount).to.equal(4)
       sinon.assert.calledWith(