Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 15 additions & 2 deletions apps/sim/app/api/table/[tableId]/columns/route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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'
Expand Down
7 changes: 2 additions & 5 deletions apps/sim/app/api/table/[tableId]/columns/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -18,6 +17,7 @@ import {
accessError,
checkAccess,
normalizeColumn,
orchestrationOutcomeErrorResponse,
rootErrorMessage,
tableLockErrorResponse,
} from '@/app/api/table/utils'
Expand Down Expand Up @@ -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.
Expand Down
27 changes: 9 additions & 18 deletions apps/sim/app/api/table/[tableId]/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -24,6 +23,7 @@ import {
accessError,
checkAccess,
normalizeColumn,
orchestrationOutcomeErrorResponse,
tableLockErrorResponse,
} from '@/app/api/table/utils'

Expand Down Expand Up @@ -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')
}
}

Expand All @@ -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')
}
}

Expand All @@ -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'
)
}
}
Expand Down Expand Up @@ -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({
Expand Down
10 changes: 3 additions & 7 deletions apps/sim/app/api/table/[tableId]/rows/[rowId]/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -27,7 +26,7 @@ import {
accessError,
checkAccess,
orchestrationErrorResponse,
rowWriteErrorResponse,
orchestrationOutcomeErrorResponse,
tableLockErrorResponse,
} from '@/app/api/table/utils'

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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.
Expand Down
12 changes: 6 additions & 6 deletions apps/sim/app/api/table/[tableId]/rows/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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')

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions apps/sim/app/api/table/[tableId]/rows/upsert/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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')

Expand Down Expand Up @@ -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)
Expand Down
84 changes: 74 additions & 10 deletions apps/sim/app/api/table/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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(
Expand All @@ -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)
Expand All @@ -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()
})
})

Expand Down Expand Up @@ -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',
})
})
})
Loading
Loading