Skip to content

Commit d449610

Browse files
refactor(tables): classify failures by type instead of by message text
The table module decided HTTP statuses by searching error messages for phrases. `VALIDATION_MESSAGE_FRAGMENTS` and `ROW_WRITE_ERROR_PATTERNS` held 32 substrings between them, and fifteen more lists were inlined in routes — 83 matchers over 17 files, each its own copy of the guesswork and already drifted apart. It made message wording load-bearing: `TableRowLimitError`'s own doc comment noted that its text had to contain "row limit" for a route to answer 400, and adding "already exists" to a rename message silently demoted a 409 to a 400 (the bug fixed one commit ago, by adding another special case). Services now throw `OrchestrationError`, which carries the transport-neutral `OrchestrationErrorCode` the layers above already speak. Classification is one `instanceof` in `orchestrationErrorResponse` (UI + v1) and `v2CaughtOrchestrationError` (v2). Every pattern list is gone. Wording is free to change; an unclassified error still becomes a generic 500, which is what an unexpected fault should be. `asOrchestrationError` walks the `cause` chain rather than testing the caught value directly: drizzle wraps a throw raised inside a transaction callback in a `DrizzleQueryError` whose own message is the failed SQL, so a bare `instanceof` would drop every failure raised inside `withLockedTable`. That is the same reason `rootErrorMessage` had to dig for a root cause before. Three throws stay bare `Error` deliberately — `Table ID mismatch`, `Workspace ID mismatch`, and `Failed to build upsert conflict predicate` are internal invariants no consumer classified, and they keep falling through to a 500. `Insufficient capacity` was in the pattern list with no producer anywhere in the codebase. Status changes, all deliberate: - `'forbidden'` joins the code union so the table-row-limit ceiling keeps its 403; without it this refactor would have flattened it to 400. - import-async's table-limit rejection: 400 -> 403, matching the two other create routes it had drifted from. - Renaming a table to an invalid name: 500 -> 400. `validateTableName` messages don't contain "Invalid", so no matcher ever caught them. - Restoring a table that isn't archived, or into an archived workspace: 500 -> 400. - A duplicate *column* name stays `validation`/400 rather than becoming a 409 like a duplicate table name. Both v1 and the orchestration have always answered 400 for it; changing a published status is not this refactor's job. The twelve tests that changed were asserting the substring mechanism itself, constructing plain `Error`s with magic strings. They now assert the real contract, plus new cases pinning that identical wording carrying no classification stays internal and keeps its message off the wire. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YGzbVDZpe2dEALbu2BUU8a
1 parent 0ee4499 commit d449610

32 files changed

Lines changed: 532 additions & 447 deletions

File tree

apps/sim/app/api/table/[tableId]/columns/route.test.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ vi.mock('@/app/api/table/utils', () => ({
5757
tableLockErrorResponse: () => null,
5858
}))
5959

60+
import { OrchestrationError } from '@/lib/core/orchestration/types'
6061
import { PATCH } from '@/app/api/table/[tableId]/columns/route'
6162

6263
const WORKSPACE_ID = '11111111-1111-4111-8111-111111111111'
@@ -166,7 +167,10 @@ describe('PATCH /api/table/[tableId]/columns — pre-flight guards', () => {
166167
// Stands in for the race the guards cannot close: the column stopped being
167168
// a currency between the snapshot the guards read and this write.
168169
mockUpdateColumnCurrency.mockRejectedValue(
169-
new Error('Cannot set currency on column "amount" of type "string"')
170+
new OrchestrationError(
171+
'validation',
172+
'Cannot set currency on column "amount" of type "string"'
173+
)
170174
)
171175

172176
const response = await patch({ name: 'renamed', currencyCode: 'USD' })

apps/sim/app/api/table/[tableId]/columns/run/route.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,12 @@ import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
88
import { TableQueryValidationError } from '@/lib/table/errors'
99
import { toLegacyFilter } from '@/lib/table/query-builder/converters'
1010
import { runWorkflowColumn } from '@/lib/table/workflow-columns'
11-
import { accessError, checkAccess, tableFilterError } from '@/app/api/table/utils'
11+
import {
12+
accessError,
13+
checkAccess,
14+
orchestrationErrorResponse,
15+
tableFilterError,
16+
} from '@/app/api/table/utils'
1217

1318
const logger = createLogger('TableRunColumnAPI')
1419

@@ -66,9 +71,8 @@ export const POST = withRouteHandler(async (request: NextRequest, { params }: Ro
6671
if (error instanceof TableQueryValidationError) {
6772
return NextResponse.json({ error: error.message }, { status: 400 })
6873
}
69-
if (error instanceof Error && error.message === 'Invalid workspace ID') {
70-
return NextResponse.json({ error: 'Invalid workspace ID' }, { status: 400 })
71-
}
74+
const classified = orchestrationErrorResponse(error)
75+
if (classified) return classified
7276
logger.error(`run-column failed:`, error)
7377
return NextResponse.json({ error: 'Failed to run columns' }, { status: 500 })
7478
}

apps/sim/app/api/table/[tableId]/import/route.test.ts

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ vi.mock('@/lib/table/billing', () => ({
7979
limit >= 0 && current + added > limit,
8080
}))
8181

82+
import { OrchestrationError } from '@/lib/core/orchestration/types'
8283
import { TableLockedError } from '@/lib/table/mutation-locks'
8384
import { POST } from '@/app/api/table/[tableId]/import/route'
8485

@@ -372,7 +373,10 @@ describe('POST /api/table/[tableId]/import', () => {
372373

373374
it('surfaces unique violations from importAppendRows as 400', async () => {
374375
mockImportAppendRows.mockRejectedValueOnce(
375-
new Error('Row 1: Column "name" must be unique. Value "Alice" already exists in row row_xxx')
376+
new OrchestrationError(
377+
'validation',
378+
'Row 1: Column "name" must be unique. Value "Alice" already exists in row row_xxx'
379+
)
376380
)
377381
const response = await callPost(
378382
createFormData(createCsvFile('name,age\nAlice,30'), { mode: 'append' })
@@ -516,7 +520,9 @@ describe('POST /api/table/[tableId]/import', () => {
516520
})
517521

518522
it('surfaces column-creation failures from importAppendRows as 400', async () => {
519-
mockImportAppendRows.mockRejectedValueOnce(new Error('Column "email" already exists'))
523+
mockImportAppendRows.mockRejectedValueOnce(
524+
new OrchestrationError('validation', 'Column "email" already exists')
525+
)
520526
const response = await callPost(
521527
createFormData(createCsvFile('name,age,email\nAlice,30,a@x.io'), {
522528
mode: 'append',
@@ -529,7 +535,9 @@ describe('POST /api/table/[tableId]/import', () => {
529535
})
530536

531537
it('surfaces row insert failures without success when schema was mutated', async () => {
532-
mockImportAppendRows.mockRejectedValueOnce(new Error('must be unique'))
538+
mockImportAppendRows.mockRejectedValueOnce(
539+
new OrchestrationError('validation', 'must be unique')
540+
)
533541
const response = await callPost(
534542
createFormData(createCsvFile('name,age,email\nAlice,30,a@x.io'), {
535543
mode: 'append',

apps/sim/app/api/table/[tableId]/import/route.ts

Lines changed: 13 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
import { ianaTimezoneSchema } from '@/lib/api/contracts/user'
1515
import { getValidationErrorMessage } from '@/lib/api/server'
1616
import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid'
17+
import { asOrchestrationError, statusForOrchestrationError } from '@/lib/core/orchestration/types'
1718
import { isMultipartError, readMultipart } from '@/lib/core/utils/multipart'
1819
import { generateRequestId } from '@/lib/core/utils/request'
1920
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
@@ -349,21 +350,13 @@ export const POST = withRouteHandler(async (request: NextRequest, { params }: Ro
349350
createdColumns: additions.length,
350351
error: message,
351352
})
352-
const isClientError =
353-
message.includes('row limit') ||
354-
message.includes('Insufficient capacity') ||
355-
message.includes('Schema validation') ||
356-
message.includes('must be unique') ||
357-
message.includes('Row size exceeds') ||
358-
message.includes('already exists') ||
359-
message.includes('Invalid column name') ||
360-
/^Row \d+:/.test(message)
353+
const classified = asOrchestrationError(err)
361354
return NextResponse.json(
362355
{
363-
error: isClientError ? message : 'Failed to import CSV',
356+
error: classified ? classified.message : 'Failed to import CSV',
364357
data: { insertedCount: 0 },
365358
},
366-
{ status: isClientError ? 400 : 500 }
359+
{ status: classified ? statusForOrchestrationError(classified.code) : 500 }
367360
)
368361
}
369362
}
@@ -400,17 +393,12 @@ export const POST = withRouteHandler(async (request: NextRequest, { params }: Ro
400393
},
401394
})
402395
} catch (err) {
403-
const message = toError(err).message
404-
const isClientError =
405-
message.includes('row limit') ||
406-
message.includes('Schema validation') ||
407-
message.includes('must be unique') ||
408-
message.includes('Row size exceeds') ||
409-
message.includes('already exists') ||
410-
message.includes('Invalid column name') ||
411-
/^Row \d+:/.test(message)
412-
if (isClientError) {
413-
return NextResponse.json({ error: message }, { status: 400 })
396+
const classified = asOrchestrationError(err)
397+
if (classified) {
398+
return NextResponse.json(
399+
{ error: classified.message },
400+
{ status: statusForOrchestrationError(classified.code) }
401+
)
414402
}
415403
throw err
416404
}
@@ -419,17 +407,12 @@ export const POST = withRouteHandler(async (request: NextRequest, { params }: Ro
419407
if (lockError) return lockError
420408
if (isMultipartError(error)) return multipartErrorResponse(error)
421409

422-
const message = toError(error).message
423410
logger.error(`[${requestId}] CSV import into existing table failed:`, error)
424411

425-
const isClientError =
426-
message.includes('CSV file has no') ||
427-
message.includes('already exists') ||
428-
message.includes('Invalid column name')
429-
412+
const classified = asOrchestrationError(error)
430413
return NextResponse.json(
431-
{ error: isClientError ? message : 'Failed to import CSV' },
432-
{ status: isClientError ? 400 : 500 }
414+
{ error: classified ? classified.message : 'Failed to import CSV' },
415+
{ status: classified ? statusForOrchestrationError(classified.code) : 500 }
433416
)
434417
} finally {
435418
fileStream?.destroy()

apps/sim/app/api/table/[tableId]/rows/[rowId]/route.ts

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import { db } from '@sim/db'
22
import { userTableRows } from '@sim/db/schema'
33
import { createLogger } from '@sim/logger'
4-
import { toError } from '@sim/utils/errors'
54
import { and, eq } from 'drizzle-orm'
65
import { type NextRequest, NextResponse } from 'next/server'
76
import {
@@ -21,7 +20,7 @@ import { rowWireTranslators } from '@/app/api/table/row-wire'
2120
import {
2221
accessError,
2322
checkAccess,
24-
rootErrorMessage,
23+
orchestrationErrorResponse,
2524
rowWriteErrorResponse,
2625
tableLockErrorResponse,
2726
} from '@/app/api/table/utils'
@@ -175,10 +174,6 @@ export const PATCH = withRouteHandler(async (request: NextRequest, context: RowR
175174
},
176175
})
177176
} catch (error) {
178-
if (rootErrorMessage(error) === 'Row not found') {
179-
return NextResponse.json({ error: 'Row not found' }, { status: 404 })
180-
}
181-
182177
const response = rowWriteErrorResponse(error)
183178
if (response) return response
184179

@@ -233,11 +228,8 @@ export const DELETE = withRouteHandler(async (request: NextRequest, context: Row
233228
const lockError = tableLockErrorResponse(error)
234229
if (lockError) return lockError
235230

236-
const errorMessage = toError(error).message
237-
238-
if (errorMessage === 'Row not found') {
239-
return NextResponse.json({ error: errorMessage }, { status: 404 })
240-
}
231+
const classified = orchestrationErrorResponse(error)
232+
if (classified) return classified
241233

242234
logger.error(`[${requestId}] Error deleting row:`, error)
243235
return NextResponse.json({ error: 'Failed to delete row' }, { status: 500 })

apps/sim/app/api/table/import-async/route.ts

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,11 @@ import {
1818
releaseJobClaim,
1919
sanitizeName,
2020
TABLE_LIMITS,
21-
TableConflictError,
2221
} from '@/lib/table'
2322
import { runTableImport, type TableImportPayload } from '@/lib/table/import-runner'
2423
import { getUserSettings } from '@/lib/users/queries'
2524
import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils'
25+
import { orchestrationErrorResponse } from '@/app/api/table/utils'
2626

2727
const logger = createLogger('TableImportAsync')
2828

@@ -101,12 +101,8 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
101101
requestId
102102
)
103103
} catch (error) {
104-
if (error instanceof TableConflictError) {
105-
return NextResponse.json({ error: error.message }, { status: 409 })
106-
}
107-
if (error instanceof Error && error.message.includes('maximum table limit')) {
108-
return NextResponse.json({ error: error.message }, { status: 400 })
109-
}
104+
const classified = orchestrationErrorResponse(error)
105+
if (classified) return classified
110106
throw error
111107
}
112108

apps/sim/app/api/table/import-csv/route.test.ts

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
* @vitest-environment node
33
*/
44
import { hybridAuthMockFns, permissionsMock, permissionsMockFns } from '@sim/testing'
5-
import { getErrorMessage } from '@sim/utils/errors'
65
import type { NextRequest } from 'next/server'
76
import { beforeEach, describe, expect, it, vi } from 'vitest'
87

@@ -32,6 +31,9 @@ vi.mock('@/lib/table/rows/service', () => ({
3231
vi.mock('@/lib/table/billing', () => ({ getWorkspaceTableLimits: mockGetLimits }))
3332
vi.mock('@/app/api/table/utils', async () => {
3433
const { NextResponse } = await import('next/server')
34+
const { asOrchestrationError, statusForOrchestrationError } = await import(
35+
'@/lib/core/orchestration/types'
36+
)
3537
return {
3638
normalizeColumn: (column: unknown) => column,
3739
csvProxyBodyCapResponse: () => null,
@@ -40,16 +42,20 @@ vi.mock('@/app/api/table/utils', async () => {
4042
{ error: error.message },
4143
{ status: error.code === 'FILE_TOO_LARGE' ? 413 : 400 }
4244
),
43-
rowWriteErrorResponse: (error: unknown) => {
44-
const message = getErrorMessage(error)
45-
return message.includes('row limit')
46-
? NextResponse.json({ error: message }, { status: 400 })
45+
orchestrationErrorResponse: (error: unknown) => {
46+
const classified = asOrchestrationError(error)
47+
return classified
48+
? NextResponse.json(
49+
{ error: classified.message },
50+
{ status: statusForOrchestrationError(classified.code) }
51+
)
4752
: null
4853
},
4954
}
5055
})
5156
vi.mock('@/lib/workspaces/permissions/utils', () => permissionsMock)
5257

58+
import { OrchestrationError } from '@/lib/core/orchestration/types'
5359
import { POST } from '@/app/api/table/import-csv/route'
5460

5561
type Part =
@@ -184,7 +190,10 @@ describe('POST /api/table/import-csv', () => {
184190

185191
it('returns 400 with the reason when an insert exceeds the plan row limit', async () => {
186192
mockBatchInsertRows.mockRejectedValueOnce(
187-
new Error('This table has reached its row limit (1,000 rows) on your current plan.')
193+
new OrchestrationError(
194+
'validation',
195+
'This table has reached its row limit (1,000 rows) on your current plan.'
196+
)
188197
)
189198
const response = await POST(makeRequest(uploadParts(csvWithRows(250))))
190199
const data = await response.json()

apps/sim/app/api/table/import-csv/route.ts

Lines changed: 6 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import type { Readable } from 'node:stream'
22
import { createLogger } from '@sim/logger'
3-
import { toError } from '@sim/utils/errors'
43
import { generateId } from '@sim/utils/id'
54
import { type NextRequest, NextResponse } from 'next/server'
65
import { csvExtensionSchema, csvImportFormSchema } from '@/lib/api/contracts/tables'
@@ -34,7 +33,7 @@ import {
3433
csvProxyBodyCapResponse,
3534
multipartErrorResponse,
3635
normalizeColumn,
37-
rowWriteErrorResponse,
36+
orchestrationErrorResponse,
3837
} from '@/app/api/table/utils'
3938

4039
const logger = createLogger('TableImportCSV')
@@ -250,22 +249,12 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
250249

251250
logger.error(`[${requestId}] CSV import failed:`, error)
252251

253-
// Row-write failures (e.g. the plan row-limit check) map to a 400 with the real reason.
254-
const rowWriteError = rowWriteErrorResponse(error)
255-
if (rowWriteError) return rowWriteError
252+
// Every caller-fixable failure on this path — the plan row-limit check, the
253+
// schema and CSV-shape validation, a name collision — arrives classified.
254+
const classified = orchestrationErrorResponse(error)
255+
if (classified) return classified
256256

257-
const message = toError(error).message
258-
const isClientError =
259-
message.includes('maximum table limit') ||
260-
message.includes('CSV file has no') ||
261-
message.includes('Invalid table name') ||
262-
message.includes('Invalid schema') ||
263-
message.includes('already exists')
264-
265-
return NextResponse.json(
266-
{ error: isClientError ? message : 'Failed to import CSV' },
267-
{ status: isClientError ? 400 : 500 }
268-
)
257+
return NextResponse.json({ error: 'Failed to import CSV' }, { status: 500 })
269258
} finally {
270259
fileStream?.destroy()
271260
}

apps/sim/app/api/table/route.ts

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import {
1616
type TableScope,
1717
} from '@/lib/table'
1818
import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils'
19-
import { normalizeColumn } from '@/app/api/table/utils'
19+
import { normalizeColumn, orchestrationErrorResponse } from '@/app/api/table/utils'
2020

2121
const logger = createLogger('TableAPI')
2222

@@ -153,18 +153,8 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
153153
},
154154
})
155155
} catch (error) {
156-
if (error instanceof Error) {
157-
if (error.message.includes('maximum table limit')) {
158-
return NextResponse.json({ error: error.message }, { status: 403 })
159-
}
160-
if (
161-
error.message.includes('Invalid table name') ||
162-
error.message.includes('Invalid schema') ||
163-
error.message.includes('already exists')
164-
) {
165-
return NextResponse.json({ error: error.message }, { status: 400 })
166-
}
167-
}
156+
const classified = orchestrationErrorResponse(error)
157+
if (classified) return classified
168158

169159
logger.error(`[${requestId}] Error creating table:`, error)
170160
return NextResponse.json({ error: 'Failed to create table' }, { status: 500 })

0 commit comments

Comments
 (0)