diff --git a/apps/sim/app/api/table/[tableId]/columns/route.test.ts b/apps/sim/app/api/table/[tableId]/columns/route.test.ts index e8497a0fa04..6223c12bff6 100644 --- a/apps/sim/app/api/table/[tableId]/columns/route.test.ts +++ b/apps/sim/app/api/table/[tableId]/columns/route.test.ts @@ -10,7 +10,7 @@ */ import { hybridAuthMockFns } from '@sim/testing' import { getErrorMessage } from '@sim/utils/errors' -import { NextRequest } from 'next/server' +import { NextRequest, NextResponse } from 'next/server' import { beforeEach, describe, expect, it, vi } from 'vitest' const { @@ -53,11 +53,24 @@ vi.mock('@/app/api/table/utils', () => ({ accessError: () => new Response('denied', { status: 403 }), checkAccess: mockCheckAccess, normalizeColumn: (c: unknown) => c, + orchestrationOutcomeErrorResponse: ( + outcome: { error?: string; errorCode?: OrchestrationErrorCode }, + fallback: string + ) => + NextResponse.json( + { error: messageForOrchestrationError(outcome, fallback) }, + { status: statusForOrchestrationError(outcome.errorCode) } + ), rootErrorMessage: (e: unknown) => getErrorMessage(e), tableLockErrorResponse: () => null, })) -import { OrchestrationError } from '@/lib/core/orchestration/types' +import { + messageForOrchestrationError, + OrchestrationError, + type OrchestrationErrorCode, + statusForOrchestrationError, +} from '@/lib/core/orchestration/types' import { PATCH } from '@/app/api/table/[tableId]/columns/route' const WORKSPACE_ID = '11111111-1111-4111-8111-111111111111' diff --git a/apps/sim/app/api/table/[tableId]/columns/route.ts b/apps/sim/app/api/table/[tableId]/columns/route.ts index d899372d2be..54ca6de54e8 100644 --- a/apps/sim/app/api/table/[tableId]/columns/route.ts +++ b/apps/sim/app/api/table/[tableId]/columns/route.ts @@ -8,7 +8,6 @@ import { import { parseRequest } from '@/lib/api/server' import { isZodError, validationErrorResponse } from '@/lib/api/server/validation' import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid' -import { statusForOrchestrationError } from '@/lib/core/orchestration/types' import { generateRequestId } from '@/lib/core/utils/request' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' import { addTableColumn, deleteColumn } from '@/lib/table' @@ -18,6 +17,7 @@ import { accessError, checkAccess, normalizeColumn, + orchestrationOutcomeErrorResponse, rootErrorMessage, tableLockErrorResponse, } from '@/app/api/table/utils' @@ -122,10 +122,7 @@ export const PATCH = withRouteHandler(async (request: NextRequest, context: Colu request, }) if (!outcome.success || !outcome.table) { - return NextResponse.json( - { error: outcome.error ?? 'Failed to update column' }, - { status: statusForOrchestrationError(outcome.errorCode) } - ) + return orchestrationOutcomeErrorResponse(outcome, 'Failed to update column') } // Live-collab: tell open viewers the change landed so they refetch. diff --git a/apps/sim/app/api/table/[tableId]/route.ts b/apps/sim/app/api/table/[tableId]/route.ts index 12f40106575..e14a3bdf775 100644 --- a/apps/sim/app/api/table/[tableId]/route.ts +++ b/apps/sim/app/api/table/[tableId]/route.ts @@ -5,7 +5,6 @@ import { getTableQuerySchema, updateTableContract } from '@/lib/api/contracts/ta import { isZodError, parseRequest, validationErrorResponse } from '@/lib/api/server/validation' import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid' import { isFeatureEnabled } from '@/lib/core/config/feature-flags' -import { statusForOrchestrationError } from '@/lib/core/orchestration/types' import { generateRequestId } from '@/lib/core/utils/request' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' import { findActiveFolder } from '@/lib/folders/queries' @@ -24,6 +23,7 @@ import { accessError, checkAccess, normalizeColumn, + orchestrationOutcomeErrorResponse, tableLockErrorResponse, } from '@/app/api/table/utils' @@ -187,10 +187,7 @@ export const PATCH = withRouteHandler( request, }) if (!lockOutcome.success) { - return NextResponse.json( - { error: lockOutcome.error ?? 'Failed to update table locks' }, - { status: statusForOrchestrationError(lockOutcome.errorCode) } - ) + return orchestrationOutcomeErrorResponse(lockOutcome, 'Failed to update table locks') } } @@ -203,10 +200,7 @@ export const PATCH = withRouteHandler( request, }) if (!renameOutcome.success) { - return NextResponse.json( - { error: renameOutcome.error ?? 'Failed to rename table' }, - { status: statusForOrchestrationError(renameOutcome.errorCode) } - ) + return orchestrationOutcomeErrorResponse(renameOutcome, 'Failed to rename table') } } @@ -229,11 +223,11 @@ export const PATCH = withRouteHandler( request, }) if (!moveOutcome.success) { - return NextResponse.json( - { - error: moveOutcome.errorCode === 'not_found' ? 'Table not found' : moveOutcome.error, - }, - { status: statusForOrchestrationError(moveOutcome.errorCode) } + return orchestrationOutcomeErrorResponse( + moveOutcome.errorCode === 'not_found' + ? { ...moveOutcome, error: 'Table not found' } + : moveOutcome, + 'Failed to move table' ) } } @@ -302,10 +296,7 @@ export const DELETE = withRouteHandler( request, }) if (!outcome.success) { - return NextResponse.json( - { error: outcome.error ?? 'Failed to delete table' }, - { status: statusForOrchestrationError(outcome.errorCode) } - ) + return orchestrationOutcomeErrorResponse(outcome, 'Failed to delete table') } return NextResponse.json({ diff --git a/apps/sim/app/api/table/[tableId]/rows/[rowId]/route.ts b/apps/sim/app/api/table/[tableId]/rows/[rowId]/route.ts index 6e9249c5863..67a293081c0 100644 --- a/apps/sim/app/api/table/[tableId]/rows/[rowId]/route.ts +++ b/apps/sim/app/api/table/[tableId]/rows/[rowId]/route.ts @@ -10,7 +10,6 @@ import { } from '@/lib/api/contracts/tables' import { isZodError, parseRequest, validationErrorResponse } from '@/lib/api/server/validation' import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid' -import { statusForOrchestrationError } from '@/lib/core/orchestration/types' import { generateRequestId } from '@/lib/core/utils/request' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' import type { RowData, TableSchema } from '@/lib/table' @@ -27,7 +26,7 @@ import { accessError, checkAccess, orchestrationErrorResponse, - rowWriteErrorResponse, + orchestrationOutcomeErrorResponse, tableLockErrorResponse, } from '@/app/api/table/utils' @@ -211,7 +210,7 @@ export const PATCH = withRouteHandler(async (request: NextRequest, context: RowR rows: [updatedRow], }) } catch (error) { - const response = rowWriteErrorResponse(error) + const response = orchestrationErrorResponse(error) if (response) return response logger.error(`[${requestId}] Error updating row:`, error) @@ -248,10 +247,7 @@ export const DELETE = withRouteHandler(async (request: NextRequest, context: Row const outcome = await performDeleteTableRow({ table, rowId, requestId }) if (!outcome.success) { - return NextResponse.json( - { error: outcome.error ?? 'Failed to delete row' }, - { status: statusForOrchestrationError(outcome.errorCode) } - ) + return orchestrationOutcomeErrorResponse(outcome, 'Failed to delete row') } // Live-collab: tell open viewers the change landed so they refetch. diff --git a/apps/sim/app/api/table/[tableId]/rows/route.ts b/apps/sim/app/api/table/[tableId]/rows/route.ts index a18d99397ef..461e8040b19 100644 --- a/apps/sim/app/api/table/[tableId]/rows/route.ts +++ b/apps/sim/app/api/table/[tableId]/rows/route.ts @@ -40,7 +40,7 @@ import { resolveTableWriteSecretProvenance, } from '@/app/api/table/row-secret-provenance' import { type RowWireTranslators, rowWireTranslators } from '@/app/api/table/row-wire' -import { accessError, checkAccess, rowWriteErrorResponse } from '@/app/api/table/utils' +import { accessError, checkAccess, orchestrationErrorResponse } from '@/app/api/table/utils' const logger = createLogger('TableRowsAPI') @@ -168,7 +168,7 @@ async function handleBatchInsert( rows: insertedRows, }) } catch (error) { - const response = rowWriteErrorResponse(error) + const response = orchestrationErrorResponse(error) if (response) return response logger.error(`[${requestId}] Error batch inserting rows:`, error) @@ -284,7 +284,7 @@ export const POST = withRouteHandler( return validationErrorResponse(error) } - const response = rowWriteErrorResponse(error) + const response = orchestrationErrorResponse(error) if (response) return response logger.error(`[${requestId}] Error inserting row:`, error) @@ -527,7 +527,7 @@ export const PUT = withRouteHandler( return NextResponse.json({ error: error.message }, { status: 400 }) } - const response = rowWriteErrorResponse(error) + const response = orchestrationErrorResponse(error) if (response) return response logger.error(`[${requestId}] Error updating rows by filter:`, error) @@ -627,7 +627,7 @@ export const DELETE = withRouteHandler( return NextResponse.json({ error: error.message }, { status: 400 }) } - const response = rowWriteErrorResponse(error) + const response = orchestrationErrorResponse(error) if (response) return response logger.error(`[${requestId}] Error deleting rows:`, error) @@ -716,7 +716,7 @@ export const PATCH = withRouteHandler( return validationErrorResponse(error) } - const response = rowWriteErrorResponse(error) + const response = orchestrationErrorResponse(error) if (response) return response logger.error(`[${requestId}] Error batch updating rows:`, error) diff --git a/apps/sim/app/api/table/[tableId]/rows/upsert/route.ts b/apps/sim/app/api/table/[tableId]/rows/upsert/route.ts index 3ca0be7f8ba..abfbc7e0384 100644 --- a/apps/sim/app/api/table/[tableId]/rows/upsert/route.ts +++ b/apps/sim/app/api/table/[tableId]/rows/upsert/route.ts @@ -15,7 +15,7 @@ import { resolveTableWriteSecretProvenance, } from '@/app/api/table/row-secret-provenance' import { rowWireTranslators } from '@/app/api/table/row-wire' -import { accessError, checkAccess, rowWriteErrorResponse } from '@/app/api/table/utils' +import { accessError, checkAccess, orchestrationErrorResponse } from '@/app/api/table/utils' const logger = createLogger('TableUpsertAPI') @@ -105,7 +105,7 @@ export const POST = withRouteHandler(async (request: NextRequest, context: Upser return validationErrorResponse(error) } - const response = rowWriteErrorResponse(error) + const response = orchestrationErrorResponse(error) if (response) return response logger.error(`[${requestId}] Error upserting row:`, error) diff --git a/apps/sim/app/api/table/utils.test.ts b/apps/sim/app/api/table/utils.test.ts index fa7b57ac6dd..34c2a14cbbf 100644 --- a/apps/sim/app/api/table/utils.test.ts +++ b/apps/sim/app/api/table/utils.test.ts @@ -5,7 +5,12 @@ import { describe, expect, it } from 'vitest' import { OrchestrationError } from '@/lib/core/orchestration/types' import { TableRowLimitError } from '@/lib/table/billing' import type { ColumnDefinition } from '@/lib/table/types' -import { rootErrorMessage, rowWriteErrorResponse, tableFilterError } from '@/app/api/table/utils' +import { + orchestrationErrorResponse, + orchestrationOutcomeErrorResponse, + rootErrorMessage, + tableFilterError, +} from '@/app/api/table/utils' /** Mimics drizzle's DrizzleQueryError: message is the failed SQL, real error on `cause`. */ function wrapLikeDrizzle(cause: Error): Error { @@ -29,9 +34,9 @@ describe('rootErrorMessage', () => { }) }) -describe('rowWriteErrorResponse', () => { +describe('orchestrationErrorResponse', () => { it('passes the plan row-limit error through as a 400', async () => { - const response = rowWriteErrorResponse(new TableRowLimitError(10000)) + const response = orchestrationErrorResponse(new TableRowLimitError(10000)) expect(response?.status).toBe(400) const body = await response?.json() expect(body.error).toBe( @@ -40,7 +45,7 @@ describe('rowWriteErrorResponse', () => { }) it('passes a classified validation failure through as 400', async () => { - const response = rowWriteErrorResponse( + const response = orchestrationErrorResponse( new OrchestrationError('validation', 'Value for column "email" must be unique') ) expect(response?.status).toBe(400) @@ -50,24 +55,26 @@ describe('rowWriteErrorResponse', () => { it('answers the code the failure carries, not one derived from its wording', () => { expect( - rowWriteErrorResponse(new OrchestrationError('not_found', 'Row not found'))?.status + orchestrationErrorResponse(new OrchestrationError('not_found', 'Row not found'))?.status ).toBe(404) // The phrase that used to force a 400 no longer decides anything. expect( - rowWriteErrorResponse(new OrchestrationError('conflict', 'Row 3: must be unique'))?.status + orchestrationErrorResponse(new OrchestrationError('conflict', 'Row 3: must be unique')) + ?.status ).toBe(409) }) it('unwraps a classified failure drizzle wrapped in a query error', () => { expect( - rowWriteErrorResponse(wrapLikeDrizzle(new OrchestrationError('validation', 'Row 3: bad'))) - ?.status + orchestrationErrorResponse( + wrapLikeDrizzle(new OrchestrationError('validation', 'Row 3: bad')) + )?.status ).toBe(400) }) it('returns null for unknown errors so callers keep their generic 500', () => { - expect(rowWriteErrorResponse(new Error('connection refused'))).toBeNull() - expect(rowWriteErrorResponse(wrapLikeDrizzle(new Error('deadlock detected')))).toBeNull() + expect(orchestrationErrorResponse(new Error('connection refused'))).toBeNull() + expect(orchestrationErrorResponse(wrapLikeDrizzle(new Error('deadlock detected')))).toBeNull() }) }) @@ -117,3 +124,60 @@ describe('tableFilterError', () => { expect(tableFilterError({ col_status: { $regex: 'x' } } as never, columns)?.status).toBe(400) }) }) + +describe('orchestrationOutcomeErrorResponse', () => { + /** + * Shaped like a driver fault surfacing verbatim — a statement plus its bound + * parameters — so the assertion proves none of it reaches the response body. + */ + const leakyMessage = + 'Failed query: delete from "user_table" where "user_table"."id" = $1 params: tbl-1' + + it('replaces an unclassified failure message with the fallback', async () => { + const response = orchestrationOutcomeErrorResponse( + { success: false, error: leakyMessage, errorCode: 'internal' }, + 'Failed to delete table' + ) + + expect(response.status).toBe(500) + const body = await response.json() + expect(body).toEqual({ error: 'Failed to delete table' }) + expect(JSON.stringify(body)).not.toContain('Failed query') + expect(JSON.stringify(body)).not.toContain('params:') + }) + + it('replaces an unclassified failure with no error code too', async () => { + const response = orchestrationOutcomeErrorResponse( + { error: leakyMessage }, + 'Failed to delete table' + ) + + expect(response.status).toBe(500) + expect(await response.json()).toEqual({ error: 'Failed to delete table' }) + }) + + it('keeps the message of a classified failure', async () => { + const response = orchestrationOutcomeErrorResponse( + { error: 'A table named "Orders" already exists in this workspace', errorCode: 'conflict' }, + 'Failed to rename table' + ) + + expect(response.status).toBe(409) + expect(await response.json()).toEqual({ + error: 'A table named "Orders" already exists in this workspace', + }) + }) + + it('carries the rejecting lock kind on a 423', async () => { + const response = orchestrationOutcomeErrorResponse( + { error: 'Table is locked against deletion', errorCode: 'locked', lock: 'delete' }, + 'Failed to delete table' + ) + + expect(response.status).toBe(423) + expect(await response.json()).toEqual({ + error: 'Table is locked against deletion', + lock: 'delete', + }) + }) +}) diff --git a/apps/sim/app/api/table/utils.ts b/apps/sim/app/api/table/utils.ts index 442594b4233..e049b1a3f68 100644 --- a/apps/sim/app/api/table/utils.ts +++ b/apps/sim/app/api/table/utils.ts @@ -8,7 +8,12 @@ import { updateTableColumnBodySchema, } from '@/lib/api/contracts/tables' import { isFeatureEnabled } from '@/lib/core/config/feature-flags' -import { asOrchestrationError, statusForOrchestrationError } from '@/lib/core/orchestration/types' +import { + asOrchestrationError, + messageForOrchestrationError, + type OrchestrationErrorCode, + statusForOrchestrationError, +} from '@/lib/core/orchestration/types' import type { MultipartError } from '@/lib/core/utils/multipart' import type { ColumnDefinition, Filter, TableDefinition, TablePredicate } from '@/lib/table' import { buildFilterClause, getTableById, TableQueryValidationError } from '@/lib/table' @@ -17,6 +22,7 @@ import { USER_TABLE_ROWS_SQL_NAME } from '@/lib/table/constants' import { TableLockedError } from '@/lib/table/mutation-locks' import { isTablePredicate } from '@/lib/table/query-builder/converters' import { validateStoragePredicate } from '@/lib/table/query-builder/validate' +import type { TableLockKind } from '@/lib/table/types' import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils' import { getWorkspaceOrganizationId } from '@/lib/workspaces/utils' @@ -133,10 +139,43 @@ export function orchestrationErrorResponse(error: unknown): NextResponse | null } /** - * {@link orchestrationErrorResponse} under the name the row-write routes call - * it by. Row writes have no classification rules of their own any more. + * The failure half of a `lib/table/orchestration` result. Every `perform*` + * function returns this shape, so one projection serves all of them. */ -export const rowWriteErrorResponse = orchestrationErrorResponse +export interface TableOrchestrationFailure { + error?: string + errorCode?: OrchestrationErrorCode + /** Which lock rejected the write. Set only when `errorCode` is `'locked'`. */ + lock?: TableLockKind +} + +/** + * Projects an orchestration failure RESULT onto its HTTP response, the + * counterpart of {@link orchestrationErrorResponse} for the functions that + * return a failure instead of throwing one. + * + * Routes go through this rather than reading `outcome.error` themselves, for + * two reasons the per-route spellings kept getting wrong: + * + * - An unclassified failure carries whatever text the fault happened to have — + * a driver's failed SQL and its bound parameters — so it renders `fallback` + * instead. Only a classified, caller-fixable failure keeps its own message. + * - A `'locked'` failure answers 423 with `{ error, lock }`. The lock kind is + * the only thing that tells a client which lock to clear, and it is computed + * by every `perform*` function already. + */ +export function orchestrationOutcomeErrorResponse( + outcome: TableOrchestrationFailure, + fallback: string +): NextResponse { + return NextResponse.json( + { + error: messageForOrchestrationError(outcome, fallback), + ...(outcome.lock ? { lock: outcome.lock } : {}), + }, + { status: statusForOrchestrationError(outcome.errorCode) } + ) +} /** * Next.js buffers the request body for the proxy and silently truncates it past this diff --git a/apps/sim/app/api/v1/tables/[tableId]/columns/route.ts b/apps/sim/app/api/v1/tables/[tableId]/columns/route.ts index 988c24825c9..aa3f74d8157 100644 --- a/apps/sim/app/api/v1/tables/[tableId]/columns/route.ts +++ b/apps/sim/app/api/v1/tables/[tableId]/columns/route.ts @@ -7,7 +7,6 @@ import { v1UpdateTableColumnContract, } from '@/lib/api/contracts/v1/tables' import { parseRequest } from '@/lib/api/server' -import { statusForOrchestrationError } from '@/lib/core/orchestration/types' import { generateRequestId } from '@/lib/core/utils/request' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' import { addTableColumn, deleteColumn } from '@/lib/table' @@ -18,6 +17,7 @@ import { checkAccess, normalizeColumn, orchestrationErrorResponse, + orchestrationOutcomeErrorResponse, tableLockErrorResponse, } from '@/app/api/table/utils' import { @@ -143,10 +143,7 @@ export const PATCH = withRouteHandler(async (request: NextRequest, context: Colu request, }) if (!outcome.success || !outcome.table) { - return NextResponse.json( - { error: outcome.error ?? 'Failed to update column' }, - { status: statusForOrchestrationError(outcome.errorCode) } - ) + return orchestrationOutcomeErrorResponse(outcome, 'Failed to update column') } // Live-collab: tell open viewers the change landed so they refetch. diff --git a/apps/sim/app/api/v1/tables/[tableId]/route.test.ts b/apps/sim/app/api/v1/tables/[tableId]/route.test.ts new file mode 100644 index 00000000000..7a71898fb9f --- /dev/null +++ b/apps/sim/app/api/v1/tables/[tableId]/route.test.ts @@ -0,0 +1,148 @@ +/** + * @vitest-environment node + * + * DELETE /api/v1/tables/[tableId] projects an orchestration failure onto the + * wire. Two properties of that projection are load-bearing and have regressed + * before, so they are pinned here rather than left to the helper's own unit + * test: an UNCLASSIFIED failure must never reach an API-key holder (its message + * is whatever the fault happened to carry — a driver's failed SQL and its bound + * parameters), and a `locked` failure must carry `lock` so the client knows + * which lock to clear. + */ +import { createMockRequest } from '@sim/testing' +import { NextResponse } from 'next/server' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { + mockCheckRateLimit, + mockCheckWorkspaceScope, + mockGetTableById, + mockGetUserEntityPermissions, + mockPerformDeleteTable, +} = vi.hoisted(() => ({ + mockCheckRateLimit: vi.fn(), + mockCheckWorkspaceScope: vi.fn(), + mockGetTableById: vi.fn(), + mockGetUserEntityPermissions: vi.fn(), + mockPerformDeleteTable: vi.fn(), +})) + +vi.mock('@/app/api/v1/middleware', () => ({ + checkRateLimit: mockCheckRateLimit, + checkWorkspaceScope: mockCheckWorkspaceScope, + createRateLimitResponse: () => NextResponse.json({ error: 'Rate limited' }, { status: 429 }), +})) + +vi.mock('@/lib/table', () => ({ + buildFilterClause: vi.fn(), + getTableById: mockGetTableById, + TableQueryValidationError: class TableQueryValidationError extends Error {}, +})) + +vi.mock('@/lib/workspaces/permissions/utils', () => ({ + getUserEntityPermissions: mockGetUserEntityPermissions, +})) + +vi.mock('@/lib/workspaces/utils', () => ({ + getWorkspaceOrganizationId: vi.fn().mockResolvedValue(null), +})) + +vi.mock('@/lib/table/orchestration', () => ({ + performDeleteTable: mockPerformDeleteTable, +})) + +import { DELETE } from '@/app/api/v1/tables/[tableId]/route' + +const TABLE_ID = '22222222-2222-4222-8222-222222222222' +const WORKSPACE_ID = '11111111-1111-4111-8111-111111111111' + +/** + * Stands in for a driver fault surfacing verbatim. Shaped like a real one — + * a statement plus its bound parameters — so the assertions can prove none of + * it reaches the response body. + */ +const LEAKY_INTERNAL_MESSAGE = + 'Failed query: delete from "user_table" where "user_table"."id" = $1 params: 22222222-2222-4222-8222-222222222222' + +function makeRequest() { + return createMockRequest( + 'DELETE', + undefined, + {}, + `http://localhost:3000/api/v1/tables/${TABLE_ID}?workspaceId=${WORKSPACE_ID}` + ) +} + +function makeContext() { + return { params: Promise.resolve({ tableId: TABLE_ID }) } +} + +describe('DELETE /api/v1/tables/[tableId] — orchestration failure projection', () => { + beforeEach(() => { + vi.clearAllMocks() + mockCheckRateLimit.mockResolvedValue({ allowed: true, userId: 'user-1' }) + mockCheckWorkspaceScope.mockResolvedValue(null) + mockGetTableById.mockResolvedValue({ + id: TABLE_ID, + name: 'Table', + workspaceId: WORKSPACE_ID, + }) + mockGetUserEntityPermissions.mockResolvedValue('admin') + }) + + it('renders an unclassified internal failure as a fixed generic message', async () => { + mockPerformDeleteTable.mockResolvedValue({ + success: false, + error: LEAKY_INTERNAL_MESSAGE, + errorCode: 'internal', + }) + + const response = await DELETE(makeRequest(), makeContext()) + const body = await response.json() + + expect(response.status).toBe(500) + expect(body).toEqual({ error: 'Failed to delete table' }) + expect(JSON.stringify(body)).not.toContain('Failed query') + expect(JSON.stringify(body)).not.toContain('params:') + expect(JSON.stringify(body)).not.toContain('$1') + }) + + it('renders an unclassified failure with no error code as the same generic message', async () => { + mockPerformDeleteTable.mockResolvedValue({ success: false, error: LEAKY_INTERNAL_MESSAGE }) + + const response = await DELETE(makeRequest(), makeContext()) + + expect(response.status).toBe(500) + expect(await response.json()).toEqual({ error: 'Failed to delete table' }) + }) + + it('keeps the specific message of a classified failure', async () => { + mockPerformDeleteTable.mockResolvedValue({ + success: false, + error: 'Table not found', + errorCode: 'not_found', + }) + + const response = await DELETE(makeRequest(), makeContext()) + + expect(response.status).toBe(404) + expect(await response.json()).toEqual({ error: 'Table not found' }) + }) + + it('returns 423 with the rejecting lock kind', async () => { + mockPerformDeleteTable.mockResolvedValue({ + success: false, + error: 'Table is locked against deletion', + errorCode: 'locked', + lock: 'delete', + }) + + const response = await DELETE(makeRequest(), makeContext()) + + expect(response.status).toBe(423) + expect(await response.json()).toEqual({ + error: 'Table is locked against deletion', + lock: 'delete', + }) + }) +}) diff --git a/apps/sim/app/api/v1/tables/[tableId]/route.ts b/apps/sim/app/api/v1/tables/[tableId]/route.ts index 149bc674651..caaf87d8be7 100644 --- a/apps/sim/app/api/v1/tables/[tableId]/route.ts +++ b/apps/sim/app/api/v1/tables/[tableId]/route.ts @@ -2,7 +2,6 @@ import { createLogger } from '@sim/logger' import { type NextRequest, NextResponse } from 'next/server' import { v1DeleteTableContract, v1GetTableContract } from '@/lib/api/contracts/v1/tables' import { parseRequest } from '@/lib/api/server' -import { statusForOrchestrationError } from '@/lib/core/orchestration/types' import { generateRequestId } from '@/lib/core/utils/request' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' import type { TableSchema } from '@/lib/table' @@ -11,6 +10,7 @@ import { accessError, checkAccess, normalizeColumn, + orchestrationOutcomeErrorResponse, tableLockErrorResponse, } from '@/app/api/table/utils' import { @@ -142,10 +142,7 @@ export const DELETE = withRouteHandler(async (request: NextRequest, context: Tab const outcome = await performDeleteTable({ table: result.table, userId, requestId, request }) if (!outcome.success) { - return NextResponse.json( - { error: outcome.error ?? 'Failed to delete table' }, - { status: statusForOrchestrationError(outcome.errorCode) } - ) + return orchestrationOutcomeErrorResponse(outcome, 'Failed to delete table') } return NextResponse.json({ diff --git a/apps/sim/app/api/v1/tables/[tableId]/rows/[rowId]/route.ts b/apps/sim/app/api/v1/tables/[tableId]/rows/[rowId]/route.ts index 38c992235fd..034fb8cd8f8 100644 --- a/apps/sim/app/api/v1/tables/[tableId]/rows/[rowId]/route.ts +++ b/apps/sim/app/api/v1/tables/[tableId]/rows/[rowId]/route.ts @@ -9,7 +9,6 @@ import { v1UpdateTableRowContract, } from '@/lib/api/contracts/v1/tables' import { parseRequest } from '@/lib/api/server' -import { statusForOrchestrationError } from '@/lib/core/orchestration/types' import { generateRequestId } from '@/lib/core/utils/request' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' import type { RowData, TableSchema } from '@/lib/table' @@ -23,6 +22,7 @@ import { accessError, checkAccess, orchestrationErrorResponse, + orchestrationOutcomeErrorResponse, tableLockErrorResponse, } from '@/app/api/table/utils' import { @@ -240,10 +240,7 @@ export const DELETE = withRouteHandler(async (request: NextRequest, context: Row const outcome = await performDeleteTableRow({ table: result.table, rowId, requestId }) if (!outcome.success) { - return NextResponse.json( - { error: outcome.error ?? 'Failed to delete row' }, - { status: statusForOrchestrationError(outcome.errorCode) } - ) + return orchestrationOutcomeErrorResponse(outcome, 'Failed to delete row') } // Live-collab: tell open viewers the change landed so they refetch. diff --git a/apps/sim/app/api/v1/tables/[tableId]/rows/route.ts b/apps/sim/app/api/v1/tables/[tableId]/rows/route.ts index 624b2f5c80d..9297cc68181 100644 --- a/apps/sim/app/api/v1/tables/[tableId]/rows/route.ts +++ b/apps/sim/app/api/v1/tables/[tableId]/rows/route.ts @@ -33,7 +33,7 @@ import { signalTableRowsChanged } from '@/lib/table/events' import { createExactEmptyTableRowSecretProvenance } from '@/lib/table/rows/secret-provenance' import { queryRows } from '@/lib/table/rows/service' import { resolveFilterSelectValues } from '@/lib/table/select-values' -import { accessError, checkAccess, rowWriteErrorResponse } from '@/app/api/table/utils' +import { accessError, checkAccess, orchestrationErrorResponse } from '@/app/api/table/utils' import { checkRateLimit, checkWorkspaceScope, @@ -109,7 +109,7 @@ async function handleBatchInsert( }, }) } catch (error) { - const response = rowWriteErrorResponse(error) + const response = orchestrationErrorResponse(error) if (response) return response logger.error(`[${requestId}] Error batch inserting rows:`, error) @@ -306,7 +306,7 @@ export const POST = withRouteHandler( const validationResponse = v1ValidationErrorResponseFromError(error) if (validationResponse) return validationResponse - const response = rowWriteErrorResponse(error) + const response = orchestrationErrorResponse(error) if (response) return response logger.error(`[${requestId}] Error inserting row:`, error) @@ -402,7 +402,7 @@ export const PUT = withRouteHandler(async (request: NextRequest, context: TableR return NextResponse.json({ error: error.message }, { status: 400 }) } - const response = rowWriteErrorResponse(error) + const response = orchestrationErrorResponse(error) if (response) return response logger.error(`[${requestId}] Error updating rows by filter:`, error) @@ -497,7 +497,7 @@ export const DELETE = withRouteHandler( return NextResponse.json({ error: error.message }, { status: 400 }) } - const response = rowWriteErrorResponse(error) + const response = orchestrationErrorResponse(error) if (response) return response logger.error(`[${requestId}] Error deleting rows:`, error) diff --git a/apps/sim/app/api/v1/tables/route.test.ts b/apps/sim/app/api/v1/tables/route.test.ts new file mode 100644 index 00000000000..b20d5664ad7 --- /dev/null +++ b/apps/sim/app/api/v1/tables/route.test.ts @@ -0,0 +1,118 @@ +/** + * @vitest-environment node + * + * POST /api/v1/tables maps the create-table service's classified failures onto + * status codes. A duplicate name is a `conflict` and answers 409 — the same + * status every other v1 duplicate-name surface uses (knowledge, files, workflow + * import) — while bad input stays 400 and a quota ceiling stays 403. + */ +import { createMockRequest } from '@sim/testing' +import { NextResponse } from 'next/server' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { mockCheckRateLimit, mockValidateWorkspaceAccess, mockCreateTable, mockGetLimits } = + vi.hoisted(() => ({ + mockCheckRateLimit: vi.fn(), + mockValidateWorkspaceAccess: vi.fn(), + mockCreateTable: vi.fn(), + mockGetLimits: vi.fn(), + })) + +vi.mock('@/app/api/v1/middleware', () => ({ + checkRateLimit: mockCheckRateLimit, + createRateLimitResponse: () => NextResponse.json({ error: 'Rate limited' }, { status: 429 }), + validateWorkspaceAccess: mockValidateWorkspaceAccess, + v1ValidationErrorResponse: (error: { issues: unknown[] }) => + NextResponse.json({ error: 'Validation error', details: error.issues }, { status: 400 }), + v1ValidationErrorResponseFromError: () => null, +})) + +vi.mock('@/lib/table', () => ({ + buildFilterClause: vi.fn(), + createTable: mockCreateTable, + getTableById: vi.fn(), + getWorkspaceTableLimits: mockGetLimits, + listTables: vi.fn(), + TableQueryValidationError: class TableQueryValidationError extends Error {}, +})) + +vi.mock('@/lib/workspaces/permissions/utils', () => ({ + getUserEntityPermissions: vi.fn(), +})) + +vi.mock('@/lib/workspaces/utils', () => ({ + getWorkspaceOrganizationId: vi.fn().mockResolvedValue(null), +})) + +import { OrchestrationError } from '@/lib/core/orchestration/types' +import { POST } from '@/app/api/v1/tables/route' + +const WORKSPACE_ID = '11111111-1111-4111-8111-111111111111' + +function makeRequest() { + return createMockRequest( + 'POST', + { + workspaceId: WORKSPACE_ID, + name: 'Orders', + schema: { columns: [{ name: 'amount', type: 'number' }] }, + }, + {}, + 'http://localhost:3000/api/v1/tables' + ) +} + +describe('POST /api/v1/tables — create failure statuses', () => { + beforeEach(() => { + vi.clearAllMocks() + mockCheckRateLimit.mockResolvedValue({ allowed: true, userId: 'user-1' }) + mockValidateWorkspaceAccess.mockResolvedValue(null) + mockGetLimits.mockResolvedValue({ maxTables: 100 }) + }) + + it('answers 409 for a duplicate table name', async () => { + mockCreateTable.mockRejectedValue( + new OrchestrationError('conflict', 'A table named "Orders" already exists in this workspace') + ) + + const response = await POST(makeRequest()) + + expect(response.status).toBe(409) + expect(await response.json()).toEqual({ + error: 'A table named "Orders" already exists in this workspace', + }) + }) + + it('answers 400 for invalid input', async () => { + mockCreateTable.mockRejectedValue( + new OrchestrationError('validation', 'Invalid table name: name is reserved') + ) + + const response = await POST(makeRequest()) + + expect(response.status).toBe(400) + }) + + it('answers 403 for a workspace at its table limit', async () => { + mockCreateTable.mockRejectedValue( + new OrchestrationError('forbidden', 'Workspace has reached maximum table limit (100)') + ) + + const response = await POST(makeRequest()) + + expect(response.status).toBe(403) + }) + + it('answers a fixed generic 500 for an unclassified failure', async () => { + mockCreateTable.mockRejectedValue( + new Error('Failed query: insert into "user_table" ... params: Orders') + ) + + const response = await POST(makeRequest()) + const body = await response.json() + + expect(response.status).toBe(500) + expect(body).toEqual({ error: 'Failed to create table' }) + expect(JSON.stringify(body)).not.toContain('Failed query') + }) +}) diff --git a/apps/sim/app/api/v2/tables/utils.ts b/apps/sim/app/api/v2/tables/utils.ts index 2a686a18ced..dbf26a6e19d 100644 --- a/apps/sim/app/api/v2/tables/utils.ts +++ b/apps/sim/app/api/v2/tables/utils.ts @@ -17,8 +17,8 @@ import { getUserEmailsByIds, requireResolvedUserEmail } from '@/lib/users/querie import { CSV_IMPORT_PROXY_BODY_CAP_BYTES, normalizeColumn, + orchestrationErrorResponse, rootErrorMessage, - rowWriteErrorResponse, } from '@/app/api/table/utils' import { v2Error, v2ErrorForOrchestration } from '@/app/api/v2/lib/response' @@ -257,12 +257,12 @@ export function v2TableOrchestrationError( /** * Maps a known user-facing row-write failure (schema/size/unique/limit) to a v2 - * `BAD_REQUEST`, reusing v1's {@link rowWriteErrorResponse} classifier as the + * `BAD_REQUEST`, reusing v1's {@link orchestrationErrorResponse} classifier as the * single source of truth for which messages are safe to surface. Returns `null` * for unrecognized errors so the caller logs and returns a generic 500. */ export function v2RowWriteError(error: unknown): NextResponse | null { - if (!rowWriteErrorResponse(error)) return null + if (!orchestrationErrorResponse(error)) return null return v2Error('BAD_REQUEST', rootErrorMessage(error)) }