Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion apps/sim/lib/db/read-retry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,20 @@ export function isTransientDatabaseReadError(error: unknown): boolean {
* rebuild the query outside any transaction and must have no side effects or locks.
* A failed connection cannot establish whether a write committed, so writes and
* transactions must never use this helper.
*
* `label` names the read in the retry log line so callers that wrap several can tell which flapped.
*/
export async function withDatabaseReadRetry<T>(read: () => Promise<T>): Promise<T> {
export async function withDatabaseReadRetry<T>(
read: () => Promise<T>,
options: { label?: string } = {}
): Promise<T> {
for (let attempt = 1; ; attempt++) {
try {
return await read()
} catch (error) {
if (attempt >= 3 || !isTransientDatabaseReadError(error)) throw error
logger.warn('Retrying transient database read', {
label: options.label,
attempt,
code: getPostgresErrorCode(error),
})
Expand Down
42 changes: 37 additions & 5 deletions apps/sim/lib/execution/preprocessing.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,18 @@
import { loggingSessionMock, workflowAuthzMockFns } from '@sim/testing'
import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest'
import { ADMISSION_ERROR_CODE } from '@/lib/core/admission/transient-failure'
import type { LoggingSession } from '@/lib/logs/execution/logging-session'

const {
mockSleep,
mockCheckAttributedUsageLimits,
mockCheckRateLimit,
mockGetActivelyBannedUserIds,
mockReserveExecutionSlot,
mockResolveBillingAttribution,
mockResolveSystemBillingAttribution,
} = vi.hoisted(() => ({
mockSleep: vi.fn().mockResolvedValue(undefined),
mockCheckAttributedUsageLimits: vi.fn(),
mockCheckRateLimit: vi.fn(),
mockGetActivelyBannedUserIds: vi.fn().mockResolvedValue([]),
Expand All @@ -22,6 +25,9 @@ const {
mockResolveSystemBillingAttribution: vi.fn(),
}))

vi.mock('@sim/utils/helpers', () => ({
sleep: mockSleep,
}))
vi.mock('@/lib/auth/ban', () => ({
getActivelyBannedUserIds: mockGetActivelyBannedUserIds,
}))
Expand Down Expand Up @@ -248,6 +254,10 @@ describe('preprocessExecution logPreprocessingErrors option', () => {
})

describe('preprocessExecution suppressRetryableFailureLogs option', () => {
beforeEach(() => {
vi.clearAllMocks()
})

const baseOptions = {
workflowId: 'workflow-1',
userId: 'owner-1',
Expand All @@ -266,16 +276,21 @@ describe('preprocessExecution suppressRetryableFailureLogs option', () => {
}
}

/** Preprocessing only reaches `safeStart`/`safeCompleteWithError`, so the mock stands in for the full session. */
function asLoggingSession(session: ReturnType<typeof makeLoggingSession>): LoggingSession {
return session as unknown as LoggingSession
}

it('skips the failure row for a retryable infrastructure failure', async () => {
workflowAuthzMockFns.mockGetActiveWorkflowRecord.mockRejectedValueOnce(
workflowAuthzMockFns.mockGetActiveWorkflowRecord.mockRejectedValue(
Object.assign(new Error('write CONNECT_TIMEOUT'), { code: 'CONNECT_TIMEOUT' })
)
const loggingSession = makeLoggingSession()

const result = await preprocessExecution({
...baseOptions,
suppressRetryableFailureLogs: true,
loggingSession: loggingSession as any,
loggingSession: asLoggingSession(loggingSession),
})

expect(result).toMatchObject({
Expand All @@ -298,7 +313,7 @@ describe('preprocessExecution suppressRetryableFailureLogs option', () => {
const result = await preprocessExecution({
...baseOptions,
suppressRetryableFailureLogs: true,
loggingSession: loggingSession as any,
loggingSession: asLoggingSession(loggingSession),
})

expect(result).toMatchObject({
Expand All @@ -308,15 +323,32 @@ describe('preprocessExecution suppressRetryableFailureLogs option', () => {
expect(loggingSession.safeStart).toHaveBeenCalled()
})

it('retries the workflow fetch before surfacing a transient failure', async () => {
workflowAuthzMockFns.mockGetActiveWorkflowRecord.mockRejectedValue(
Object.assign(new Error('read ECONNRESET'), { code: 'ECONNRESET' })
)

const result = await preprocessExecution({
...baseOptions,
loggingSession: asLoggingSession(makeLoggingSession()),
})

expect(workflowAuthzMockFns.mockGetActiveWorkflowRecord).toHaveBeenCalledTimes(3)
expect(result).toMatchObject({
success: false,
error: { message: 'Internal error while fetching workflow', retryable: true },
})
})

it('records retryable failures when the option is absent', async () => {
workflowAuthzMockFns.mockGetActiveWorkflowRecord.mockRejectedValueOnce(
workflowAuthzMockFns.mockGetActiveWorkflowRecord.mockRejectedValue(
Object.assign(new Error('write CONNECT_TIMEOUT'), { code: 'CONNECT_TIMEOUT' })
)
const loggingSession = makeLoggingSession()

const result = await preprocessExecution({
...baseOptions,
loggingSession: loggingSession as any,
loggingSession: asLoggingSession(loggingSession),
})

expect(result).toMatchObject({
Expand Down
30 changes: 24 additions & 6 deletions apps/sim/lib/execution/preprocessing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import {
} from '@/lib/core/execution-limits/metrics'
import { RateLimiter } from '@/lib/core/rate-limiter/rate-limiter'
import type { SubscriptionPlan } from '@/lib/core/rate-limiter/types'
import { withDatabaseReadRetry } from '@/lib/db/read-retry'
import { LoggingSession, type SessionStartParams } from '@/lib/logs/execution/logging-session'
import type { CoreTriggerType } from '@/stores/logs/filters/types'

Expand Down Expand Up @@ -236,7 +237,9 @@ export async function preprocessExecution(
let workflowRecord: WorkflowRecord | null = prefetchedWorkflowRecord ?? null
if (!workflowRecord) {
try {
workflowRecord = await getActiveWorkflowRecord(workflowId)
workflowRecord = await withDatabaseReadRetry(() => getActiveWorkflowRecord(workflowId), {
label: 'getActiveWorkflowRecord',
})

if (!workflowRecord) {
logger.warn(`[${requestId}] Workflow not found: ${workflowId}`)
Expand Down Expand Up @@ -297,7 +300,9 @@ export async function preprocessExecution(
},
}
} else {
const activeWorkflow = await getActiveWorkflowRecord(workflowId)
const activeWorkflow = await withDatabaseReadRetry(() => getActiveWorkflowRecord(workflowId), {
label: 'getActiveWorkflowRecord',
})
if (!activeWorkflow) {
logger.warn(`[${requestId}] Workflow archived before execution started: ${workflowId}`)
return {
Expand Down Expand Up @@ -365,7 +370,10 @@ export async function preprocessExecution(
}

if (!actorUserId) {
billingAttribution = await resolveSystemBillingAttribution(workspaceId)
billingAttribution = await withDatabaseReadRetry(
() => resolveSystemBillingAttribution(workspaceId),
{ label: 'resolveSystemBillingAttribution' }
)
actorUserId = billingAttribution.actorUserId
logger.info(`[${requestId}] Using atomically resolved system actor and payer`, {
actorUserId,
Expand Down Expand Up @@ -402,7 +410,11 @@ export async function preprocessExecution(
}

if (!billingAttribution) {
billingAttribution = await resolveBillingAttribution({ actorUserId, workspaceId })
const attributionInput = { actorUserId, workspaceId }
billingAttribution = await withDatabaseReadRetry(
() => resolveBillingAttribution(attributionInput),
{ label: 'resolveBillingAttribution' }
)
}
} catch (error) {
logger.error(`[${requestId}] Error resolving billing attribution`, { error, workflowId })
Expand Down Expand Up @@ -487,7 +499,10 @@ export async function preprocessExecution(
banCandidateIds.push(userId)
}
try {
const bannedUserIds = await getActivelyBannedUserIds(banCandidateIds)
const bannedUserIds = await withDatabaseReadRetry(
() => getActivelyBannedUserIds(banCandidateIds),
{ label: 'getActivelyBannedUserIds' }
)
if (bannedUserIds.length > 0) {
logger.warn(`[${requestId}] Execution blocked: banned account`, {
workflowId,
Expand Down Expand Up @@ -558,7 +573,10 @@ export async function preprocessExecution(
if (skipUsageLimits) return { failure: null, snapshot: null }
let snapshot: UsageSnapshot | null = null
try {
const usageCheck = await checkAttributedUsageLimits(billingAttribution)
const usageCheck = await withDatabaseReadRetry(
() => checkAttributedUsageLimits(billingAttribution),
{ label: 'checkAttributedUsageLimits' }
)
snapshot = usageCheck.payerUsage
? {
...usageCheck.payerUsage,
Expand Down
3 changes: 2 additions & 1 deletion apps/sim/lib/workflows/executor/execution-core.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,8 @@ describe('executeWorkflowCore terminal finalization sequencing', () => {
loggingSession: loggingSession as any,
})

await Promise.resolve()
// setImmediate, not a fixed hop count: the assertion is about ordering, not how many microtasks precede the loads
await new Promise((resolve) => setImmediate(resolve))

expect(callOrder).toContain('load-workflow:start')
expect(callOrder).toContain('load-env:start')
Expand Down
35 changes: 25 additions & 10 deletions apps/sim/lib/workflows/executor/execution-core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
getTimeoutErrorMessage,
isTimeoutAbortReason,
} from '@/lib/core/execution-limits'
import { withDatabaseReadRetry } from '@/lib/db/read-retry'
import { getExecutionEnvironment } from '@/lib/environment/utils'
import { clearExecutionCancellation } from '@/lib/execution/cancellation'
import { warmLargeValueRefs } from '@/lib/execution/payloads/hydration'
Expand Down Expand Up @@ -381,7 +382,11 @@ export async function executeWorkflowCore(
options: ExecuteWorkflowCoreOptions
): Promise<ExecutionResult> {
const workspaceId = options.snapshot.metadata.workspaceId
const rows = workspaceId ? await getCustomBlockRowsForWorkspace(workspaceId) : []
const rows = workspaceId
? await withDatabaseReadRetry(() => getCustomBlockRowsForWorkspace(workspaceId), {
label: 'getCustomBlockRowsForWorkspace',
})
: []
return withCustomBlockOverlay(rows, () => executeWorkflowCoreImpl(options))
}

Expand Down Expand Up @@ -560,8 +565,11 @@ async function executeWorkflowCoreImpl(
}

const [workflowState, env] = await Promise.all([
loadWorkflowState(),
getExecutionEnvironment(personalEnvUserId, workspaceEnvUserId, providedWorkspaceId),
withDatabaseReadRetry(loadWorkflowState, { label: 'loadWorkflowState' }),
withDatabaseReadRetry(
() => getExecutionEnvironment(personalEnvUserId, workspaceEnvUserId, providedWorkspaceId),
{ label: 'getExecutionEnvironment' }
),
])

const { blocks, loops, parallels } = workflowState
Expand Down Expand Up @@ -847,12 +855,16 @@ async function executeWorkflowCoreImpl(
// stage (below) and the block-outputs stage (threaded into the executor).
// Stored rules are the source of truth; absence yields the disabled default
// with one indexed lookup and no masking cost for non-PII organizations.
const [row] = await db
.select({ orgSettings: organization.dataRetentionSettings })
.from(workspace)
.leftJoin(organization, eq(organization.id, workspace.organizationId))
.where(eq(workspace.id, providedWorkspaceId))
.limit(1)
const [row] = await withDatabaseReadRetry(
() =>
db
.select({ orgSettings: organization.dataRetentionSettings })
.from(workspace)
.leftJoin(organization, eq(organization.id, workspace.organizationId))
.where(eq(workspace.id, providedWorkspaceId))
.limit(1),
{ label: 'resolvePiiRedactionPolicy' }
)
const piiRedaction: EffectivePiiRedaction = resolveEffectivePiiRedaction({
orgSettings: row?.orgSettings,
workspaceId: providedWorkspaceId,
Expand Down Expand Up @@ -933,7 +945,10 @@ async function executeWorkflowCoreImpl(
(block) => block.id === resolvedTriggerBlockId
)
if (entryBlock && isRunMetadataEnabled(entryBlock)) {
const runIdentity = await resolveStartBlockRunIdentity(metadata.principal)
const runIdentity = await withDatabaseReadRetry(
() => resolveStartBlockRunIdentity(metadata.principal),
{ label: 'resolveStartBlockRunIdentity' }
)
startRunMetadata = {
...runIdentity,
workspaceId: providedWorkspaceId,
Expand Down
Loading