Skip to content

Commit 1eecc18

Browse files
committed
fix(files): make staged cleanup safe across database failures
1 parent e0b359e commit 1eecc18

3 files changed

Lines changed: 301 additions & 119 deletions

File tree

‎apps/sim/lib/uploads/contexts/workspace/__integration__/file-versions.integration.ts‎

Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ vi.mock('@/lib/uploads/core/setup.server', () => ({
3131
},
3232
}))
3333

34+
import * as storageBilling from '@/lib/billing/storage'
35+
import { OrchestrationError } from '@/lib/core/orchestration/types'
3436
import { encryptSecret } from '@/lib/core/security/encryption'
3537
import {
3638
createKnowledgeAclFixtureIds,
@@ -49,6 +51,7 @@ import {
4951
uploadWorkspaceFile,
5052
} from '@/lib/uploads/contexts/workspace/workspace-file-manager'
5153
import type { WorkspaceFileSecretProvenance } from '@/lib/uploads/contexts/workspace/workspace-file-secret-provenance'
54+
import * as storageCleanup from '@/lib/uploads/contexts/workspace/workspace-file-storage-cleanup-outbox'
5255
import { WORKSPACE_FILE_STORAGE_CLEANUP_OUTBOX_EVENT } from '@/lib/uploads/contexts/workspace/workspace-file-storage-cleanup-outbox'
5356
import {
5457
getCurrentWorkspaceFileVersion,
@@ -57,6 +60,7 @@ import {
5760
queryWorkspaceFileVersions,
5861
releaseWorkspaceFileVersionsForPurgeInTx,
5962
} from '@/lib/uploads/contexts/workspace/workspace-file-versions'
63+
import * as storageService from '@/lib/uploads/core/storage-service'
6064
import { v2FileErrorPolicies } from '@/lib/workspace-files/api'
6165
import { presentWorkspaceFileText } from '@/lib/workspace-files/api/text-presenter'
6266
import {
@@ -187,6 +191,167 @@ describe('workspace file version history in PostgreSQL', () => {
187191
}
188192
}
189193

194+
it.each([
195+
{ phase: 'upload', deleteUnavailable: false },
196+
{ phase: 'content', deleteUnavailable: false },
197+
{ phase: 'storage', deleteUnavailable: false },
198+
{ phase: 'content', deleteUnavailable: true },
199+
] as const)(
200+
'preserves the original $phase failure and cleans uncommitted bytes when possible (delete unavailable=$deleteUnavailable)',
201+
async ({ phase, deleteUnavailable }) => {
202+
const fixture = await seedFile('original')
203+
const operationError = new OrchestrationError('conflict', 'Staged operation failed')
204+
const deleteFailure = deleteUnavailable
205+
? vi
206+
.spyOn(storageService, 'deleteFile')
207+
.mockRejectedValueOnce(new Error('Storage unavailable'))
208+
: null
209+
const enqueue = storageCleanup.enqueueWorkspaceFileStorageCleanups
210+
const enqueueFailure = vi
211+
.spyOn(storageCleanup, 'enqueueWorkspaceFileStorageCleanups')
212+
.mockImplementation((executor, ...args) =>
213+
executor === db
214+
? Promise.reject(new Error('Cleanup database unavailable'))
215+
: enqueue(executor, ...args)
216+
)
217+
const upload = storageService.uploadFile
218+
let stagedKey = ''
219+
const uploadFailure = vi
220+
.spyOn(storageService, 'uploadFile')
221+
.mockImplementation(async (args) => {
222+
const result = await upload(args)
223+
stagedKey = result.key
224+
if (phase === 'storage') throw operationError
225+
return result
226+
})
227+
const accountingFailure =
228+
phase === 'storage'
229+
? null
230+
: vi
231+
.spyOn(storageBilling, 'incrementStorageUsageForBillingContextInTx')
232+
.mockRejectedValueOnce(operationError)
233+
try {
234+
const operation =
235+
phase === 'content'
236+
? updateWorkspaceFileContent(
237+
fixture.workspaceId,
238+
fixture.fileId,
239+
fixture.aliceId,
240+
Buffer.from('replacement with more bytes'),
241+
undefined,
242+
{ version: { source: 'api', authorUserId: fixture.aliceId } }
243+
)
244+
: uploadWorkspaceFile(
245+
fixture.workspaceId,
246+
fixture.aliceId,
247+
Buffer.from('new upload'),
248+
'failed.txt',
249+
'text/plain',
250+
{ notifyWorkspaceChange: false }
251+
)
252+
await expect(operation).rejects.toBe(operationError)
253+
} finally {
254+
deleteFailure?.mockRestore()
255+
accountingFailure?.mockRestore()
256+
uploadFailure.mockRestore()
257+
enqueueFailure.mockRestore()
258+
}
259+
expect(stagedKey).not.toBe('')
260+
expect(await objectExists(stagedKey)).toBe(deleteUnavailable)
261+
const retained = await getWorkspaceFile(fixture.workspaceId, fixture.fileId)
262+
if (!retained) throw new Error('file missing')
263+
expect((await fetchWorkspaceFileBuffer(retained, { maxBytes: 1024 })).toString()).toBe(
264+
'original'
265+
)
266+
expect(
267+
await db
268+
.select({ id: workspaceFiles.id })
269+
.from(workspaceFiles)
270+
.where(eq(workspaceFiles.key, stagedKey))
271+
).toEqual([])
272+
}
273+
)
274+
275+
it.each([
276+
{ operation: 'upload', enqueueAvailable: true },
277+
{ operation: 'content', enqueueAvailable: true },
278+
{ operation: 'upload', enqueueAvailable: false },
279+
{ operation: 'content', enqueueAvailable: false },
280+
] as const)(
281+
'retains committed $operation bytes after acknowledgement loss (cleanup database available=$enqueueAvailable)',
282+
async ({ operation, enqueueAvailable }) => {
283+
const fixture = await seedFile('original')
284+
const transaction = db.transaction.bind(db)
285+
const lostAcknowledgement = vi
286+
.spyOn(db, 'transaction')
287+
.mockImplementationOnce(async (callback, config) => {
288+
await transaction(callback, config)
289+
throw new Error('Commit acknowledgement lost')
290+
})
291+
const enqueue = storageCleanup.enqueueWorkspaceFileStorageCleanups
292+
const enqueueFailure = vi
293+
.spyOn(storageCleanup, 'enqueueWorkspaceFileStorageCleanups')
294+
.mockImplementation((executor, ...args) =>
295+
!enqueueAvailable && executor === db
296+
? Promise.reject(new Error('Cleanup database unavailable'))
297+
: enqueue(executor, ...args)
298+
)
299+
const upload = storageService.uploadFile
300+
let stagedKey = ''
301+
const capture = vi.spyOn(storageService, 'uploadFile').mockImplementation(async (args) => {
302+
const result = await upload(args)
303+
stagedKey = result.key
304+
return result
305+
})
306+
try {
307+
const result =
308+
operation === 'content'
309+
? updateWorkspaceFileContent(
310+
fixture.workspaceId,
311+
fixture.fileId,
312+
fixture.aliceId,
313+
Buffer.from('committed content'),
314+
undefined,
315+
{ version: { source: 'api', authorUserId: fixture.aliceId } }
316+
)
317+
: uploadWorkspaceFile(
318+
fixture.workspaceId,
319+
fixture.aliceId,
320+
Buffer.from('committed content'),
321+
'committed.txt',
322+
'text/plain',
323+
{ notifyWorkspaceChange: false }
324+
)
325+
await expect(result).rejects.toThrow('Commit acknowledgement lost')
326+
} finally {
327+
capture.mockRestore()
328+
enqueueFailure.mockRestore()
329+
lostAcknowledgement.mockRestore()
330+
}
331+
const [committed] = await db
332+
.select({ id: workspaceFiles.id })
333+
.from(workspaceFiles)
334+
.where(eq(workspaceFiles.key, stagedKey))
335+
expect(committed?.id).toEqual(expect.any(String))
336+
const file = await getWorkspaceFile(fixture.workspaceId, committed.id)
337+
if (!file) throw new Error('committed file missing')
338+
expect((await fetchWorkspaceFileBuffer(file, { maxBytes: 1024 })).toString()).toBe(
339+
'committed content'
340+
)
341+
expect(
342+
await db
343+
.select({ id: outboxEvent.id })
344+
.from(outboxEvent)
345+
.where(
346+
and(
347+
eq(outboxEvent.eventType, WORKSPACE_FILE_STORAGE_CLEANUP_OUTBOX_EVENT),
348+
sql`${outboxEvent.payload}->>'key' = ${stagedKey}`
349+
)
350+
)
351+
).toEqual([])
352+
}
353+
)
354+
190355
it('lists a never-rewritten file as an implicit version 1 attributed to its uploader', async () => {
191356
const fixture = await seedFile('original')
192357
const file = await getWorkspaceFile(fixture.workspaceId, fixture.fileId)

0 commit comments

Comments
 (0)