Skip to content

Commit 92f841f

Browse files
committed
fix(uploads): handle register retries and name-collision races
Two bugs in registerUploadedWorkspaceFile: 1. Register retry could orphan storage. When a successful response was lost on the wire and the client retried, the quota check saw the bytes already counted, failed, and cleanupOrphan deleted the already-registered storage object — leaving the DB row pointing to nothing. Fix: check getFileMetadataByKey before quota guard and short-circuit on existing record. 2. Concurrent same-named uploads could lose data. allocateUniqueWorkspaceFileName is best-effort; two racing uploads can pass it and both attempt the same display name. The loser's insert hits 23505, the catch block called cleanupOrphan, and successfully-uploaded bytes were deleted. Fix: retry on 23505 with a fresh allocateUniqueWorkspaceFileName, matching the pattern in uploadWorkspaceFile. Throw FileConflictError after exhaustion.
1 parent e77f8d5 commit 92f841f

1 file changed

Lines changed: 56 additions & 29 deletions

File tree

apps/sim/lib/uploads/contexts/workspace/workspace-file-manager.ts

Lines changed: 56 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -322,48 +322,75 @@ export async function registerUploadedWorkspaceFile(params: {
322322
throw new Error(`File size exceeds maximum of ${MAX_WORKSPACE_FILE_SIZE} bytes`)
323323
}
324324

325-
const quotaCheck = await checkStorageQuota(userId, verifiedSize)
326-
if (!quotaCheck.allowed) {
327-
await cleanupOrphan('quota rejection')
328-
throw new Error(quotaCheck.error || 'Storage limit exceeded')
329-
}
325+
/**
326+
* Existence check runs before the quota guard so a retry after a
327+
* successful-but-network-dropped register does not (a) fail quota because
328+
* the bytes are now counted, or (b) invoke cleanupOrphan on an already
329+
* registered object.
330+
*/
331+
const existing = await getFileMetadataByKey(key, 'workspace')
330332

331-
let fileId = `wf_${generateShortId()}`
332-
let displayName: string
333+
const fileId = existing?.id ?? `wf_${generateShortId()}`
334+
let displayName = existing?.originalName ?? ''
333335
let created = false
334-
const existing = await getFileMetadataByKey(key, 'workspace')
335-
if (existing) {
336-
fileId = existing.id
337-
displayName = existing.originalName
338-
logger.info(`Using existing metadata record for direct upload: ${key}`)
339-
} else {
340-
created = true
341-
displayName = await allocateUniqueWorkspaceFileName(workspaceId, originalName)
342-
try {
343-
await insertFileMetadata({
344-
id: fileId,
345-
key,
346-
userId,
347-
workspaceId,
348-
context: 'workspace',
349-
originalName: displayName,
350-
contentType,
351-
size: verifiedSize,
352-
})
353-
} catch (insertError) {
336+
337+
if (!existing) {
338+
const quotaCheck = await checkStorageQuota(userId, verifiedSize)
339+
if (!quotaCheck.allowed) {
340+
await cleanupOrphan('quota rejection')
341+
throw new Error(quotaCheck.error || 'Storage limit exceeded')
342+
}
343+
344+
let lastInsertError: unknown
345+
for (let attempt = 0; attempt < MAX_UPLOAD_UNIQUE_RETRIES; attempt++) {
346+
displayName = await allocateUniqueWorkspaceFileName(workspaceId, originalName)
347+
try {
348+
await insertFileMetadata({
349+
id: fileId,
350+
key,
351+
userId,
352+
workspaceId,
353+
context: 'workspace',
354+
originalName: displayName,
355+
contentType,
356+
size: verifiedSize,
357+
})
358+
created = true
359+
lastInsertError = undefined
360+
break
361+
} catch (insertError) {
362+
lastInsertError = insertError
363+
if (getPostgresErrorCode(insertError) === '23505') {
364+
logger.warn(
365+
`Unique name conflict on register (attempt ${attempt + 1}/${MAX_UPLOAD_UNIQUE_RETRIES}), retrying with a new name`
366+
)
367+
continue
368+
}
369+
break
370+
}
371+
}
372+
373+
if (!created) {
354374
logger.error(
355375
'Failed to insert metadata after direct upload; cleaning up storage object',
356-
insertError
376+
lastInsertError
357377
)
358378
await cleanupOrphan('metadata insert failure')
359-
throw insertError
379+
if (getPostgresErrorCode(lastInsertError) === '23505') {
380+
throw new FileConflictError(originalName)
381+
}
382+
throw lastInsertError instanceof Error
383+
? lastInsertError
384+
: new Error('Failed to insert workspace file metadata')
360385
}
361386

362387
try {
363388
await incrementStorageUsage(userId, verifiedSize)
364389
} catch (storageError) {
365390
logger.error('Failed to update storage tracking:', storageError)
366391
}
392+
} else {
393+
logger.info(`Using existing metadata record for direct upload: ${key}`)
367394
}
368395

369396
const pathPrefix = getServePathPrefix()

0 commit comments

Comments
 (0)