Skip to content

Commit 3d8e489

Browse files
fix(files): classify folder and content failures instead of 500ing them
Bugbot round 1. The v2 routes map errorCode straight to a status, so every manager failure that arrived unclassified became a 500 for what is really a caller-fixable 400 or 404. - Folder manager throws OrchestrationError: missing target/folder -> not_found, reparent cycle / self-parent / restore-into-archived-workspace -> validation. - File manager does the same for the in-transaction 'File not found' paths that the earlier pass missed. - updateWorkspaceFileContent's outer catch re-wrapped everything in a bare Error, which stripped the class off StorageLimitExceededError and the new not_found alike. It now rethrows a classified failure untouched and attaches cause to the generic wrap, so asOrchestrationError can still walk the chain. - Every remaining perform* gained the asOrchestrationError branch. - renameWorkspaceFile returned the pre-update read, so the v2 PATCH reported a stale updatedAt; it now returns the timestamp it actually wrote. Docs: upload auto-suffixes a duplicate name rather than rejecting it, matching the in-app uploader. The description claimed 409 and was simply wrong.
1 parent 6bb9e7e commit 3d8e489

5 files changed

Lines changed: 283 additions & 27 deletions

File tree

apps/docs/openapi-v2-files-audit.json

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@
144144
"post": {
145145
"operationId": "uploadFile",
146146
"summary": "Upload File",
147-
"description": "Upload a file to a workspace as `multipart/form-data` with a single `file` field. The workspace — and the optional target `folderId` — are supplied as query parameters (not form fields) so authorization runs before the request body is buffered. Maximum file size is 100MB. A name already taken in the destination folder is rejected with `409`; the in-app uploader instead auto-suffixes, so the two surfaces differ here on purpose. Returns `201 Created`.\n\nPresigned upload is not part of the public API: it debits the storage quota only in a separate register step, so a caller that never registers would leave unaccounted bytes in storage. This buffered path debits inside the upload transaction.",
147+
"description": "Upload a file to a workspace as `multipart/form-data` with a single `file` field. The workspace — and the optional target `folderId` — are supplied as query parameters (not form fields) so authorization runs before the request body is buffered. Maximum file size is 100MB. A name already taken in the destination folder is **not** an error: the name is auto-suffixed (`data.csv` -> `data (1).csv`), matching the in-app uploader, so a `201` can come back with a `name` different from the one you sent — always read `name` from the response rather than assuming it. `409` is returned only if a unique name cannot be allocated after several attempts. Use `PATCH /api/v2/files/{fileId}` if you need a specific name to be exact-or-fail. Returns `201 Created`.\n\nPresigned upload is not part of the public API: it debits the storage quota only in a separate register step, so a caller that never registers would leave unaccounted bytes in storage. This buffered path debits inside the upload transaction.",
148148
"tags": ["Files"],
149149
"x-codeSamples": [
150150
{
@@ -247,7 +247,7 @@
247247
"$ref": "#/components/responses/Forbidden"
248248
},
249249
"409": {
250-
"description": "A file with the same name already exists in this workspace.",
250+
"description": "A unique filename could not be allocated in the destination folder after several attempts. An ordinary name collision is auto-suffixed instead, not rejected.",
251251
"content": {
252252
"application/json": {
253253
"schema": {
@@ -256,7 +256,7 @@
256256
"example": {
257257
"error": {
258258
"code": "CONFLICT",
259-
"message": "A file with this name already exists in the workspace"
259+
"message": "A file named \"data.csv\" already exists in this workspace"
260260
}
261261
}
262262
}

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

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { createLogger } from '@sim/logger'
44
import { getPostgresErrorCode } from '@sim/utils/errors'
55
import { generateId } from '@sim/utils/id'
66
import { and, asc, eq, inArray, isNull, min, type SQL, sql } from 'drizzle-orm'
7+
import { OrchestrationError } from '@/lib/core/orchestration/types'
78
import { deduplicateFolderName } from '@/lib/folders/naming'
89
import { collectDescendantFolderIds } from '@/lib/folders/subtree'
910
import { getWorkspaceWithOwner } from '@/lib/workspaces/permissions/utils'
@@ -346,7 +347,7 @@ export async function assertWorkspaceFileFolderTarget(
346347

347348
const folder = await getWorkspaceFileFolder(workspaceId, normalized)
348349
if (!folder) {
349-
throw new Error('Target folder not found')
350+
throw new OrchestrationError('not_found', 'Target folder not found')
350351
}
351352

352353
return normalized
@@ -380,7 +381,7 @@ export async function createWorkspaceFileFolder(params: {
380381
.limit(1)
381382

382383
if (!target) {
383-
throw new Error('Target folder not found')
384+
throw new OrchestrationError('not_found', 'Target folder not found')
384385
}
385386
}
386387

@@ -558,7 +559,7 @@ export async function updateWorkspaceFileFolder(params: {
558559
)
559560
.limit(1)
560561

561-
if (!existing) throw new Error('Folder not found')
562+
if (!existing) throw new OrchestrationError('not_found', 'Folder not found')
562563

563564
const updates: Partial<typeof folderTable.$inferInsert> = { updatedAt: new Date() }
564565
const finalName =
@@ -568,7 +569,8 @@ export async function updateWorkspaceFileFolder(params: {
568569
const finalParentId =
569570
params.parentId !== undefined ? normalizeParentId(params.parentId) : existing.parentId
570571

571-
if (finalParentId === params.folderId) throw new Error('Folder cannot be its own parent')
572+
if (finalParentId === params.folderId)
573+
throw new OrchestrationError('validation', 'Folder cannot be its own parent')
572574

573575
if (finalParentId) {
574576
const [target] = await tx
@@ -585,7 +587,7 @@ export async function updateWorkspaceFileFolder(params: {
585587
.limit(1)
586588

587589
if (!target) {
588-
throw new Error('Target folder not found')
590+
throw new OrchestrationError('not_found', 'Target folder not found')
589591
}
590592
}
591593

@@ -603,7 +605,10 @@ export async function updateWorkspaceFileFolder(params: {
603605

604606
const descendants = collectDescendantFolderIds(activeFolders, params.folderId)
605607
if (finalParentId && descendants.includes(finalParentId)) {
606-
throw new Error('Cannot move a folder into one of its descendants')
608+
throw new OrchestrationError(
609+
'validation',
610+
'Cannot move a folder into one of its descendants'
611+
)
607612
}
608613
}
609614

@@ -653,7 +658,7 @@ export async function updateWorkspaceFileFolder(params: {
653658
)
654659
.returning()
655660

656-
if (!updatedFolder) throw new Error('Folder not found')
661+
if (!updatedFolder) throw new OrchestrationError('not_found', 'Folder not found')
657662
return updatedFolder
658663
} catch (error) {
659664
if (getPostgresErrorCode(error) === '23505') {
@@ -717,12 +722,12 @@ export async function moveWorkspaceFileItems(params: {
717722
.limit(1)
718723

719724
if (!target) {
720-
throw new Error('Target folder not found')
725+
throw new OrchestrationError('not_found', 'Target folder not found')
721726
}
722727
}
723728

724729
if (folderIds.includes(targetFolderId ?? '')) {
725-
throw new Error('Cannot move a folder into itself')
730+
throw new OrchestrationError('validation', 'Cannot move a folder into itself')
726731
}
727732

728733
if (folderIds.length > 0) {
@@ -740,7 +745,10 @@ export async function moveWorkspaceFileItems(params: {
740745
for (const folderId of folderIds) {
741746
const descendants = collectDescendantFolderIds(activeFolders, folderId)
742747
if (targetFolderId && descendants.includes(targetFolderId)) {
743-
throw new Error('Cannot move a folder into one of its descendants')
748+
throw new OrchestrationError(
749+
'validation',
750+
'Cannot move a folder into one of its descendants'
751+
)
744752
}
745753
}
746754
}
@@ -891,7 +899,7 @@ export async function archiveWorkspaceFileFolderRecursive(
891899
)
892900
.limit(1)
893901

894-
if (!folder) throw new Error('Folder not found')
902+
if (!folder) throw new OrchestrationError('not_found', 'Folder not found')
895903

896904
const activeFolders = await tx
897905
.select({ id: folderTable.id, parentId: folderTable.parentId })
@@ -944,7 +952,7 @@ export async function restoreWorkspaceFileFolder(
944952
): Promise<WorkspaceFileFolderRestoreResult> {
945953
const ws = await getWorkspaceWithOwner(workspaceId)
946954
if (!ws || ws.archivedAt) {
947-
throw new Error('Cannot restore folder into an archived workspace')
955+
throw new OrchestrationError('validation', 'Cannot restore folder into an archived workspace')
948956
}
949957

950958
const { restored, restoredItems } = await db.transaction(async (tx) => {
@@ -959,8 +967,8 @@ export async function restoreWorkspaceFileFolder(
959967
.limit(1)
960968
.then((rows) => rows[0] ?? null)
961969

962-
if (!raw) throw new Error('Folder not found')
963-
if (!raw.deletedAt) throw new Error('Folder is not archived')
970+
if (!raw) throw new OrchestrationError('not_found', 'Folder not found')
971+
if (!raw.deletedAt) throw new OrchestrationError('validation', 'Folder is not archived')
964972

965973
const folderDeletedAt = raw.deletedAt
966974

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

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import {
1919
} from '@/lib/billing/storage'
2020
import { normalizeVfsSegment } from '@/lib/copilot/vfs/normalize-segment'
2121
import { canonicalWorkspaceFilePath, decodeVfsPathSegments } from '@/lib/copilot/vfs/path-utils'
22-
import { OrchestrationError } from '@/lib/core/orchestration/types'
22+
import { asOrchestrationError, OrchestrationError } from '@/lib/core/orchestration/types'
2323
import { generateRequestId } from '@/lib/core/utils/request'
2424
import { generateRestoreName } from '@/lib/core/utils/restore-name'
2525
import type { DbOrTx } from '@/lib/db/types'
@@ -1123,7 +1123,7 @@ export async function updateWorkspaceFileContent(
11231123
.for('update')
11241124
.limit(1)
11251125
if (!currentFile) {
1126-
throw new Error('File not found')
1126+
throw new OrchestrationError('not_found', 'File not found')
11271127
}
11281128

11291129
// Optimistic-concurrency guard: the row is `FOR UPDATE`-locked, so comparing its committed
@@ -1170,7 +1170,7 @@ export async function updateWorkspaceFileContent(
11701170
)
11711171
.returning()
11721172
if (!updatedFile) {
1173-
throw new Error('File not found or could not be updated')
1173+
throw new OrchestrationError('not_found', 'File not found or could not be updated')
11741174
}
11751175

11761176
let updatedUsage: number | undefined
@@ -1256,8 +1256,15 @@ export async function updateWorkspaceFileContent(
12561256
// the optimistic-concurrency guard, not a failure to wrap. The orphan upload was already cleaned up
12571257
// by the inner finalization catch before it propagated here.
12581258
if (error instanceof ContentVersionConflictError) throw error
1259+
// Same reasoning for an already-classified failure: a missing file and a blown storage quota are
1260+
// caller-fixable outcomes that every surface maps to 404/413 by class. Re-wrapping them in a bare
1261+
// Error stripped that classification and turned both into a 500.
1262+
const classified = asOrchestrationError(error)
1263+
if (classified) throw classified
12591264
logger.error(`Failed to update workspace file content ${fileId}:`, error)
1260-
throw new Error(`Failed to update file content: ${getErrorMessage(error, 'Unknown error')}`)
1265+
throw new Error(`Failed to update file content: ${getErrorMessage(error, 'Unknown error')}`, {
1266+
cause: error,
1267+
})
12611268
}
12621269
}
12631270

@@ -1289,10 +1296,11 @@ export async function renameWorkspaceFile(
12891296
}
12901297

12911298
let updated: { id: string }[]
1299+
const renamedAt = new Date()
12921300
try {
12931301
updated = await db
12941302
.update(workspaceFiles)
1295-
.set({ originalName: normalizedName, updatedAt: new Date() })
1303+
.set({ originalName: normalizedName, updatedAt: renamedAt })
12961304
.where(
12971305
and(
12981306
eq(workspaceFiles.id, fileId),
@@ -1309,14 +1317,15 @@ export async function renameWorkspaceFile(
13091317
}
13101318

13111319
if (updated.length === 0) {
1312-
throw new Error('File not found or could not be renamed')
1320+
throw new OrchestrationError('not_found', 'File not found or could not be renamed')
13131321
}
13141322

13151323
logger.info(`Successfully renamed workspace file ${fileId} to "${normalizedName}"`)
13161324

13171325
return {
13181326
...fileRecord,
13191327
name: normalizedName,
1328+
updatedAt: renamedAt,
13201329
}
13211330
}
13221331

@@ -1337,7 +1346,7 @@ export async function moveRenameWorkspaceFile(params: {
13371346

13381347
const fileRecord = await getWorkspaceFile(params.workspaceId, params.fileId)
13391348
if (!fileRecord) {
1340-
throw new Error('File not found')
1349+
throw new OrchestrationError('not_found', 'File not found')
13411350
}
13421351

13431352
const targetFolderId = await assertWorkspaceFileFolderTarget(
@@ -1377,7 +1386,7 @@ export async function moveRenameWorkspaceFile(params: {
13771386
}
13781387

13791388
if (updated.length === 0) {
1380-
throw new Error('File not found or could not be moved')
1389+
throw new OrchestrationError('not_found', 'File not found or could not be moved')
13811390
}
13821391

13831392
return {
@@ -1400,7 +1409,7 @@ export async function deleteWorkspaceFile(workspaceId: string, fileId: string):
14001409
try {
14011410
const fileRecord = await findWorkspaceFileForLifecycle(db, workspaceId, fileId)
14021411
if (!fileRecord) {
1403-
throw new Error('File not found')
1412+
throw new OrchestrationError('not_found', 'File not found')
14041413
}
14051414
if (fileRecord.deletedAt) return
14061415

@@ -1442,7 +1451,7 @@ export async function restoreWorkspaceFile(workspaceId: string, fileId: string):
14421451

14431452
const ws = await getWorkspaceWithOwner(workspaceId)
14441453
if (!ws || ws.archivedAt) {
1445-
throw new Error('Cannot restore file into an archived workspace')
1454+
throw new OrchestrationError('validation', 'Cannot restore file into an archived workspace')
14461455
}
14471456

14481457
/**

0 commit comments

Comments
 (0)