From 520d892b54a5acfd2095960ac083c4dd4eafe40a Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 13 Nov 2026 18:21:54 -0800 Subject: [PATCH 1/2] improvement(api): log the v2 error code, field, and refusal reason on the request's closing line --- apps/sim/app/api/v2/lib/response.ts | 13 +++++++++-- packages/logger/src/index.ts | 6 +++++ packages/logger/src/request-context.ts | 28 +++++++++++++++++++++++ packages/testing/src/mocks/logger.mock.ts | 1 + 4 files changed, 46 insertions(+), 2 deletions(-) diff --git a/apps/sim/app/api/v2/lib/response.ts b/apps/sim/app/api/v2/lib/response.ts index 82459661bec..12942452f58 100644 --- a/apps/sim/app/api/v2/lib/response.ts +++ b/apps/sim/app/api/v2/lib/response.ts @@ -1,4 +1,4 @@ -import { getRequestContext } from '@sim/logger' +import { getRequestContext, setRequestError } from '@sim/logger' import { NextResponse } from 'next/server' import type { ZodError } from 'zod' import { @@ -211,6 +211,7 @@ export function v2Error( message, } if (options.details !== undefined) error.details = options.details + setRequestError({ code, reason: detailCode(options.details) }) const requestId = getRequestContext()?.requestId if (requestId) error.requestId = requestId const status = options.status ?? V2_ERROR_STATUS_BY_CODE[code] @@ -231,6 +232,12 @@ export function v2Error( ) } +/** The cause a refusal names in `details.code`, a closed vocabulary safe to log. */ +function detailCode(details: unknown): string | undefined { + if (typeof details !== 'object' || details === null || !('code' in details)) return undefined + return typeof details.code === 'string' ? details.code : undefined +} + /** Renders a trusted typed HTTP error without changing the v2 envelope. */ export function v2HttpError(error: HttpError): NextResponse { const code = V2_ERROR_CODE_BY_STATUS[error.statusCode] @@ -267,9 +274,11 @@ export function v2ValidationError(error: ZodError, slice?: RequestSlice): NextRe ? issueMessage.slice(fieldLabel.length) : issueMessage const location = [slice, ...path].filter((part) => part !== undefined && part !== '').join('.') - return v2Error('BAD_REQUEST', location ? `${location}: ${message}` : message, { + const response = v2Error('BAD_REQUEST', location ? `${location}: ${message}` : message, { details: serializeZodIssues(error), }) + if (location) setRequestError({ code: 'BAD_REQUEST', field: location }) + return response } /** diff --git a/packages/logger/src/index.ts b/packages/logger/src/index.ts index aea883cf36a..cd2e782d6df 100644 --- a/packages/logger/src/index.ts +++ b/packages/logger/src/index.ts @@ -400,6 +400,11 @@ const requestContextMetadata = (context: RequestContext): LoggerMetadata => { if (context.auth.clientId) metadata.authClientId = context.auth.clientId } if (context.callChain) metadata.callDepth = context.callChain.length + if (context.error) { + metadata.errorCode = context.error.code + if (context.error.field) metadata.errorField = context.error.field + if (context.error.reason) metadata.errorReason = context.error.reason + } return metadata } @@ -626,6 +631,7 @@ export { getRequestContext, runWithRequestContext, setRequestAuth, + setRequestError, setRequestTraceId, } from './request-context' diff --git a/packages/logger/src/request-context.ts b/packages/logger/src/request-context.ts index 04cd36734da..412d5e737bb 100644 --- a/packages/logger/src/request-context.ts +++ b/packages/logger/src/request-context.ts @@ -29,6 +29,23 @@ export interface RequestContext { * oldest first. Present only when one workflow's execution made this call. */ callChain?: readonly string[] + /** + * What the request's error response told the caller, stamped as the response + * is rendered so the route's closing log line can be grouped by it without + * reading the body. + */ + error?: RequestError +} + +/** + * The machine-readable part of an error response: its stable `code`, where a + * validation failure occurred (`body.columns.0.type`), and the cause a refusal + * named (`insufficient_scope`). Never the message, which can quote user input. + */ +export interface RequestError { + code: string + field?: string + reason?: string } /** @@ -122,3 +139,14 @@ export function setRequestAuth(auth: RequestAuth, options: SetRequestAuthOptions store.auth = auth if (store.client) store.client = attributeUndeclaredClient(store.client, auth.kind) } + +/** + * Records the error the current request is answering with, so every later log + * line in it, including the route's closing one, carries the code. A later + * call replaces an earlier one: the response that is returned is the one built + * last. No-op outside a request context. + */ +export function setRequestError(error: RequestError): void { + const store = storage.getStore() + if (store) store.error = error +} diff --git a/packages/testing/src/mocks/logger.mock.ts b/packages/testing/src/mocks/logger.mock.ts index fd92ae4db40..d2c6e2411fc 100644 --- a/packages/testing/src/mocks/logger.mock.ts +++ b/packages/testing/src/mocks/logger.mock.ts @@ -89,4 +89,5 @@ export const loggerMock = { getRequestContext: vi.fn(() => undefined), setRequestTraceId: vi.fn(), setRequestAuth: vi.fn(), + setRequestError: vi.fn(), } From 74c96ede75f9a702fc342453b1698e6c31b1d678 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 13 Nov 2026 18:33:03 -0800 Subject: [PATCH 2/2] improvement(api): tag the returned response instead of the request, and log only the declared top-level field --- apps/sim/app/api/v2/lib/response.ts | 17 ++++++++--- apps/sim/lib/core/utils/response-error.ts | 30 +++++++++++++++++++ apps/sim/lib/core/utils/with-route-handler.ts | 5 ++-- packages/logger/src/index.ts | 6 ---- packages/logger/src/request-context.ts | 28 ----------------- packages/testing/src/mocks/logger.mock.ts | 1 - 6 files changed, 46 insertions(+), 41 deletions(-) create mode 100644 apps/sim/lib/core/utils/response-error.ts diff --git a/apps/sim/app/api/v2/lib/response.ts b/apps/sim/app/api/v2/lib/response.ts index 12942452f58..1e95d321f71 100644 --- a/apps/sim/app/api/v2/lib/response.ts +++ b/apps/sim/app/api/v2/lib/response.ts @@ -1,4 +1,4 @@ -import { getRequestContext, setRequestError } from '@sim/logger' +import { getRequestContext } from '@sim/logger' import { NextResponse } from 'next/server' import type { ZodError } from 'zod' import { @@ -22,6 +22,7 @@ import { type OrchestrationErrorCode, } from '@/lib/core/orchestration/types' import type { HttpError } from '@/lib/core/utils/http-error' +import { describeResponseError } from '@/lib/core/utils/response-error' import type { RateLimitResult } from '@/app/api/v1/middleware' /** @@ -211,14 +212,13 @@ export function v2Error( message, } if (options.details !== undefined) error.details = options.details - setRequestError({ code, reason: detailCode(options.details) }) const requestId = getRequestContext()?.requestId if (requestId) error.requestId = requestId const status = options.status ?? V2_ERROR_STATUS_BY_CODE[code] const retryAfterSeconds = options.omitRetryAfter ? undefined : RETRY_AFTER_SECONDS_BY_STATUS[status] - return NextResponse.json( + const response = NextResponse.json( { error }, { status, @@ -230,6 +230,9 @@ export function v2Error( }, } ) + const reason = detailCode(options.details) + describeResponseError(response, { errorCode: code, ...(reason ? { errorReason: reason } : {}) }) + return response } /** The cause a refusal names in `details.code`, a closed vocabulary safe to log. */ @@ -277,7 +280,13 @@ export function v2ValidationError(error: ZodError, slice?: RequestSlice): NextRe const response = v2Error('BAD_REQUEST', location ? `${location}: ${message}` : message, { details: serializeZodIssues(error), }) - if (location) setRequestError({ code: 'BAD_REQUEST', field: location }) + // Deeper segments can be record keys the caller wrote (an import's CSV headers); no slice is itself a record. + const field = [slice, path[0]].filter((part) => part !== undefined && part !== '').join('.') + describeResponseError(response, { + errorCode: 'BAD_REQUEST', + ...(field ? { errorField: field } : {}), + ...(error.issues[0] ? { errorReason: error.issues[0].code } : {}), + }) return response } diff --git a/apps/sim/lib/core/utils/response-error.ts b/apps/sim/lib/core/utils/response-error.ts new file mode 100644 index 00000000000..a7445a0e68b --- /dev/null +++ b/apps/sim/lib/core/utils/response-error.ts @@ -0,0 +1,30 @@ +/** + * The machine-readable part of an error response, kept beside the response so + * the route handler's closing log line can be grouped by why a request was + * refused without reading the body. Never the message, which can quote user + * input. + */ +export interface ResponseErrorSummary { + /** The stable error code the body carries, e.g. `BAD_REQUEST`. */ + errorCode: string + /** For a validation failure, the request slice and its declared top-level field, e.g. `body.mapping`. */ + errorField?: string + /** The cause the refusal named (`READ_ONLY_API_KEY`) or the validation issue kind (`invalid_type`). */ + errorReason?: string +} + +/** + * Keyed by the response object, so only a response a route actually returns is + * ever read back. One built only to extract its message is never logged. + */ +const summaries = new WeakMap() + +/** Records what `response` tells its caller; a later call for the same response replaces it. */ +export function describeResponseError(response: Response, summary: ResponseErrorSummary): void { + summaries.set(response, summary) +} + +/** What `describeResponseError` recorded for `response`, if anything. */ +export function readResponseError(response: Response): ResponseErrorSummary | undefined { + return summaries.get(response) +} diff --git a/apps/sim/lib/core/utils/with-route-handler.ts b/apps/sim/lib/core/utils/with-route-handler.ts index 7a5fc3889c4..7de2af3d220 100644 --- a/apps/sim/lib/core/utils/with-route-handler.ts +++ b/apps/sim/lib/core/utils/with-route-handler.ts @@ -15,6 +15,7 @@ import { ADMISSION_RETRY_AFTER_SECONDS } from '@/lib/core/admission/transient-fa import { FILE_DELIVERY_CSP_PATH_PATTERN, getMainCSPPolicy } from '@/lib/core/security/csp' import { HttpError } from '@/lib/core/utils/http-error' import { generateRequestId } from '@/lib/core/utils/request' +import { readResponseError } from '@/lib/core/utils/response-error' import { MAX_CALL_CHAIN_DEPTH, parseCallChain, SIM_VIA_HEADER } from '@/lib/execution/call-chain' import { trackInFlightRequest } from '@/lib/monitoring/in-flight-requests' import { withPermissionGroupScope } from '@/lib/permission-groups/request-scope.server' @@ -274,9 +275,9 @@ export function withRouteHandler( const duration = Date.now() - startTime if (status >= 500) { - logger.error('Server error response', { status, duration }) + logger.error('Server error response', { status, duration, ...readResponseError(response) }) } else if (status >= 400) { - logger.warn('Client error response', { status, duration }) + logger.warn('Client error response', { status, duration, ...readResponseError(response) }) } else if (status > 0) { logger.info('OK', { status, duration }) } diff --git a/packages/logger/src/index.ts b/packages/logger/src/index.ts index cd2e782d6df..aea883cf36a 100644 --- a/packages/logger/src/index.ts +++ b/packages/logger/src/index.ts @@ -400,11 +400,6 @@ const requestContextMetadata = (context: RequestContext): LoggerMetadata => { if (context.auth.clientId) metadata.authClientId = context.auth.clientId } if (context.callChain) metadata.callDepth = context.callChain.length - if (context.error) { - metadata.errorCode = context.error.code - if (context.error.field) metadata.errorField = context.error.field - if (context.error.reason) metadata.errorReason = context.error.reason - } return metadata } @@ -631,7 +626,6 @@ export { getRequestContext, runWithRequestContext, setRequestAuth, - setRequestError, setRequestTraceId, } from './request-context' diff --git a/packages/logger/src/request-context.ts b/packages/logger/src/request-context.ts index 412d5e737bb..04cd36734da 100644 --- a/packages/logger/src/request-context.ts +++ b/packages/logger/src/request-context.ts @@ -29,23 +29,6 @@ export interface RequestContext { * oldest first. Present only when one workflow's execution made this call. */ callChain?: readonly string[] - /** - * What the request's error response told the caller, stamped as the response - * is rendered so the route's closing log line can be grouped by it without - * reading the body. - */ - error?: RequestError -} - -/** - * The machine-readable part of an error response: its stable `code`, where a - * validation failure occurred (`body.columns.0.type`), and the cause a refusal - * named (`insufficient_scope`). Never the message, which can quote user input. - */ -export interface RequestError { - code: string - field?: string - reason?: string } /** @@ -139,14 +122,3 @@ export function setRequestAuth(auth: RequestAuth, options: SetRequestAuthOptions store.auth = auth if (store.client) store.client = attributeUndeclaredClient(store.client, auth.kind) } - -/** - * Records the error the current request is answering with, so every later log - * line in it, including the route's closing one, carries the code. A later - * call replaces an earlier one: the response that is returned is the one built - * last. No-op outside a request context. - */ -export function setRequestError(error: RequestError): void { - const store = storage.getStore() - if (store) store.error = error -} diff --git a/packages/testing/src/mocks/logger.mock.ts b/packages/testing/src/mocks/logger.mock.ts index d2c6e2411fc..fd92ae4db40 100644 --- a/packages/testing/src/mocks/logger.mock.ts +++ b/packages/testing/src/mocks/logger.mock.ts @@ -89,5 +89,4 @@ export const loggerMock = { getRequestContext: vi.fn(() => undefined), setRequestTraceId: vi.fn(), setRequestAuth: vi.fn(), - setRequestError: vi.fn(), }