Skip to content

Commit 8ccf529

Browse files
fix(api): report the lock kind on classified 423s too, not just thrown ones
The previous commit named the lock only where the rejection was thrown and caught at the route boundary. Where it instead arrives as a classified `errorCode: 'locked'` outcome — delete table, delete row, update column, and the table mutations — the kind was dropped, so those 423s stayed unactionable while their neighbours improved. The orchestration results now carry `lock`, and a shared `v2TableOrchestrationError` renders both arrival paths into the same `{ code, message, details: { lock } }` body. `details` is omitted rather than sent null when the kind is unknown, so a caller branching on it sees absence instead of a phantom value.
1 parent cfbaee7 commit 8ccf529

9 files changed

Lines changed: 109 additions & 45 deletions

File tree

apps/sim/app/api/v2/tables/[tableId]/columns/route.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,11 @@ import {
1919
v2CaughtOrchestrationError,
2020
v2Data,
2121
v2Error,
22-
v2ErrorForOrchestration,
2322
v2RateLimitError,
2423
v2ValidationError,
2524
v2WorkspaceAccessError,
2625
} from '@/app/api/v2/lib/response'
27-
import { v2TableAccessError } from '@/app/api/v2/tables/utils'
26+
import { v2TableAccessError, v2TableOrchestrationError } from '@/app/api/v2/tables/utils'
2827

2928
const logger = createLogger('V2TableColumnsAPI')
3029

@@ -136,7 +135,7 @@ export const PATCH = withRouteHandler(async (request: NextRequest, context: Colu
136135
request,
137136
})
138137
if (!outcome.success || !outcome.table) {
139-
return v2ErrorForOrchestration(outcome.errorCode, outcome.error ?? 'Failed to update column')
138+
return v2TableOrchestrationError(outcome, 'Failed to update column')
140139
}
141140

142141
return v2Data({ columns: outcome.table.schema.columns.map(normalizeColumn) }, { rateLimit })

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

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,16 @@ import { v2ApiGateError } from '@/app/api/v2/lib/gate'
2020
import {
2121
v2Data,
2222
v2Error,
23-
v2ErrorForOrchestration,
2423
v2RateLimitError,
2524
v2ValidationError,
2625
v2WorkspaceAccessError,
2726
} from '@/app/api/v2/lib/response'
28-
import { v2CsvBodyCapError, v2MultipartError, v2TableAccessError } from '@/app/api/v2/tables/utils'
27+
import {
28+
v2CsvBodyCapError,
29+
v2MultipartError,
30+
v2TableAccessError,
31+
v2TableOrchestrationError,
32+
} from '@/app/api/v2/tables/utils'
2933

3034
const logger = createLogger('V2TableImportAPI')
3135

@@ -116,14 +120,7 @@ export const POST = withRouteHandler(async (request: NextRequest, context: Table
116120
})
117121

118122
if (!outcome.success || !outcome.data) {
119-
// Naming the lock is the difference between an actionable 423 and one the
120-
// caller has to guess at — there are four flags.
121-
if (outcome.errorCode === 'locked') {
122-
return v2Error('LOCKED', outcome.error ?? 'Table is locked', {
123-
details: { lock: outcome.lock },
124-
})
125-
}
126-
return v2ErrorForOrchestration(outcome.errorCode, outcome.error ?? 'Failed to import CSV')
123+
return v2TableOrchestrationError(outcome, 'Failed to import CSV')
127124
}
128125

129126
return v2Data(outcome.data, { rateLimit })

apps/sim/app/api/v2/tables/[tableId]/route.ts

Lines changed: 18 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,16 @@ import {
2828
v2CaughtOrchestrationError,
2929
v2Data,
3030
v2Error,
31-
v2ErrorForOrchestration,
3231
v2RateLimitError,
3332
v2ValidationError,
3433
v2WorkspaceAccessError,
3534
} from '@/app/api/v2/lib/response'
36-
import { toApiTable, v2TableAccessError, v2TableLockError } from '@/app/api/v2/tables/utils'
35+
import {
36+
toApiTable,
37+
v2TableAccessError,
38+
v2TableLockError,
39+
v2TableOrchestrationError,
40+
} from '@/app/api/v2/tables/utils'
3741

3842
const logger = createLogger('V2TableDetailAPI')
3943

@@ -184,10 +188,7 @@ export const PATCH = withRouteHandler(async (request: NextRequest, context: Tabl
184188
if (outcome.success) {
185189
applied = true
186190
} else {
187-
failure = v2ErrorForOrchestration(
188-
outcome.errorCode,
189-
outcome.error ?? 'Failed to update table locks'
190-
)
191+
failure = v2TableOrchestrationError(outcome, 'Failed to update table locks')
191192
}
192193
}
193194

@@ -202,10 +203,7 @@ export const PATCH = withRouteHandler(async (request: NextRequest, context: Tabl
202203
if (outcome.success) {
203204
applied = true
204205
} else {
205-
failure = v2ErrorForOrchestration(
206-
outcome.errorCode,
207-
outcome.error ?? 'Failed to rename table'
208-
)
206+
failure = v2TableOrchestrationError(outcome, 'Failed to rename table')
209207
}
210208
}
211209

@@ -217,16 +215,16 @@ export const PATCH = withRouteHandler(async (request: NextRequest, context: Tabl
217215
requestId,
218216
request,
219217
})
220-
if (outcome.success) applied = true
221-
// The move re-asserts workspace and active state, so a miss means the
222-
// table was archived between `checkAccess` and the write.
223-
else
224-
failure = v2ErrorForOrchestration(
225-
outcome.errorCode,
226-
outcome.errorCode === 'not_found'
227-
? 'Table not found'
228-
: (outcome.error ?? 'Failed to move table')
218+
if (outcome.success) {
219+
applied = true
220+
} else {
221+
// The move re-asserts workspace and active state, so a miss means the
222+
// table was archived between `checkAccess` and the write.
223+
failure = v2TableOrchestrationError(
224+
outcome.errorCode === 'not_found' ? { ...outcome, error: 'Table not found' } : outcome,
225+
'Failed to move table'
229226
)
227+
}
230228
}
231229

232230
// Live-collab: tell open viewers the definition changed so they refetch.
@@ -285,7 +283,7 @@ export const DELETE = withRouteHandler(async (request: NextRequest, context: Tab
285283

286284
const outcome = await performDeleteTable({ table: result.table, userId, requestId, request })
287285
if (!outcome.success) {
288-
return v2ErrorForOrchestration(outcome.errorCode, outcome.error ?? 'Failed to delete table')
286+
return v2TableOrchestrationError(outcome, 'Failed to delete table')
289287
}
290288

291289
return v2Data({ id: tableId }, { rateLimit })

apps/sim/app/api/v2/tables/[tableId]/rows/[rowId]/route.test.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,4 +96,27 @@ describe('DELETE /api/v2/tables/[tableId]/rows/[rowId]', () => {
9696
expect(res.status).toBe(status)
9797
expect((await res.json()).error.code).toBe(code)
9898
})
99+
100+
it('names the lock on a 423 that arrived as a classified outcome, not a throw', async () => {
101+
mockPerformDeleteRow.mockResolvedValue({
102+
success: false,
103+
errorCode: 'locked',
104+
error: 'Row deletes are locked for this table',
105+
lock: 'delete',
106+
})
107+
108+
const res = await callDelete()
109+
110+
expect(res.status).toBe(423)
111+
expect((await res.json()).error.details).toEqual({ lock: 'delete' })
112+
})
113+
114+
it('omits details entirely when the lock kind is unknown', async () => {
115+
// A caller branching on `details.lock` should see absence, not a null.
116+
mockPerformDeleteRow.mockResolvedValue({ success: false, errorCode: 'locked', error: 'nope' })
117+
118+
const res = await callDelete()
119+
120+
expect((await res.json()).error.details).toBeUndefined()
121+
})
99122
})

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

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,16 @@ import {
2323
v2CaughtOrchestrationError,
2424
v2Data,
2525
v2Error,
26-
v2ErrorForOrchestration,
2726
v2RateLimitError,
2827
v2ValidationError,
2928
v2WorkspaceAccessError,
3029
} from '@/app/api/v2/lib/response'
31-
import { toApiRow, v2TableAccessError, v2TableLockError } from '@/app/api/v2/tables/utils'
30+
import {
31+
toApiRow,
32+
v2TableAccessError,
33+
v2TableLockError,
34+
v2TableOrchestrationError,
35+
} from '@/app/api/v2/tables/utils'
3236

3337
const logger = createLogger('V2TableRowAPI')
3438

@@ -209,7 +213,7 @@ export const DELETE = withRouteHandler(async (request: NextRequest, context: Row
209213

210214
const outcome = await performDeleteTableRow({ table: result.table, rowId, requestId })
211215
if (!outcome.success) {
212-
return v2ErrorForOrchestration(outcome.errorCode, outcome.error ?? 'Failed to delete row')
216+
return v2TableOrchestrationError(outcome, 'Failed to delete row')
213217
}
214218

215219
// v2 mirrors the bulk delete shape: always returns `deletedRowIds`.

apps/sim/app/api/v2/tables/utils.ts

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import type { NextResponse } from 'next/server'
2+
import type { OrchestrationErrorCode } from '@/lib/core/orchestration/types'
23
import type { MultipartError } from '@/lib/core/utils/multipart'
34
import type { RowData, TableDefinition, TablePredicate, TableSchema } from '@/lib/table'
45
import { getColumnId } from '@/lib/table/column-keys'
@@ -9,15 +10,15 @@ import {
910
validateStoragePredicate,
1011
} from '@/lib/table/query-builder/validate'
1112
import { predicateToStorage } from '@/lib/table/select-values'
12-
import type { Filter } from '@/lib/table/types'
13+
import type { Filter, TableLockKind } from '@/lib/table/types'
1314
import type { TableView } from '@/lib/table/views/service'
1415
import {
1516
CSV_IMPORT_PROXY_BODY_CAP_BYTES,
1617
normalizeColumn,
1718
rootErrorMessage,
1819
rowWriteErrorResponse,
1920
} from '@/app/api/table/utils'
20-
import { v2Error } from '@/app/api/v2/lib/response'
21+
import { v2Error, v2ErrorForOrchestration } from '@/app/api/v2/lib/response'
2122

2223
/**
2324
* Shared serialization + error helpers for the v2 tables surface. Every v2
@@ -178,6 +179,31 @@ export function v2TableLockError(error: unknown): NextResponse | null {
178179
return null
179180
}
180181

182+
/**
183+
* Renders a `lib/table/orchestration` failure in the v2 envelope, naming the
184+
* lock when one caused it.
185+
*
186+
* A lock rejection reaches a route two different ways — thrown and caught at
187+
* the boundary ({@link v2TableLockError}), or returned as a classified
188+
* `errorCode: 'locked'` outcome — and both must produce the same body. Plain
189+
* {@link v2ErrorForOrchestration} cannot, because the `lock` kind lives on the
190+
* outcome rather than the code, so every table route that renders an
191+
* orchestration result goes through this instead.
192+
*/
193+
export function v2TableOrchestrationError(
194+
outcome: { errorCode?: OrchestrationErrorCode; error?: string; lock?: TableLockKind },
195+
fallback: string
196+
): NextResponse {
197+
if (outcome.errorCode === 'locked') {
198+
return v2Error('LOCKED', outcome.error ?? fallback, {
199+
// Omitted rather than sent as null when the kind is unknown — a caller
200+
// branching on `details.lock` should see absence, not a phantom value.
201+
...(outcome.lock ? { details: { lock: outcome.lock } } : {}),
202+
})
203+
}
204+
return v2ErrorForOrchestration(outcome.errorCode, outcome.error ?? fallback)
205+
}
206+
181207
/**
182208
* Maps a known user-facing row-write failure (schema/size/unique/limit) to a v2
183209
* `BAD_REQUEST`, reusing v1's {@link rowWriteErrorResponse} classifier as the

apps/sim/lib/table/orchestration/columns.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import {
1818
import { isSupportedCurrencyCode } from '@/lib/table/currency'
1919
import { TableLockedError } from '@/lib/table/mutation-locks'
2020
import { normalizeSelectOptionsInput } from '@/lib/table/select-options'
21-
import type { ColumnType, SelectOption, TableDefinition } from '@/lib/table/types'
21+
import type { ColumnType, SelectOption, TableDefinition, TableLockKind } from '@/lib/table/types'
2222

2323
const logger = createLogger('TableColumnOrchestration')
2424

@@ -45,12 +45,14 @@ export interface PerformUpdateTableColumnResult {
4545
success: boolean
4646
error?: string
4747
errorCode?: OrchestrationErrorCode
48+
/** Which lock rejected the write. Set only when `errorCode` is `'locked'`. */
49+
lock?: TableLockKind
4850
table?: TableDefinition
4951
}
5052

5153
function classify(error: unknown): PerformUpdateTableColumnResult {
5254
if (error instanceof TableLockedError) {
53-
return { success: false, error: error.message, errorCode: 'locked' }
55+
return { success: false, error: error.message, errorCode: 'locked', lock: error.lock }
5456
}
5557
if (error instanceof OrchestrationError) {
5658
return { success: false, error: error.message, errorCode: error.code }

apps/sim/lib/table/orchestration/tables.test.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ describe('performDeleteTable', () => {
8585

8686
const result = await performDeleteTable({ table: TABLE, userId: 'user-1' })
8787

88-
expect(result).toMatchObject({ success: false, errorCode: 'locked' })
88+
expect(result).toMatchObject({ success: false, errorCode: 'locked', lock: 'delete' })
8989
expect(mockCaptureServerEvent).not.toHaveBeenCalled()
9090
})
9191
})
@@ -129,7 +129,10 @@ describe('performDeleteTableRow', () => {
129129
it('classifies a delete lock as locked', async () => {
130130
mockDeleteRow.mockRejectedValue(new TableLockedError('delete'))
131131

132-
expect((await performDeleteTableRow({ table: TABLE, rowId: 'row-1' })).errorCode).toBe('locked')
132+
const rowResult = await performDeleteTableRow({ table: TABLE, rowId: 'row-1' })
133+
expect(rowResult.errorCode).toBe('locked')
134+
// The kind rides along so the route can name which flag to clear.
135+
expect(rowResult.lock).toBe('delete')
133136
})
134137

135138
it('classifies a missing row as not_found', async () => {

apps/sim/lib/table/orchestration/tables.ts

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {
1515
TABLE_LOCK_FLAGS,
1616
TABLE_LOCK_KINDS,
1717
type TableDefinition,
18+
type TableLockKind,
1819
type TableLocks,
1920
} from '@/lib/table/types'
2021

@@ -32,6 +33,8 @@ export interface PerformDeleteTableResult {
3233
success: boolean
3334
error?: string
3435
errorCode?: OrchestrationErrorCode
36+
/** Which lock rejected the write. Set only when `errorCode` is `'locked'`. */
37+
lock?: TableLockKind
3538
}
3639

3740
/**
@@ -54,7 +57,7 @@ export async function performDeleteTable(
5457
;({ archived } = await deleteTable(table.id, requestId))
5558
} catch (error) {
5659
if (error instanceof TableLockedError) {
57-
return { success: false, error: error.message, errorCode: 'locked' }
60+
return { success: false, error: error.message, errorCode: 'locked', lock: error.lock }
5861
}
5962
if (error instanceof OrchestrationError) {
6063
return { success: false, error: error.message, errorCode: error.code }
@@ -98,6 +101,8 @@ export interface PerformDeleteTableRowResult {
98101
success: boolean
99102
error?: string
100103
errorCode?: OrchestrationErrorCode
104+
/** Which lock rejected the write. Set only when `errorCode` is `'locked'`. */
105+
lock?: TableLockKind
101106
}
102107

103108
/**
@@ -116,7 +121,7 @@ export async function performDeleteTableRow(
116121
return { success: true }
117122
} catch (error) {
118123
if (error instanceof TableLockedError) {
119-
return { success: false, error: error.message, errorCode: 'locked' }
124+
return { success: false, error: error.message, errorCode: 'locked', lock: error.lock }
120125
}
121126
if (error instanceof OrchestrationError) {
122127
return { success: false, error: error.message, errorCode: error.code }
@@ -139,12 +144,19 @@ export interface PerformTableMutationResult {
139144
success: boolean
140145
error?: string
141146
errorCode?: OrchestrationErrorCode
147+
/** Which lock rejected the write. Set only when `errorCode` is `'locked'`. */
148+
lock?: TableLockKind
142149
table?: TableDefinition
143150
}
144151

145152
function classifyTableMutation(error: unknown, requestId: string, tableId: string) {
146153
if (error instanceof TableLockedError) {
147-
return { success: false as const, error: error.message, errorCode: 'locked' as const }
154+
return {
155+
success: false as const,
156+
error: error.message,
157+
errorCode: 'locked' as const,
158+
lock: error.lock,
159+
}
148160
}
149161
// `TableConflictError` is an `OrchestrationError('conflict')`, so a duplicate
150162
// rename reaches 409 through this branch — by class, not by the message

0 commit comments

Comments
 (0)