fix(logs): return 503 instead of 500 when a v2 logs read is cancelled

Author: waleedlatif1

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.' },

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,

apps/sim/lib/logs/api/route-policies.test.ts

@@ -3,14 +3,55 @@ 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'
 
 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

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<string, V2ErrorPolicy>