fix(api): stamp requestId on internal auth and parse failures

Author: waleedlatif1

apps/sim/lib/api/server/routes/internal-binary-route.ts

@@ -5,9 +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 { withRequestId } from '@/lib/api/server/routes/request-id'
+import { responseWithRequestId, withRequestId } from '@/lib/api/server/routes/request-id'
 import type {
   BinaryApiRouteContract,
   BinaryResponseDescriptor,
@@ -82,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)

apps/sim/lib/api/server/routes/internal-json-route.test.ts

@@ -8,6 +8,7 @@ import { z } from 'zod'
 import { defineRouteContract } from '@/lib/api/contracts'
 import {
   defineInternalJsonRoute,
+  InternalUnauthenticatedError,
   internalErrorResponse,
   internalOrchestrationErrorPolicy,
   internalRateLimits,
@@ -148,6 +149,72 @@ describe('defineInternalJsonRoute', () => {
     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({

apps/sim/lib/api/server/routes/internal-json-route.ts

@@ -8,7 +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 { withRequestId } from '@/lib/api/server/routes/request-id'
+import { responseWithRequestId, withRequestId } from '@/lib/api/server/routes/request-id'
 import type {
   JsonApiRouteContract,
   JsonErrorResponseDescriptor,
@@ -304,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
       }
@@ -325,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 })

apps/sim/lib/api/server/routes/request-id.test.ts

@@ -2,8 +2,9 @@
  * @vitest-environment node
  */
 import { getRequestContext } from '@sim/logger'
+import { NextResponse } from 'next/server'
 import { beforeEach, describe, expect, it, vi } from 'vitest'
-import { withRequestId } from '@/lib/api/server/routes/request-id'
+import { responseWithRequestId, withRequestId } from '@/lib/api/server/routes/request-id'
 
 const mockGetRequestContext = vi.mocked(getRequestContext)
 
@@ -43,3 +44,63 @@ describe('withRequestId', () => {
     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

@@ -1,4 +1,5 @@
 import { getRequestContext } from '@sim/logger'
+import { NextResponse } from 'next/server'
 
 /**
  * Stamps the ambient request id onto an internal error body.
@@ -22,3 +23,37 @@ export function withRequestId(body: unknown): unknown {
 
   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 })
+}