Browse Source

Move logging into ArchiveManager

GitOrigin-RevId: 71a28a0215c5f1a6975c9e2fcdcd476513df1cbc
andrew rumble 2 years ago
parent
commit
1409e32010

+ 0 - 6
services/clsi/app/js/OutputController.js

@@ -1,4 +1,3 @@
-const logger = require('@overleaf/logger')
 const OutputFileArchiveManager = require('./OutputFileArchiveManager')
 const OutputFileArchiveManager = require('./OutputFileArchiveManager')
 const { expressify } = require('@overleaf/promise-utils')
 const { expressify } = require('@overleaf/promise-utils')
 
 
@@ -8,7 +7,6 @@ async function createOutputZip(req, res) {
     user_id: userId,
     user_id: userId,
     build_id: buildId,
     build_id: buildId,
   } = req.params
   } = req.params
-  logger.debug({ projectId, userId, buildId }, 'Will create zip file')
 
 
   const archive = await OutputFileArchiveManager.archiveFilesForBuild(
   const archive = await OutputFileArchiveManager.archiveFilesForBuild(
     projectId,
     projectId,
@@ -16,10 +14,6 @@ async function createOutputZip(req, res) {
     buildId
     buildId
   )
   )
 
 
-  archive.on('error', err => {
-    logger.warn({ err }, 'error emitted when creating output files archive')
-  })
-
   res.attachment('output.zip')
   res.attachment('output.zip')
   res.setHeader('X-Content-Type-Options', 'nosniff')
   res.setHeader('X-Content-Type-Options', 'nosniff')
   archive.pipe(res)
   archive.pipe(res)

+ 5 - 0
services/clsi/app/js/OutputFileArchiveManager.js

@@ -5,6 +5,7 @@ const Settings = require('@overleaf/settings')
 const { open } = require('node:fs/promises')
 const { open } = require('node:fs/promises')
 const path = require('path')
 const path = require('path')
 const { NotFoundError } = require('./Errors')
 const { NotFoundError } = require('./Errors')
+const logger = require('@overleaf/logger')
 
 
 // NOTE: Updating this list requires a corresponding change in
 // NOTE: Updating this list requires a corresponding change in
 // * services/web/frontend/js/features/pdf-preview/util/file-list.js
 // * services/web/frontend/js/features/pdf-preview/util/file-list.js
@@ -22,10 +23,14 @@ function getContentDir(projectId, userId) {
 
 
 module.exports = {
 module.exports = {
   async archiveFilesForBuild(projectId, userId, build) {
   async archiveFilesForBuild(projectId, userId, build) {
+    logger.debug({ projectId, userId, build }, 'Will create zip file')
     const validFiles = await this._getAllOutputFiles(projectId, userId, build)
     const validFiles = await this._getAllOutputFiles(projectId, userId, build)
 
 
     const archive = archiver('zip')
     const archive = archiver('zip')
 
 
+    archive.on('error', err => {
+      logger.warn({ err }, 'error emitted when creating output files archive')
+    })
     const missingFiles = []
     const missingFiles = []
 
 
     for (const file of validFiles) {
     for (const file of validFiles) {

+ 5 - 0
services/clsi/test/unit/js/OutputFileArchiveManagerTests.js

@@ -32,6 +32,7 @@ describe('OutputFileArchiveManager', function () {
     this.archive = {
     this.archive = {
       append: sinon.stub(),
       append: sinon.stub(),
       finalize: sinon.stub(),
       finalize: sinon.stub(),
+      on: sinon.stub(),
     }
     }
 
 
     this.archiver = sinon.stub().returns(this.archive)
     this.archiver = sinon.stub().returns(this.archive)
@@ -80,6 +81,10 @@ describe('OutputFileArchiveManager', function () {
       sinon.assert.calledWith(this.archiver, 'zip')
       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 () {
     it('adds all the output files to the archive', function () {
       expect(this.archive.append.callCount).to.equal(4)
       expect(this.archive.append.callCount).to.equal(4)
       sinon.assert.calledWith(
       sinon.assert.calledWith(