diff --git a/services/web/app/src/Features/History/RestoreManager.mjs b/services/web/app/src/Features/History/RestoreManager.mjs index dfa2118453..6bbf1eba52 100644 --- a/services/web/app/src/Features/History/RestoreManager.mjs +++ b/services/web/app/src/Features/History/RestoreManager.mjs @@ -1,5 +1,6 @@ import Settings from '@overleaf/settings' import Path from 'node:path' +import fs from 'node:fs' import FileWriter from '../../infrastructure/FileWriter.mjs' import Metrics from '../../infrastructure/Metrics.mjs' import FileSystemImportManager from '../Uploads/FileSystemImportManager.mjs' @@ -34,30 +35,34 @@ const RestoreManager = { version, pathname ) - const basename = Path.basename(pathname) - let dirname = Path.dirname(pathname) - if (dirname === '.') { - // no directory - dirname = '' - } - const parentFolderId = await RestoreManager._findOrCreateFolder( - projectId, - dirname, - userId - ) - const addEntityWithName = async name => - await FileSystemImportManager.promises.addEntity( - userId, + try { + const basename = Path.basename(pathname) + let dirname = Path.dirname(pathname) + if (dirname === '.') { + // no directory + dirname = '' + } + const parentFolderId = await RestoreManager._findOrCreateFolder( projectId, - parentFolderId, - name, - fsPath, - false + dirname, + userId ) - return await RestoreManager._addEntityWithUniqueName( - addEntityWithName, - basename - ) + const addEntityWithName = async name => + await FileSystemImportManager.promises.addEntity( + userId, + projectId, + parentFolderId, + name, + fsPath, + false + ) + return await RestoreManager._addEntityWithUniqueName( + addEntityWithName, + basename + ) + } finally { + await fs.promises.unlink(fsPath).catch(() => {}) + } }, async revertFile(userId, projectId, version, pathname, options = {}) { @@ -188,20 +193,24 @@ const RestoreManager = { historyId, snapshotFile ) - const newFile = await EditorController.promises.upsertFile( - projectId, - parentFolderId, - basename, - fsPath, - fileMetadata, - options.origin, - userId - ) + try { + const newFile = await EditorController.promises.upsertFile( + projectId, + parentFolderId, + basename, + fsPath, + fileMetadata, + options.origin, + userId + ) - endTimer({ type: 'file' }) - return { - _id: newFile._id, - type: 'file', + endTimer({ type: 'file' }) + return { + _id: newFile._id, + type: 'file', + } + } finally { + await fs.promises.unlink(fsPath).catch(() => {}) } } diff --git a/services/web/app/src/Features/LinkedFiles/LinkedFilesHandler.mjs b/services/web/app/src/Features/LinkedFiles/LinkedFilesHandler.mjs index f9c9bf81bb..aee33a6eb0 100644 --- a/services/web/app/src/Features/LinkedFiles/LinkedFilesHandler.mjs +++ b/services/web/app/src/Features/LinkedFiles/LinkedFilesHandler.mjs @@ -1,3 +1,4 @@ +import fs from 'node:fs' import FileWriter from '../../infrastructure/FileWriter.mjs' import EditorController from '../Editor/EditorController.mjs' import ProjectLocator from '../Project/ProjectLocator.mjs' @@ -63,15 +64,19 @@ const LinkedFilesHandler = { readStream ) - return await EditorController.promises.upsertFile( - projectId, - parentFolderId, - name, - fsPath, - linkedFileData, - 'upload', - userId - ) + try { + return await EditorController.promises.upsertFile( + projectId, + parentFolderId, + name, + fsPath, + linkedFileData, + 'upload', + userId + ) + } finally { + await fs.promises.unlink(fsPath).catch(() => {}) + } }, async importContent( @@ -87,15 +92,19 @@ const LinkedFilesHandler = { content ) - return await EditorController.promises.upsertFile( - projectId, - parentFolderId, - name, - fsPath, - linkedFileData, - 'upload', - userId - ) + try { + return await EditorController.promises.upsertFile( + projectId, + parentFolderId, + name, + fsPath, + linkedFileData, + 'upload', + userId + ) + } finally { + await fs.promises.unlink(fsPath).catch(() => {}) + } }, } diff --git a/services/web/test/unit/src/History/RestoreManager.test.mjs b/services/web/test/unit/src/History/RestoreManager.test.mjs index 450ccae874..d2cf5a3245 100644 --- a/services/web/test/unit/src/History/RestoreManager.test.mjs +++ b/services/web/test/unit/src/History/RestoreManager.test.mjs @@ -263,6 +263,7 @@ describe('RestoreManager', function () { afterEach(function () { tk.reset() + vi.resetModules() }) describe('restoreFileFromV2', function () { @@ -349,6 +350,35 @@ describe('RestoreManager', function () { .should.equal(true) }) }) + + describe('when addEntity throws an error', function () { + beforeEach(function (ctx) { + ctx.pathname = 'foo.tex' + ctx.FileSystemImportManager.promises.addEntity = sinon + .stub() + .rejects(new Error('Failed to add entity')) + ctx.fsUnlink = sinon.stub().resolves() + vi.doMock('node:fs', () => ({ + default: { promises: { unlink: ctx.fsUnlink } }, + })) + }) + + it('should clean up the temporary file and propagate the error', async function (ctx) { + let error + try { + await ctx.RestoreManager.promises.restoreFileFromV2( + ctx.user_id, + ctx.project_id, + ctx.version, + ctx.pathname + ) + } catch (err) { + error = err + } + expect(error).to.exist + expect(error.message).to.equal('Failed to add entity') + }) + }) }) describe('_findOrCreateFolder', function () {