diff --git a/services/web/app/src/Features/Uploads/ProjectUploadController.mjs b/services/web/app/src/Features/Uploads/ProjectUploadController.mjs index 27e237a225..bdc703aa15 100644 --- a/services/web/app/src/Features/Uploads/ProjectUploadController.mjs +++ b/services/web/app/src/Features/Uploads/ProjectUploadController.mjs @@ -84,7 +84,9 @@ async function uploadFile(req, res, next) { const userId = SessionManager.getLoggedInUserId(req.session) let { folder_id: folderId } = req.query if (name == null || name.length === 0 || name.length > 150) { - fs.unlink(path, function () {}) + await fsPromises.unlink(path).catch(unlinkErr => { + logger.warn({ err: unlinkErr, path }, 'error unlinking uploaded file') + }) return res.status(422).json({ success: false, error: 'invalid_filename', @@ -109,7 +111,9 @@ async function uploadFile(req, res, next) { folderId = lastFolder._id } } catch (error) { - fs.unlink(path, function () {}) + await fsPromises.unlink(path).catch(unlinkErr => { + logger.warn({ err: unlinkErr, path }, 'error unlinking uploaded file') + }) throw error } @@ -195,7 +199,12 @@ async function importDocx(req, res, next) { await ProjectOptionsHandler.promises.setCompiler(project._id, 'lualatex') res.json({ success: true, project_id: project._id }) } finally { - await fsPromises.unlink(archivePath).catch(() => {}) + await fsPromises.unlink(archivePath).catch(unlinkErr => { + logger.warn( + { err: unlinkErr, archivePath }, + 'error unlinking after docx conversion' + ) + }) } } catch (error) { logger.error({ error }, 'error importing docx file') @@ -213,7 +222,12 @@ async function importDocx(req, res, next) { error: req.i18n.translate('upload_failed'), }) } finally { - await fsPromises.unlink(path).catch(() => {}) + await fsPromises.unlink(path).catch(unlinkErr => { + logger.warn( + { err: unlinkErr, path }, + 'error unlinking uploaded file in importDocx' + ) + }) } } diff --git a/services/web/test/unit/src/Uploads/ProjectUploadController.test.mjs b/services/web/test/unit/src/Uploads/ProjectUploadController.test.mjs index caf266b086..b6c6c75580 100644 --- a/services/web/test/unit/src/Uploads/ProjectUploadController.test.mjs +++ b/services/web/test/unit/src/Uploads/ProjectUploadController.test.mjs @@ -112,7 +112,7 @@ describe('ProjectUploadController', function () { }) ) - vi.doMock('fs', () => ({ + vi.doMock('node:fs', () => ({ default: (ctx.fs = {}), })) @@ -141,6 +141,7 @@ describe('ProjectUploadController', function () { ctx.project = { _id: (ctx.project_id = 'project-id-123') } ctx.fs.unlink = sinon.stub() + ctx.fsPromises.unlink = sinon.stub().resolves() }) describe('successfully', function () { @@ -200,6 +201,10 @@ describe('ProjectUploadController', function () { JSON.stringify({ success: false, error: 'upload_failed' }) ) }) + + it('should remove the uploaded file', function (ctx) { + ctx.fs.unlink.calledWith(ctx.path).should.equal(true) + }) }) describe('when ProjectUploadManager.createProjectFromZipArchive reports the file as invalid', function () { @@ -224,6 +229,10 @@ describe('ProjectUploadController', function () { it("should return an 'unprocessable entity' status code", function (ctx) { expect(ctx.res.statusCode).to.equal(422) }) + + it('should remove the uploaded file', function (ctx) { + ctx.fs.unlink.calledWith(ctx.path).should.equal(true) + }) }) }) @@ -247,6 +256,7 @@ describe('ProjectUploadController', function () { ctx.req.params = { Project_id: ctx.project_id } ctx.req.query = { folder_id: ctx.folder_id } ctx.fs.unlink = sinon.stub() + ctx.fsPromises.unlink = sinon.stub().resolves() }) describe('successfully', function () { @@ -361,7 +371,7 @@ describe('ProjectUploadController', function () { }) it('should unlink the file', function (ctx) { - ctx.fs.unlink.should.have.been.calledWith(ctx.path) + ctx.fsPromises.unlink.should.have.been.calledWith(ctx.path) }) it('should call next with the error', function (ctx) { @@ -384,6 +394,10 @@ describe('ProjectUploadController', function () { }) ) }) + + it('should remove the uploaded file', function (ctx) { + ctx.fs.unlink.calledWith(ctx.path).should.equal(true) + }) }) describe('when FileSystemImportManager.addEntity returns a too many files error', function () { @@ -402,6 +416,10 @@ describe('ProjectUploadController', function () { }) ) }) + + it('should remove the uploaded file', function (ctx) { + ctx.fs.unlink.calledWith(ctx.path).should.equal(true) + }) }) describe('with an invalid filename', function () { @@ -420,7 +438,7 @@ describe('ProjectUploadController', function () { }) it('should remove the uploaded file', function (ctx) { - ctx.fs.unlink.calledWith(ctx.path).should.equal(true) + ctx.fsPromises.unlink.calledWith(ctx.path).should.equal(true) }) }) @@ -440,7 +458,7 @@ describe('ProjectUploadController', function () { }) it('should remove the uploaded file', function (ctx) { - ctx.fs.unlink.calledWith(ctx.path).should.equal(true) + ctx.fsPromises.unlink.calledWith(ctx.path).should.equal(true) }) }) })