opensource/sim
3
0
Fork 0
mirror of https://github.com/simstudioai/sim.git synced 2026-09-24 15:45:35 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix(api): collapse the internal error envelope and restore requestId (#6584)

Browse Source 
* fix(api): collapse the internal error envelope and restore requestId

The builders shipped two internal error envelopes: internalOrchestrationErrorPolicy
emitted { success: false, error } while internalPlainOrchestrationErrorPolicy
emitted { error }. That split approximated pre-builder behavior, where the shape
depended on which branch failed - guard clauses returned { error } and a route's
terminal try/catch returned { success: false, error }. A per-route policy cannot
express a per-branch rule, so the two disagreed on the same status across families.

Collapse to the bare { error } shape. It is what messageFromErrorBody reads on the
client and what most migrated routes already emitted. requestJson throws
ApiClientError for any non-2xx, so no typed client ever observes the discriminator.
success: true on success bodies is a separate contract and is untouched.

Also restore requestId to internal error bodies. withRouteHandler stamps it on the
bodies it generates, but the builder overrides dropped it, leaving it only on the
x-request-id header - invisible when a user pastes an error out of the UI. It is now
applied at the createJsonErrorResponse chokepoint and in both wrapper overrides, and
is omitted when there is no active request scope.

* fix(api): stamp requestId on internal auth and parse failures
This commit is contained in:
Waleed
2026-08-11 19:32:59 -07:00
committed by GitHub
parent 9f3c20290b
commit 1874ceccda
32 changed files with 351 additions and 94 deletions
Download Patch File Download Diff File Expand all files Collapse all files
apps/sim/app/api/audit-logs/route.ts
+2 -2
View File
@@ -1,7 +1,7 @@
import { listAuditLogsContract } from '@/lib/api/contracts/audit-logs'
import {
defineInternalJsonRoute,
internalPlainOrchestrationErrorPolicy,
internalOrchestrationErrorPolicy,
internalRateLimits,
internalSessionAuth,
} from '@/lib/api/server/routes'
@@ -18,7 +18,7 @@ export const GET = defineInternalJsonRoute({
rateLimit: internalRateLimits.none({
reason: 'Existing authenticated audit-log settings read has no request-rate policy',
}),
errorPolicy: internalPlainOrchestrationErrorPolicy,
errorPolicy: internalOrchestrationErrorPolicy,
mapInput: ({ query }) => ({
organizationId: query.organizationId,
includeDeparted: query.includeDeparted,
apps/sim/app/api/table/[tableId]/exports/route.ts
+2 -2
View File
@@ -1,7 +1,7 @@
import { createTableExportResourceContract } from '@/lib/api/contracts/table-transfers'
import {
defineInternalJsonRoute,
internalPlainOrchestrationErrorPolicy,
internalOrchestrationErrorPolicy,
internalRateLimits,
} from '@/lib/api/server/routes'
import { internalTableSessionOrExecutorAuth } from '@/lib/table/api'
@@ -16,7 +16,7 @@ export const POST = defineInternalJsonRoute({
rateLimit: internalRateLimits.none({
reason: 'Existing authenticated table export creation has no request-rate policy',
}),
errorPolicy: internalPlainOrchestrationErrorPolicy,
errorPolicy: internalOrchestrationErrorPolicy,
mapInput: ({ params, body }) => ({
tableId: params.tableId,
workspaceId: body.workspaceId,
apps/sim/app/api/table/[tableId]/groups/route.test.ts
+1 -1
View File
@@ -36,7 +36,7 @@ vi.mock('@/lib/api/server/routes', () => ({
},
extendInternalErrorPolicy: vi.fn(() => ({ kind: 'table' })),
internalErrorResponse: vi.fn(),
internalPlainOrchestrationErrorPolicy: { kind: 'plain' },
internalOrchestrationErrorPolicy: { kind: 'plain' },
internalRateLimits: {
none: ({ reason }: { reason: string }) => ({ kind: 'none', reason }),
},
apps/sim/app/api/table/[tableId]/groups/route.ts
+2 -2
View File
@@ -7,7 +7,7 @@ import {
defineInternalJsonRoute,
extendInternalErrorPolicy,
internalErrorResponse,
internalPlainOrchestrationErrorPolicy,
internalOrchestrationErrorPolicy,
internalRateLimits,
} from '@/lib/api/server/routes'
import { internalTableSessionOrExecutorAuth } from '@/lib/table/api'
@@ -21,7 +21,7 @@ import { TableLockedError } from '@/lib/table/mutation-locks'
import type { TableDefinition } from '@/lib/table/types'
import { normalizeColumn } from '@/app/api/table/utils'
const errorPolicy = extendInternalErrorPolicy(internalPlainOrchestrationErrorPolicy, (error) =>
const errorPolicy = extendInternalErrorPolicy(internalOrchestrationErrorPolicy, (error) =>
error instanceof TableLockedError
? internalErrorResponse(423, { error: error.message, lock: error.lock })
: null
apps/sim/app/api/table/exports/[exportId]/download/route.ts
+2 -2
View File
@@ -1,7 +1,7 @@
import { downloadTableExportResourceContract } from '@/lib/api/contracts/table-transfers'
import {
defineInternalJsonRoute,
internalPlainOrchestrationErrorPolicy,
internalOrchestrationErrorPolicy,
internalRateLimits,
} from '@/lib/api/server/routes'
import { internalTableSessionOrExecutorAuth } from '@/lib/table/api'
@@ -15,7 +15,7 @@ export const GET = defineInternalJsonRoute({
rateLimit: internalRateLimits.none({
reason: 'Existing authenticated table export download signing has no request-rate policy',
}),
errorPolicy: internalPlainOrchestrationErrorPolicy,
errorPolicy: internalOrchestrationErrorPolicy,
mapInput: ({ params, query }) => ({
exportId: params.exportId,
workspaceId: query.workspaceId,
apps/sim/app/api/table/exports/[exportId]/route.ts
+3 -3
View File
@@ -4,7 +4,7 @@ import {
} from '@/lib/api/contracts/table-transfers'
import {
defineInternalJsonRoute,
internalPlainOrchestrationErrorPolicy,
internalOrchestrationErrorPolicy,
internalRateLimits,
} from '@/lib/api/server/routes'
import { internalTableSessionOrExecutorAuth } from '@/lib/table/api'
@@ -21,7 +21,7 @@ export const GET = defineInternalJsonRoute({
auth: internalTableSessionOrExecutorAuth,
operation: tableOperations.readExport,
rateLimit,
errorPolicy: internalPlainOrchestrationErrorPolicy,
errorPolicy: internalOrchestrationErrorPolicy,
mapInput: ({ params, query }) => ({
exportId: params.exportId,
workspaceId: query.workspaceId,
@@ -35,7 +35,7 @@ export const DELETE = defineInternalJsonRoute({
auth: internalTableSessionOrExecutorAuth,
operation: tableOperations.cancelExport,
rateLimit,
errorPolicy: internalPlainOrchestrationErrorPolicy,
errorPolicy: internalOrchestrationErrorPolicy,
mapInput: ({ params, query }) => ({
exportId: params.exportId,
workspaceId: query.workspaceId,
apps/sim/app/api/table/imports/[importId]/complete/route.ts
+2 -2
View File
@@ -1,7 +1,7 @@
import { completeTableImportResourceContract } from '@/lib/api/contracts/table-transfers'
import {
defineInternalJsonRoute,
internalPlainOrchestrationErrorPolicy,
internalOrchestrationErrorPolicy,
internalRateLimits,
} from '@/lib/api/server/routes'
import { internalTableSessionOrExecutorAuth } from '@/lib/table/api'
@@ -16,7 +16,7 @@ export const POST = defineInternalJsonRoute({
rateLimit: internalRateLimits.none({
reason: 'Existing authenticated table import completion has no request-rate policy',
}),
errorPolicy: internalPlainOrchestrationErrorPolicy,
errorPolicy: internalOrchestrationErrorPolicy,
mapInput: ({ params, query, headers }) => ({
importId: params.importId,
workspaceId: query.workspaceId,
apps/sim/app/api/table/imports/[importId]/parts/route.ts
+2 -2
View File
@@ -1,7 +1,7 @@
import { createTableImportPartUrlsContract } from '@/lib/api/contracts/table-transfers'
import {
defineInternalJsonRoute,
internalPlainOrchestrationErrorPolicy,
internalOrchestrationErrorPolicy,
internalRateLimits,
} from '@/lib/api/server/routes'
import { internalTableSessionOrExecutorAuth } from '@/lib/table/api'
@@ -15,7 +15,7 @@ export const POST = defineInternalJsonRoute({
rateLimit: internalRateLimits.none({
reason: 'Existing authenticated table import part signing has no request-rate policy',
}),
errorPolicy: internalPlainOrchestrationErrorPolicy,
errorPolicy: internalOrchestrationErrorPolicy,
mapInput: ({ params, query, headers, body }) => ({
importId: params.importId,
workspaceId: query.workspaceId,
apps/sim/app/api/table/imports/[importId]/route.ts
+3 -3
View File
@@ -4,7 +4,7 @@ import {
} from '@/lib/api/contracts/table-transfers'
import {
defineInternalJsonRoute,
internalPlainOrchestrationErrorPolicy,
internalOrchestrationErrorPolicy,
internalRateLimits,
} from '@/lib/api/server/routes'
import { internalTableSessionOrExecutorAuth } from '@/lib/table/api'
@@ -21,7 +21,7 @@ export const GET = defineInternalJsonRoute({
auth: internalTableSessionOrExecutorAuth,
operation: tableOperations.readImport,
rateLimit,
errorPolicy: internalPlainOrchestrationErrorPolicy,
errorPolicy: internalOrchestrationErrorPolicy,
mapInput: ({ params, query }) => ({
importId: params.importId,
workspaceId: query.workspaceId,
@@ -35,7 +35,7 @@ export const DELETE = defineInternalJsonRoute({
auth: internalTableSessionOrExecutorAuth,
operation: tableOperations.cancelImport,
rateLimit,
errorPolicy: internalPlainOrchestrationErrorPolicy,
errorPolicy: internalOrchestrationErrorPolicy,
mapInput: ({ params, query, headers }) => ({
importId: params.importId,
workspaceId: query.workspaceId,
apps/sim/app/api/table/imports/route.ts
+2 -2
View File
@@ -1,7 +1,7 @@
import { createTableImportResourceContract } from '@/lib/api/contracts/table-transfers'
import {
defineInternalJsonRoute,
internalPlainOrchestrationErrorPolicy,
internalOrchestrationErrorPolicy,
internalRateLimits,
} from '@/lib/api/server/routes'
import { internalTableSessionOrExecutorAuth } from '@/lib/table/api'
@@ -16,7 +16,7 @@ export const POST = defineInternalJsonRoute({
rateLimit: internalRateLimits.none({
reason: 'Existing authenticated table import creation has no request-rate policy',
}),
errorPolicy: internalPlainOrchestrationErrorPolicy,
errorPolicy: internalOrchestrationErrorPolicy,
mapInput: ({ body }) => ({ body }),
useCase: createTableImportUseCase,
present: ({ import: created }) => ({ data: toV2CreateTableImport(created) }),
apps/sim/app/api/table/table-transfer-routes.test.ts
+1 -1
View File
@@ -35,7 +35,7 @@ vi.mock('@/lib/api/server/routes', () => ({
mocks.definitions.push(definition)
return vi.fn()
},
internalPlainOrchestrationErrorPolicy: { kind: 'plain' },
internalOrchestrationErrorPolicy: { kind: 'plain' },
internalRateLimits: {
none: ({ reason }: { reason: string }) => ({ kind: 'none', reason }),
},
apps/sim/app/api/workflows/[id]/deployed/route.ts
+2 -2
View File
@@ -5,7 +5,7 @@ import {
} from '@/lib/api/contracts/deployments'
import {
defineInternalJsonRoute,
internalPlainOrchestrationErrorPolicy,
internalOrchestrationErrorPolicy,
internalRateLimits,
} from '@/lib/api/server/routes'
import { internalWorkflowSessionOrExecutorAuth } from '@/lib/workflows/api'
@@ -23,7 +23,7 @@ export const GET = defineInternalJsonRoute({
rateLimit: internalRateLimits.none({
reason: 'Preserve existing internal workflow read behavior',
}),
errorPolicy: internalPlainOrchestrationErrorPolicy,
errorPolicy: internalOrchestrationErrorPolicy,
mapInput: ({ params }) => ({ workflowId: params.id, state: 'deployed' as const }),
useCase: readWorkflowDefinition,
present: ({ state }) => ({
apps/sim/app/api/workflows/[id]/route.test.ts
+1 -1
View File
@@ -20,7 +20,7 @@ vi.mock('@/lib/api/server', () => ({ parseRequest: mocks.parseRequest }))
vi.mock('@/lib/api/server/routes', () => ({
defineInternalJsonRoute: mocks.defineRoute,
InternalUnauthenticatedError: class InternalUnauthenticatedError extends Error {},
internalPlainOrchestrationErrorPolicy: { kind: 'plain-orchestration' },
internalOrchestrationErrorPolicy: { kind: 'plain-orchestration' },
internalRateLimits: { none: vi.fn(() => ({ kind: 'none' })) },
}))
apps/sim/app/api/workflows/[id]/route.ts
+3 -3
View File
@@ -12,7 +12,7 @@ import { parseRequest } from '@/lib/api/server'
import {
defineInternalJsonRoute,
InternalUnauthenticatedError,
internalPlainOrchestrationErrorPolicy,
internalOrchestrationErrorPolicy,
internalRateLimits,
} from '@/lib/api/server/routes'
import { asOrchestrationError, statusForOrchestrationError } from '@/lib/core/orchestration/types'
@@ -37,7 +37,7 @@ export const GET = defineInternalJsonRoute({
auth: internalWorkflowReadAuth,
operation: readWorkflowDefinition.operation,
rateLimit: workflowInternalRateLimit,
errorPolicy: internalPlainOrchestrationErrorPolicy,
errorPolicy: internalOrchestrationErrorPolicy,
mapInput: ({ params }) => ({ workflowId: params.id, state: 'draft' as const }),
useCase: readWorkflowDefinition,
present: ({ workflow: workflowData, state }) => {
@@ -82,7 +82,7 @@ export const DELETE = defineInternalJsonRoute({
auth: internalWorkflowSessionOrExecutorAuth,
operation: deleteWorkflow.operation,
rateLimit: workflowInternalRateLimit,
errorPolicy: internalPlainOrchestrationErrorPolicy,
errorPolicy: internalOrchestrationErrorPolicy,
mapInput: ({ params }) => ({ workflowId: params.id }),
useCase: deleteWorkflow,
present: () => ({ success: true as const }),
apps/sim/app/api/workspaces/[id]/files/[fileId]/content/route.test.ts
-1
View File
@@ -143,7 +143,6 @@ describe('PUT /api/workspaces/[id]/files/[fileId]/content', () => {
expect(response.status).toBe(402)
await expect(response.json()).resolves.toEqual({
success: false,
error: 'Storage limit exceeded',
})
})
apps/sim/app/api/workspaces/[id]/files/[fileId]/csv-preview/route.ts
+1 -1
View File
@@ -14,7 +14,7 @@ export const GET = defineInternalJsonRoute({
auth: internalSessionOrExecutorAuth,
operation: csvPreviewWorkspaceFile.operation,
rateLimit: internalRateLimits.none({ reason: 'Preserve existing internal CSV preview behavior' }),
errorPolicy: internalFileErrorPolicies.plain,
errorPolicy: internalFileErrorPolicies.default,
mapInput: ({ params, query }) => ({
fileId: params.fileId,
assertedWorkspaceId: params.id,
apps/sim/app/api/workspaces/[id]/files/[fileId]/route.test.ts
-2
View File
@@ -126,7 +126,6 @@ describe('PATCH /api/workspaces/[id]/files/[fileId]', () => {
expect(response.status).toBe(403)
expect(await response.json()).toEqual({
success: false,
error: 'Insufficient workspace permissions',
})
expect(mocks.captureServerEvent).not.toHaveBeenCalled()
@@ -139,7 +138,6 @@ describe('PATCH /api/workspaces/[id]/files/[fileId]', () => {
expect(response.status).toBe(500)
expect(await response.json()).toEqual({
success: false,
error: 'Internal server error',
})
})
apps/sim/app/api/workspaces/[id]/files/[fileId]/share/route.test.ts
+2 -2
View File
@@ -96,7 +96,7 @@ describe('/api/workspaces/[id]/files/[fileId]/share', () => {
const response = await GET(getRequest(), context)
expect(response.status).toBe(403)
expect(await response.json()).toEqual({ success: false, error: 'Access denied' })
expect(await response.json()).toEqual({ error: 'Access denied' })
})
it('renders resource absence as 404', async () => {
@@ -142,7 +142,7 @@ describe('/api/workspaces/[id]/files/[fileId]/share', () => {
const response = await PUT(putRequest({ isActive: true }), context)
expect(response.status).toBe(400)
expect(await response.json()).toEqual({ success: false, error: 'Password is required' })
expect(await response.json()).toEqual({ error: 'Password is required' })
})
it('preserves the internal caller-supplied token field for compatibility', async () => {
apps/sim/app/api/workspaces/[id]/files/folders/[folderId]/route.test.ts
-2
View File
@@ -112,7 +112,6 @@ describe('/api/workspaces/[id]/files/folders/[folderId]', () => {
expect(response.status).toBe(409)
await expect(response.json()).resolves.toEqual({
success: false,
error: 'A folder named "Reports" already exists in this location',
})
expect(mocks.captureServerEvent).not.toHaveBeenCalled()
@@ -140,7 +139,6 @@ describe('/api/workspaces/[id]/files/folders/[folderId]', () => {
expect(response.status).toBe(404)
expect(await response.json()).toEqual({
success: false,
error: `Workspace file items not found (folders: ${FOLDER_ID})`,
})
expect(mocks.captureServerEvent).not.toHaveBeenCalled()
apps/sim/app/api/workspaces/[id]/files/folders/route.test.ts
-1
View File
@@ -113,7 +113,6 @@ describe('/api/workspaces/[id]/files/folders', () => {
expect(response.status).toBe(409)
await expect(response.json()).resolves.toEqual({
success: false,
error: 'A folder named "Reports" already exists in this location',
})
expect(mocks.captureServerEvent).not.toHaveBeenCalled()
apps/sim/app/api/workspaces/[id]/files/move/route.test.ts
-1
View File
@@ -83,7 +83,6 @@ describe('/api/workspaces/[id]/files/move', () => {
expect(response.status).toBe(409)
await expect(response.json()).resolves.toEqual({
success: false,
error: 'A file named "report.csv" already exists in the destination folder',
})
expect(mocks.captureServerEvent).not.toHaveBeenCalled()
apps/sim/app/api/workspaces/[id]/files/route.test.ts
+1 -2
View File
@@ -146,13 +146,12 @@ describe('/api/workspaces/[id]/files', () => {
mocks.createFile.mockRejectedValueOnce(new OrchestrationError('conflict', 'Name exists'))
const conflict = await POST(createRequest({ name: 'notes.md' }), context)
expect(conflict.status).toBe(409)
expect(await conflict.json()).toEqual({ success: false, error: 'Name exists' })
expect(await conflict.json()).toEqual({ error: 'Name exists' })
mocks.createFile.mockRejectedValueOnce(new Error('database details'))
const unexpected = await POST(createRequest({ name: 'notes.md' }), context)
expect(unexpected.status).toBe(500)
expect(await unexpected.json()).toEqual({
success: false,
error: 'Internal server error',
})
})
apps/sim/lib/api/server/routes/index.ts
-1
View File
@@ -9,7 +9,6 @@ export {
internalErrorResponse,
internalJsonPresenters,
internalOrchestrationErrorPolicy,
internalPlainOrchestrationErrorPolicy,
internalRateLimits,
internalSessionAuth,
} from '@/lib/api/server/routes/internal-json-route'
apps/sim/lib/api/server/routes/internal-binary-route.ts
+9 -7
View File
@@ -5,8 +5,10 @@ import { requireBinaryRouteDefinition } from '@/lib/api/server/routes/definition
import {
type InternalErrorPolicy,
InternalUnauthenticatedError,
internalErrorResponse,
type internalSessionAuth,
} from '@/lib/api/server/routes/internal-json-route'
import { responseWithRequestId, withRequestId } from '@/lib/api/server/routes/request-id'
import type {
BinaryApiRouteContract,
BinaryResponseDescriptor,
@@ -81,14 +83,14 @@ export function defineInternalBinaryRoute<
principal = await options.auth.authenticate()
} catch (error) {
if (error instanceof InternalUnauthenticatedError) {
return NextResponse.json({ error: error.message }, { status: 401 })
return createJsonErrorResponse(internalErrorResponse(401, { error: error.message }))
}
throw error
}
await options.rateLimit.enforce(request, principal)
const parsed = await parseRequest(options.contract, request, context ?? {})
if (!parsed.success) return parsed.response
if (!parsed.success) return responseWithRequestId(parsed.response)
try {
const input = options.mapInput(parsed.data)
@@ -111,10 +113,10 @@ export function defineInternalBinaryRoute<
}
},
{
typedErrorResponse: ({ error, status }) =>
NextResponse.json({ error: error.message }, { status }),
unhandledErrorResponse: () =>
NextResponse.json({ error: 'Internal server error' }, { status: 500 }),
typedErrorResponse: ({ error, status, requestId }) =>
NextResponse.json({ error: error.message, requestId }, { status }),
unhandledErrorResponse: ({ requestId }) =>
NextResponse.json({ error: 'Internal server error', requestId }, { status: 500 }),
}
)
@@ -122,7 +124,7 @@ export function defineInternalBinaryRoute<
}
function createJsonErrorResponse(descriptor: JsonErrorResponseDescriptor): NextResponse {
return NextResponse.json(descriptor.body, {
return NextResponse.json(withRequestId(descriptor.body), {
status: descriptor.status,
headers: descriptor.headers,
})
apps/sim/lib/api/server/routes/internal-json-route.test.ts
+110 -10
View File
@@ -1,19 +1,23 @@
/**
* @vitest-environment node
*/
import { getRequestContext } from '@sim/logger'
import { NextRequest } from 'next/server'
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { z } from 'zod'
import { defineRouteContract } from '@/lib/api/contracts'
import {
defineInternalJsonRoute,
InternalUnauthenticatedError,
internalErrorResponse,
internalPlainOrchestrationErrorPolicy,
internalOrchestrationErrorPolicy,
internalRateLimits,
} from '@/lib/api/server/routes/internal-json-route'
import { OrchestrationError } from '@/lib/core/orchestration/types'
import { HttpError } from '@/lib/core/utils/http-error'
const mockGetRequestContext = vi.mocked(getRequestContext)
class TestLockedError extends HttpError {
readonly statusCode = 423
}
@@ -39,6 +43,7 @@ const contract = defineRouteContract({
describe('defineInternalJsonRoute', () => {
beforeEach(() => {
vi.clearAllMocks()
mockGetRequestContext.mockReturnValue(undefined)
})
it('uses the use-case result directly when it already matches the contract', async () => {
@@ -47,7 +52,7 @@ describe('defineInternalJsonRoute', () => {
auth,
operation,
rateLimit: internalRateLimits.none({ reason: 'Unit test' }),
errorPolicy: internalPlainOrchestrationErrorPolicy,
errorPolicy: internalOrchestrationErrorPolicy,
mapInput: () => undefined,
useCase: {
operation,
@@ -70,7 +75,7 @@ describe('defineInternalJsonRoute', () => {
auth,
operation,
rateLimit: internalRateLimits.none({ reason: 'Unit test' }),
errorPolicy: internalPlainOrchestrationErrorPolicy,
errorPolicy: internalOrchestrationErrorPolicy,
mapInput: () => undefined,
useCase: {
operation,
@@ -92,7 +97,7 @@ describe('defineInternalJsonRoute', () => {
auth,
operation,
rateLimit: internalRateLimits.none({ reason: 'Unit test' }),
errorPolicy: internalPlainOrchestrationErrorPolicy,
errorPolicy: internalOrchestrationErrorPolicy,
mapInput: () => undefined,
useCase: {
operation,
@@ -105,7 +110,10 @@ describe('defineInternalJsonRoute', () => {
const response = await handler(new NextRequest('http://localhost/api/test/internal-json-route'))
expect(response.status).toBe(423)
await expect(response.json()).resolves.toEqual({ error: 'Table imports are locked' })
await expect(response.json()).resolves.toEqual({
error: 'Table imports are locked',
requestId: expect.any(String),
})
expect(response.headers.get('x-request-id')).toBeTruthy()
})
@@ -115,6 +123,98 @@ describe('defineInternalJsonRoute', () => {
)
})
it('projects a classified orchestration error as a bare error envelope', async () => {
mockGetRequestContext.mockReturnValue({ requestId: 'req-orchestration' })
const handler = defineInternalJsonRoute({
contract,
auth,
operation,
rateLimit: internalRateLimits.none({ reason: 'Unit test' }),
errorPolicy: internalOrchestrationErrorPolicy,
mapInput: () => undefined,
useCase: {
operation,
async execute() {
throw new OrchestrationError('not_found', 'Widget not found')
},
},
})
const response = await handler(new NextRequest('http://localhost/api/test/internal-json-route'))
const body = await response.json()
expect(response.status).toBe(404)
expect(body).toEqual({ error: 'Widget not found', requestId: 'req-orchestration' })
expect(body).not.toHaveProperty('success')
})
it('stamps the request id onto an authentication failure', async () => {
mockGetRequestContext.mockReturnValue({ requestId: 'req-auth' })
const handler = defineInternalJsonRoute({
contract,
auth: {
authenticate: vi.fn(async () => {
throw new InternalUnauthenticatedError('Unauthorized')
}),
},
operation,
rateLimit: internalRateLimits.none({ reason: 'Unit test' }),
errorPolicy: internalOrchestrationErrorPolicy,
mapInput: () => undefined,
useCase: {
operation,
async execute() {
return { value: 'unreachable' }
},
},
})
const response = await handler(new NextRequest('http://localhost/api/test/internal-json-route'))
expect(response.status).toBe(401)
await expect(response.json()).resolves.toEqual({
error: 'Unauthorized',
requestId: 'req-auth',
})
})
it('stamps the request id onto a request parsing failure', async () => {
mockGetRequestContext.mockReturnValue({ requestId: 'req-parse' })
const queryContract = defineRouteContract({
method: 'GET',
path: '/api/test/internal-json-route',
query: z.object({ widgetId: z.string().min(1, 'widgetId is required') }),
response: {
mode: 'json',
schema: z.object({ value: z.string() }),
},
})
const handler = defineInternalJsonRoute({
contract: queryContract,
auth,
operation,
rateLimit: internalRateLimits.none({ reason: 'Unit test' }),
errorPolicy: internalOrchestrationErrorPolicy,
mapInput: () => undefined,
useCase: {
operation,
async execute() {
return { value: 'unreachable' }
},
},
})
const response = await handler(new NextRequest('http://localhost/api/test/internal-json-route'))
const body = await response.json()
expect(response.status).toBe(400)
expect(body.requestId).toBe('req-parse')
})
it('orders auth, rate limiting, parsing, async mapping, and application execution', async () => {
const events: string[] = []
const orderedContract = defineRouteContract({
@@ -142,7 +242,7 @@ describe('defineInternalJsonRoute', () => {
events.push('rate')
},
},
errorPolicy: internalPlainOrchestrationErrorPolicy,
errorPolicy: internalOrchestrationErrorPolicy,
async mapInput({ body }) {
events.push('map:start')
await Promise.resolve()
@@ -176,7 +276,7 @@ describe('defineInternalJsonRoute', () => {
auth,
operation,
rateLimit: internalRateLimits.none({ reason: 'Unit test' }),
errorPolicy: internalPlainOrchestrationErrorPolicy,
errorPolicy: internalOrchestrationErrorPolicy,
async mapInput() {
await Promise.resolve()
throw new OrchestrationError('validation', 'Invalid mapped input')
@@ -198,7 +298,7 @@ describe('defineInternalJsonRoute', () => {
auth,
operation,
rateLimit: internalRateLimits.none({ reason: 'Unit test' }),
errorPolicy: internalPlainOrchestrationErrorPolicy,
errorPolicy: internalOrchestrationErrorPolicy,
mapInput: () => undefined,
useCase: {
operation,
@@ -222,7 +322,7 @@ describe('defineInternalJsonRoute', () => {
auth,
operation,
rateLimit: internalRateLimits.none({ reason: 'Unit test' }),
errorPolicy: internalPlainOrchestrationErrorPolicy,
errorPolicy: internalOrchestrationErrorPolicy,
mapInput: () => undefined,
useCase: {
operation,
@@ -256,7 +356,7 @@ describe('defineInternalJsonRoute', () => {
auth,
operation,
rateLimit: internalRateLimits.none({ reason: 'Unit test' }),
errorPolicy: internalPlainOrchestrationErrorPolicy,
errorPolicy: internalOrchestrationErrorPolicy,
mapInput: () => undefined,
useCase: {
operation,
apps/sim/lib/api/server/routes/internal-json-route.ts
+24 -20
View File
@@ -8,6 +8,7 @@ import type { NextRequest } from 'next/server'
import { NextResponse } from 'next/server'
import type { ContractJsonResponse } from '@/lib/api/contracts'
import { requireJsonRouteDefinition } from '@/lib/api/server/routes/definition'
import { responseWithRequestId, withRequestId } from '@/lib/api/server/routes/request-id'
import type {
JsonApiRouteContract,
JsonErrorResponseDescriptor,
@@ -116,18 +117,24 @@ export interface InternalErrorPolicy {
unhandled?(): JsonErrorResponseDescriptor
}
/**
* The single internal error envelope: `{ error, requestId? }`.
*
* Routes previously chose between a bare `{ error }` and a `{ success: false,
* error }` variant. That split approximated pre-builder behavior, where the
* shape depended on which branch failed — guard clauses returned `{ error }`
* while a route's terminal `try/catch` returned `{ success: false, error }`.
* A per-route policy cannot express a per-branch rule, so the two variants
* disagreed on the same status across families. The bare shape wins because it
* is what {@link messageFromErrorBody} on the client reads and what the
* majority of migrated routes already emitted.
*
* `success: false` is not carried on error bodies: `requestJson` throws an
* `ApiClientError` for any non-2xx response, so no typed client ever observes
* the discriminator. `success: true` on *success* bodies is a separate
* contract and is unaffected.
*/
export const internalOrchestrationErrorPolicy: InternalErrorPolicy = {
project(error) {
const classified = asOrchestrationError(error)
if (!classified) return null
return internalErrorResponse(statusForOrchestrationError(classified.code), {
success: false,
error: classified.message,
})
},
}
export const internalPlainOrchestrationErrorPolicy: InternalErrorPolicy = {
project(error) {
const classified = asOrchestrationError(error)
if (!classified) return null
@@ -231,7 +238,7 @@ type InternalJsonRouteOptions<
} & InternalJsonPresenter<C, R>
function createJsonErrorResponse(descriptor: JsonErrorResponseDescriptor): NextResponse {
return NextResponse.json(descriptor.body, {
return NextResponse.json(withRequestId(descriptor.body), {
status: descriptor.status,
headers: descriptor.headers,
})
@@ -297,7 +304,7 @@ export function defineInternalJsonRoute<
principal = await options.auth.authenticate(request, rawParams)
} catch (error) {
if (error instanceof InternalUnauthenticatedError) {
return NextResponse.json({ error: error.message }, { status: 401 })
return createJsonErrorResponse(internalErrorResponse(401, { error: error.message }))
}
throw error
}
@@ -318,7 +325,7 @@ export function defineInternalJsonRoute<
context ?? {},
options.parseOptions
)
if (!parsed.success) return parsed.response
if (!parsed.success) return responseWithRequestId(parsed.response)
try {
const input = await options.mapInput(parsed.data, { principal, request })
@@ -358,15 +365,12 @@ export function defineInternalJsonRoute<
}
},
{
typedErrorResponse: ({ error, status }) =>
NextResponse.json({ error: error.message }, { status }),
typedErrorResponse: ({ error, status, requestId }) =>
NextResponse.json({ error: error.message, requestId }, { status }),
unhandledErrorResponse: () =>
createJsonErrorResponse(
options.errorPolicy.unhandled?.() ??
internalErrorResponse(500, {
success: false,
error: 'Internal server error',
})
internalErrorResponse(500, { error: 'Internal server error' })
),
}
)
apps/sim/lib/api/server/routes/request-id.test.ts
+106
View File
@@ -0,0 +1,106 @@
/**
* @vitest-environment node
*/
import { getRequestContext } from '@sim/logger'
import { NextResponse } from 'next/server'
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { responseWithRequestId, withRequestId } from '@/lib/api/server/routes/request-id'
const mockGetRequestContext = vi.mocked(getRequestContext)
describe('withRequestId', () => {
beforeEach(() => {
vi.clearAllMocks()
mockGetRequestContext.mockReturnValue(undefined)
})
it('stamps the ambient request id onto an error body', () => {
mockGetRequestContext.mockReturnValue({ requestId: 'req-123' })
expect(withRequestId({ error: 'Table not found' })).toEqual({
error: 'Table not found',
requestId: 'req-123',
})
})
it('leaves the body untouched when there is no active request scope', () => {
expect(withRequestId({ error: 'Table not found' })).toEqual({ error: 'Table not found' })
})
it('does not overwrite a requestId the policy already set', () => {
mockGetRequestContext.mockReturnValue({ requestId: 'req-123' })
expect(withRequestId({ error: 'boom', requestId: 'explicit' })).toEqual({
error: 'boom',
requestId: 'explicit',
})
})
it('passes through non-object bodies', () => {
mockGetRequestContext.mockReturnValue({ requestId: 'req-123' })
expect(withRequestId('plain text')).toBe('plain text')
expect(withRequestId(null)).toBeNull()
expect(withRequestId([{ error: 'a' }])).toEqual([{ error: 'a' }])
})
})
describe('responseWithRequestId', () => {
beforeEach(() => {
vi.clearAllMocks()
mockGetRequestContext.mockReturnValue(undefined)
})
it('stamps the request id into an already-built JSON error response', async () => {
mockGetRequestContext.mockReturnValue({ requestId: 'req-123' })
const stamped = await responseWithRequestId(
NextResponse.json({ error: 'Validation error', details: [] }, { status: 400 })
)
expect(stamped.status).toBe(400)
await expect(stamped.json()).resolves.toEqual({
error: 'Validation error',
details: [],
requestId: 'req-123',
})
})
it('preserves headers other than a stale content-length', async () => {
mockGetRequestContext.mockReturnValue({ requestId: 'req-123' })
const original = NextResponse.json(
{ error: 'Validation error' },
{ status: 400, headers: { 'x-custom': 'kept', 'content-length': '29' } }
)
const stamped = await responseWithRequestId(original)
expect(stamped.headers.get('x-custom')).toBe('kept')
expect(stamped.headers.get('content-length')).toBeNull()
})
it('returns the original response when there is no active request scope', async () => {
const original = NextResponse.json({ error: 'Validation error' }, { status: 400 })
expect(await responseWithRequestId(original)).toBe(original)
})
it('returns the original response when the body is not JSON', async () => {
mockGetRequestContext.mockReturnValue({ requestId: 'req-123' })
const original = new NextResponse('plain text', {
status: 400,
headers: { 'content-type': 'text/plain' },
})
expect(await responseWithRequestId(original)).toBe(original)
})
it('leaves a response that already carries a requestId untouched', async () => {
mockGetRequestContext.mockReturnValue({ requestId: 'req-123' })
const original = NextResponse.json({ error: 'boom', requestId: 'explicit' }, { status: 400 })
expect(await responseWithRequestId(original)).toBe(original)
})
})
apps/sim/lib/api/server/routes/request-id.ts
+59
View File
@@ -0,0 +1,59 @@
import { getRequestContext } from '@sim/logger'
import { NextResponse } from 'next/server'
/**
* Stamps the ambient request id onto an internal error body.
*
* `withRouteHandler` runs every handler inside a `runWithRequestContext` scope
* and already emits the same id as the `x-request-id` header. Carrying it in
* the body as well is what lets a user paste an error straight from the UI and
* have it correlate to a log line — a header is not visible at that point.
*
* Returns the body untouched when it is not a plain object (so array and
* scalar error bodies are preserved), when a `requestId` is already present,
* or when there is no active request scope — the last case keeps the field out
* of unit tests, where `getRequestContext` is mocked to `undefined`.
*/
export function withRequestId(body: unknown): unknown {
if (!body || typeof body !== 'object' || Array.isArray(body)) return body
if ('requestId' in body) return body
const requestId = getRequestContext()?.requestId
if (!requestId) return body
return { ...body, requestId }
}
/**
* Rebuilds an already-constructed JSON error response with the ambient request
* id stamped into its body.
*
* Request parsing failures arrive as a finished `NextResponse` from the shared
* validation helpers, which v1 and v2 routes also use and whose envelopes must
* not change. Stamping here — at the internal builders' call site rather than
* inside those helpers — keeps the added field scoped to internal routes.
*
* Returns the original response when there is no active request scope, when the
* body is not JSON, or when it cannot be re-read. The body is read from a clone
* so the original stays usable on any of those paths.
*/
export async function responseWithRequestId(
response: NextResponse<unknown>
): Promise<NextResponse<unknown>> {
if (!getRequestContext()?.requestId) return response
if (!response.headers.get('content-type')?.includes('application/json')) return response
let body: unknown
try {
body = await response.clone().json()
} catch {
return response
}
const stamped = withRequestId(body)
if (stamped === body) return response
const headers = new Headers(response.headers)
headers.delete('content-length')
return NextResponse.json(stamped, { status: response.status, headers })
}
apps/sim/lib/knowledge/api/route-policies.ts
+4 -4
View File
@@ -3,7 +3,7 @@ import {
createV2ResourceConcealmentPolicy,
type InternalErrorPolicy,
internalErrorResponse,
internalPlainOrchestrationErrorPolicy,
internalOrchestrationErrorPolicy,
type V2ErrorPolicy,
v2OrchestrationErrorPolicy,
} from '@/lib/api/server/routes'
@@ -16,7 +16,7 @@ import { v2Error } from '@/app/api/v2/lib/response'
function internalKnowledgeErrorPolicy(unhandledMessage: string): InternalErrorPolicy {
return {
project: internalPlainOrchestrationErrorPolicy.project,
project: internalOrchestrationErrorPolicy.project,
unhandled: () => internalErrorResponse(500, { error: unhandledMessage }),
}
}
@@ -29,7 +29,7 @@ const internalKnowledgeUploadErrorPolicy: InternalErrorPolicy = {
if (error instanceof KnowledgeUsageLimitExceededError) {
return internalErrorResponse(402, { error: error.message })
}
return internalPlainOrchestrationErrorPolicy.project(error)
return internalOrchestrationErrorPolicy.project(error)
},
unhandled: () =>
internalErrorResponse(500, { error: 'Failed to process knowledge upload request' }),
@@ -43,7 +43,7 @@ const internalKnowledgeSearchErrorPolicy: InternalErrorPolicy = {
if (error instanceof KnowledgeSearchProvenanceUnavailableError) {
return internalErrorResponse(422, { error: error.message })
}
return internalPlainOrchestrationErrorPolicy.project(error)
return internalOrchestrationErrorPolicy.project(error)
},
unhandled: () => internalErrorResponse(500, { error: 'Failed to perform vector search' }),
}
apps/sim/lib/table/api/route-policies.test.ts
+2 -2
View File
@@ -24,7 +24,7 @@ vi.unmock('@/lib/auth/internal')
import {
InternalUnauthenticatedError,
internalPlainOrchestrationErrorPolicy,
internalOrchestrationErrorPolicy,
} from '@/lib/api/server/routes'
import { generateInternalDelegationToken, generateInternalToken } from '@/lib/auth/internal'
import { OrchestrationError } from '@/lib/core/orchestration/types'
@@ -176,7 +176,7 @@ describe('internal Table route authentication', () => {
it('renders an invalid related workflow as 400 on internal and v2 surfaces', async () => {
const error = new OrchestrationError('validation', 'Invalid workflow ID')
expect(internalPlainOrchestrationErrorPolicy.project(error)).toEqual({
expect(internalOrchestrationErrorPolicy.project(error)).toEqual({
status: 400,
body: { error: 'Invalid workflow ID' },
headers: undefined,
apps/sim/lib/workspace-files/api/internal-error-policies.test.ts
+1 -1
View File
@@ -41,7 +41,7 @@ describe('internal file error policies', () => {
)
).toEqual({
status: 402,
body: { success: false, error: 'Storage limit exceeded' },
body: { error: 'Storage limit exceeded' },
headers: undefined,
})
})
apps/sim/lib/workspace-files/api/internal-error-policies.ts
+4 -9
View File
@@ -4,7 +4,6 @@ import {
type InternalErrorPolicy,
internalErrorResponse,
internalOrchestrationErrorPolicy,
internalPlainOrchestrationErrorPolicy,
} from '@/lib/api/server/routes'
import { StorageLimitExceededError } from '@/lib/billing/storage'
import { asOrchestrationError, statusForOrchestrationError } from '@/lib/core/orchestration/types'
@@ -16,12 +15,12 @@ import { StyleExtractionUnsupportedError } from '@/lib/workspace-files/applicati
const logger = createLogger('InternalWorkspaceFileErrors')
const style = extendInternalErrorPolicy(internalPlainOrchestrationErrorPolicy, (error) => {
const style = extendInternalErrorPolicy(internalOrchestrationErrorPolicy, (error) => {
if (!(error instanceof StyleExtractionUnsupportedError)) return null
return internalErrorResponse(422, { error: error.message })
})
const compiledCheck = extendInternalErrorPolicy(internalPlainOrchestrationErrorPolicy, (error) => {
const compiledCheck = extendInternalErrorPolicy(internalOrchestrationErrorPolicy, (error) => {
if (error instanceof CompiledCheckUnsupportedError) {
return internalErrorResponse(422, { error: error.message })
}
@@ -33,7 +32,7 @@ const compiledCheck = extendInternalErrorPolicy(internalPlainOrchestrationErrorP
const content = extendInternalErrorPolicy(internalOrchestrationErrorPolicy, (error) => {
if (!(error instanceof StorageLimitExceededError)) return null
return internalErrorResponse(402, { success: false, error: error.message })
return internalErrorResponse(402, { error: error.message })
})
const downloadUrl: InternalErrorPolicy = {
@@ -41,10 +40,7 @@ const downloadUrl: InternalErrorPolicy = {
const typed = internalOrchestrationErrorPolicy.project(error)
if (typed) return typed
logger.error('Failed to generate workspace file download URL', { error })
return internalErrorResponse(500, {
success: false,
error: 'Failed to generate download URL',
})
return internalErrorResponse(500, { error: 'Failed to generate download URL' })
},
}
@@ -84,7 +80,6 @@ const inline: InternalErrorPolicy = {
export const internalFileErrorPolicies = {
default: internalOrchestrationErrorPolicy,
plain: internalPlainOrchestrationErrorPolicy,
content,
style,
compiledCheck,