Skip to content

Commit 8afc02a

Browse files
committed
fix(uploads): address PR review — abort propagation, orphan cleanup, error handling
- Throw immediately on AbortError in KB retry loop (no useless 14s backoff) - Cleanup S3/Blob object on quota or size-cap rejection in registerUploadedWorkspaceFile - Enforce MAX_WORKSPACE_FILE_SIZE at registration (defense vs presigned PUT lying about size) - Handle non-OK / non-JSON responses in workspace-files upload paths
1 parent 0128897 commit 8afc02a

3 files changed

Lines changed: 33 additions & 16 deletions

File tree

apps/sim/app/workspace/[workspaceId]/knowledge/hooks/use-knowledge-upload.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,7 @@ export function useKnowledgeUpload(options: UseKnowledgeUploadOptions = {}) {
259259
return buildUploadedFile(file, toAbsoluteUrl(filePath))
260260
}
261261

262-
if (attempt >= MULTIPART_MAX_RETRIES || (!isNetworkError(error) && !isAbortError(error))) {
262+
if (isAbortError(error) || !isNetworkError(error) || attempt >= MULTIPART_MAX_RETRIES) {
263263
throw error
264264
}
265265

apps/sim/hooks/queries/workspace-files.ts

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -223,9 +223,20 @@ async function uploadViaApiFallback(
223223
signal,
224224
})
225225

226-
const data = await response.json()
227-
if (!data.success) {
228-
throw new Error(data.error || 'Upload failed')
226+
return parseUploadResponse(response, 'Upload failed')
227+
}
228+
229+
async function parseUploadResponse(
230+
response: Response,
231+
fallbackMessage: string
232+
): Promise<UploadFileResponse> {
233+
let data: { success?: boolean; error?: string; file?: WorkspaceFileRecord } | null = null
234+
try {
235+
data = await response.json()
236+
} catch {}
237+
238+
if (!response.ok || !data?.success) {
239+
throw new Error(data?.error || `${fallbackMessage} (${response.status})`)
229240
}
230241
return data as UploadFileResponse
231242
}
@@ -265,11 +276,7 @@ async function uploadWorkspaceFile(
265276
signal,
266277
})
267278

268-
const data = await registerResponse.json()
269-
if (!data.success) {
270-
throw new Error(data.error || 'Failed to register file')
271-
}
272-
return data as UploadFileResponse
279+
return parseUploadResponse(registerResponse, 'Failed to register file')
273280
}
274281

275282
export function useUploadWorkspaceFile() {

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

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import {
2525
uploadFile,
2626
} from '@/lib/uploads/core/storage-service'
2727
import { getFileMetadataByKey, insertFileMetadata } from '@/lib/uploads/server/metadata'
28+
import { MAX_WORKSPACE_FILE_SIZE } from '@/lib/uploads/shared/types'
2829
import { getWorkspaceWithOwner } from '@/lib/workspaces/permissions/utils'
2930
import { isUuid, sanitizeFileName } from '@/executor/constants'
3031
import type { UserFile } from '@/executor/types'
@@ -301,8 +302,23 @@ export async function registerUploadedWorkspaceFile(params: {
301302
verifiedSize = head.size ?? size
302303
}
303304

305+
const cleanupOrphan = async (reason: string) => {
306+
if (!hasCloudStorage()) return
307+
try {
308+
await deleteFile({ key, context: 'workspace' })
309+
} catch (deleteError) {
310+
logger.error(`Failed to clean up orphaned object after ${reason}`, deleteError)
311+
}
312+
}
313+
314+
if (verifiedSize > MAX_WORKSPACE_FILE_SIZE) {
315+
await cleanupOrphan('size-cap rejection')
316+
throw new Error(`File size exceeds maximum of ${MAX_WORKSPACE_FILE_SIZE} bytes`)
317+
}
318+
304319
const quotaCheck = await checkStorageQuota(userId, verifiedSize)
305320
if (!quotaCheck.allowed) {
321+
await cleanupOrphan('quota rejection')
306322
throw new Error(quotaCheck.error || 'Storage limit exceeded')
307323
}
308324

@@ -330,13 +346,7 @@ export async function registerUploadedWorkspaceFile(params: {
330346
'Failed to insert metadata after direct upload; cleaning up storage object',
331347
insertError
332348
)
333-
if (hasCloudStorage()) {
334-
try {
335-
await deleteFile({ key, context: 'workspace' })
336-
} catch (deleteError) {
337-
logger.error('Failed to clean up orphaned storage object', deleteError)
338-
}
339-
}
349+
await cleanupOrphan('metadata insert failure')
340350
throw insertError
341351
}
342352
}

0 commit comments

Comments
 (0)