From aeb8ccf556c88ebfcbb77abb67e2d9edd4ed9daf Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sun, 1 Nov 2026 19:46:04 -0800 Subject: [PATCH 1/2] fix(logs): return 503 instead of 500 when a v2 logs read is cancelled --- .../sim/lib/api/server/routes/billing-read.ts | 12 +----- apps/sim/lib/db/read-retry.ts | 15 ++++++- apps/sim/lib/logs/api/route-policies.test.ts | 43 ++++++++++++++++++- apps/sim/lib/logs/api/route-policies.ts | 15 +++++-- 4 files changed, 70 insertions(+), 15 deletions(-) diff --git a/apps/sim/lib/api/server/routes/billing-read.ts b/apps/sim/lib/api/server/routes/billing-read.ts index 97d750381f3..11da5785d05 100644 --- a/apps/sim/lib/api/server/routes/billing-read.ts +++ b/apps/sim/lib/api/server/routes/billing-read.ts @@ -1,23 +1,15 @@ -import { getPostgresCancellationReason } from '@sim/utils/errors' import { extendInternalErrorPolicy, internalErrorResponse, internalOrchestrationErrorPolicy, } from '@/lib/api/server/routes/internal-json-route' -import { DatabaseReadDeadlineError, isTransientDatabaseReadError } from '@/lib/db/read-retry' +import { isDatabaseReadUnavailable } from '@/lib/db/read-retry' /** Presents known temporary read failures as retryable 503s without masking permanent errors. */ export const internalBillingReadErrorPolicy = extendInternalErrorPolicy( internalOrchestrationErrorPolicy, (error) => { - const cancellationReason = getPostgresCancellationReason(error) - if ( - !(error instanceof DatabaseReadDeadlineError) && - !isTransientDatabaseReadError(error) && - cancellationReason !== 'statement_timeout' && - cancellationReason !== 'transaction_timeout' - ) - return null + if (!isDatabaseReadUnavailable(error)) return null return internalErrorResponse( 503, { error: 'Billing information is temporarily unavailable. Please try again.' }, diff --git a/apps/sim/lib/db/read-retry.ts b/apps/sim/lib/db/read-retry.ts index 2e95a093090..94e02fde007 100644 --- a/apps/sim/lib/db/read-retry.ts +++ b/apps/sim/lib/db/read-retry.ts @@ -1,5 +1,5 @@ import { createLogger } from '@sim/logger' -import { getPostgresErrorCode } from '@sim/utils/errors' +import { getPostgresCancellationReason, getPostgresErrorCode } from '@sim/utils/errors' import { sleep } from '@sim/utils/helpers' import { backoffWithJitter } from '@sim/utils/retry' @@ -43,6 +43,19 @@ export function isTransientDatabaseReadError(error: unknown): boolean { return code !== undefined && TRANSIENT_READ_CODES.has(code) } +/** + * Whether a failed read is temporary for the caller: a retryable driver error, an exhausted read + * deadline, or a statement or transaction timeout. Surfaces present these as retry-later responses; + * the timeouts stay out of {@link isTransientDatabaseReadError} because re-running a cancelled + * scan in-process only repeats its cost. Narrower than `isRetryableInfrastructureError`, which + * also treats a user-requested cancel as retryable. + */ +export function isDatabaseReadUnavailable(error: unknown): boolean { + if (error instanceof DatabaseReadDeadlineError || isTransientDatabaseReadError(error)) return true + const cancellationReason = getPostgresCancellationReason(error) + return cancellationReason === 'statement_timeout' || cancellationReason === 'transaction_timeout' +} + /** * Retries independent reads, rebuilding each statement or fresh read-only transaction * after the previous attempt has fully rolled back. Never retry writes, locking reads, diff --git a/apps/sim/lib/logs/api/route-policies.test.ts b/apps/sim/lib/logs/api/route-policies.test.ts index 5c8f316e871..37e121342d0 100644 --- a/apps/sim/lib/logs/api/route-policies.test.ts +++ b/apps/sim/lib/logs/api/route-policies.test.ts @@ -3,6 +3,7 @@ import { authInternalDelegationMock, authInternalDelegationMockFns, } from '@sim/testing/mocks/auth-internal-delegation.mock' +import { DrizzleQueryError } from 'drizzle-orm/errors' import { NextRequest } from 'next/server' import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest' @@ -10,7 +11,47 @@ vi.mock('@/lib/auth/internal-delegation', () => authInternalDelegationMock) vi.unmock('@/lib/auth/internal') import { generateInternalDelegationToken } from '@/lib/auth/internal' -import { internalLogsSessionOrExecutorAuth } from '@/lib/logs/api/route-policies' +import { + internalLogsSessionOrExecutorAuth, + v2LogErrorPolicies, +} from '@/lib/logs/api/route-policies' + +function failedQuery(code: string, message: string): DrizzleQueryError { + return new DrizzleQueryError('select', [], Object.assign(new Error(message), { code })) +} + +describe('v2 logs error policies', () => { + const policies = Object.entries(v2LogErrorPolicies) + + it.each(policies)( + '%s answers a replica recovery conflict with a retryable 503', + async (_, policy) => { + const response = policy.render( + failedQuery('40001', 'canceling statement due to conflict with recovery') + ) + + expect(response?.status).toBe(503) + expect(response?.headers.get('Retry-After')).toBeTruthy() + expect(await response?.json()).toMatchObject({ error: { code: 'SERVICE_UNAVAILABLE' } }) + } + ) + + it.each(policies)('%s answers a statement timeout with a retryable 503', (_, policy) => { + const response = policy.render( + failedQuery('57014', 'canceling statement due to statement timeout') + ) + + expect(response?.status).toBe(503) + expect(response?.headers.get('Retry-After')).toBeTruthy() + }) + + it.each(policies)('%s leaves a permanent database failure unhandled', (_, policy) => { + expect(policy.render(failedQuery('42P01', 'relation does not exist'))).toBeNull() + expect( + policy.render(failedQuery('57014', 'canceling statement due to user request')) + ).toBeNull() + }) +}) const mockGetSession = authMockFns.mockGetSession const mockBindDelegation = authInternalDelegationMockFns.mockBindInternalExecutorDelegation diff --git a/apps/sim/lib/logs/api/route-policies.ts b/apps/sim/lib/logs/api/route-policies.ts index 19c685eaa87..fbe4c3e513d 100644 --- a/apps/sim/lib/logs/api/route-policies.ts +++ b/apps/sim/lib/logs/api/route-policies.ts @@ -1,9 +1,11 @@ import { createInternalSessionOrExecutorAuth, createV2ResourceConcealmentPolicy, - v2OrchestrationErrorPolicy, + type V2ErrorPolicy, } from '@/lib/api/server/routes' +import { isDatabaseReadUnavailable } from '@/lib/db/read-retry' import { LOGS_DELEGATION_AUDIENCE } from '@/lib/logs/application/authorization' +import { v2CaughtOrchestrationError, v2Error } from '@/app/api/v2/lib/response' const internalLogsSessionOrExecutorAuthBase = createInternalSessionOrExecutorAuth({ audience: LOGS_DELEGATION_AUDIENCE, @@ -27,6 +29,12 @@ export const internalLogsSessionOrExecutorAuth = { }, } +const renderV2LogError: V2ErrorPolicy['render'] = (error) => { + if (isDatabaseReadUnavailable(error)) + return v2Error('SERVICE_UNAVAILABLE', 'Logs are temporarily unavailable. Please try again.') + return v2CaughtOrchestrationError(error) +} + /** * `GET /logs` and `GET /billing/logs` both take a caller-named `workspaceId` and * answer differently when the caller cannot reach it: this list projects the @@ -49,8 +57,9 @@ export const internalLogsSessionOrExecutorAuth = { * inconsistency for a larger one. */ export const v2LogErrorPolicies = { - default: v2OrchestrationErrorPolicy, + default: { render: renderV2LogError }, concealDetailAuthorization: createV2ResourceConcealmentPolicy({ notFoundMessage: 'Log not found', + render: renderV2LogError, }), -} as const +} as const satisfies Record From 080877bbdd27ecae1b98ff7c81b7c0a9fd830833 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sun, 1 Nov 2026 20:00:16 -0800 Subject: [PATCH 2/2] docs(db): describe how the unavailable-read predicate overlaps the infrastructure one --- apps/sim/lib/db/read-retry.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/sim/lib/db/read-retry.ts b/apps/sim/lib/db/read-retry.ts index 94e02fde007..2835d8aaa8e 100644 --- a/apps/sim/lib/db/read-retry.ts +++ b/apps/sim/lib/db/read-retry.ts @@ -47,8 +47,8 @@ export function isTransientDatabaseReadError(error: unknown): boolean { * Whether a failed read is temporary for the caller: a retryable driver error, an exhausted read * deadline, or a statement or transaction timeout. Surfaces present these as retry-later responses; * the timeouts stay out of {@link isTransientDatabaseReadError} because re-running a cancelled - * scan in-process only repeats its cost. Narrower than `isRetryableInfrastructureError`, which - * also treats a user-requested cancel as retryable. + * scan in-process only repeats its cost. Not interchangeable with `isRetryableInfrastructureError`: + * that one treats every 57014 cancel as retryable, including a user-requested one. */ export function isDatabaseReadUnavailable(error: unknown): boolean { if (error instanceof DatabaseReadDeadlineError || isTransientDatabaseReadError(error)) return true