Skip to content

Commit a2a2633

Browse files
waleedlatif1claude
andcommitted
fix(execution): retry transient database failures during execution setup
A dropped Postgres connection during workflow execution setup killed the run permanently. The first read in preprocessing is the workflow fetch; an ECONNRESET there surfaced as "Internal error while fetching workflow", and because background executions run with maxAttempts 1 there was no retry. Route the read-only setup operations through the existing withDatabaseReadRetry helper so a dropped connection is retried in place, before any effect exists. The retried operations are all reads, so the rate-limit token debit and the concurrency reservation are never re-entered. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BYi7yz8qo98ziQWZmRpqb8
1 parent 568a539 commit a2a2633

5 files changed

Lines changed: 86 additions & 20 deletions

File tree

apps/sim/lib/db/read-retry.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,14 +38,20 @@ export function isTransientDatabaseReadError(error: unknown): boolean {
3838
* rebuild the query outside any transaction and must have no side effects or locks.
3939
* A failed connection cannot establish whether a write committed, so writes and
4040
* transactions must never use this helper.
41+
*
42+
* `label` names the read in the retry log line so callers that wrap several can tell which flapped.
4143
*/
42-
export async function withDatabaseReadRetry<T>(read: () => Promise<T>): Promise<T> {
44+
export async function withDatabaseReadRetry<T>(
45+
read: () => Promise<T>,
46+
options: { label?: string } = {}
47+
): Promise<T> {
4348
for (let attempt = 1; ; attempt++) {
4449
try {
4550
return await read()
4651
} catch (error) {
4752
if (attempt >= 3 || !isTransientDatabaseReadError(error)) throw error
4853
logger.warn('Retrying transient database read', {
54+
label: options.label,
4955
attempt,
5056
code: getPostgresErrorCode(error),
5157
})

apps/sim/lib/execution/preprocessing.test.ts

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,15 @@ import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest'
77
import { ADMISSION_ERROR_CODE } from '@/lib/core/admission/transient-failure'
88

99
const {
10+
mockSleep,
1011
mockCheckAttributedUsageLimits,
1112
mockCheckRateLimit,
1213
mockGetActivelyBannedUserIds,
1314
mockReserveExecutionSlot,
1415
mockResolveBillingAttribution,
1516
mockResolveSystemBillingAttribution,
1617
} = vi.hoisted(() => ({
18+
mockSleep: vi.fn().mockResolvedValue(undefined),
1719
mockCheckAttributedUsageLimits: vi.fn(),
1820
mockCheckRateLimit: vi.fn(),
1921
mockGetActivelyBannedUserIds: vi.fn().mockResolvedValue([]),
@@ -22,6 +24,9 @@ const {
2224
mockResolveSystemBillingAttribution: vi.fn(),
2325
}))
2426

27+
vi.mock('@sim/utils/helpers', () => ({
28+
sleep: mockSleep,
29+
}))
2530
vi.mock('@/lib/auth/ban', () => ({
2631
getActivelyBannedUserIds: mockGetActivelyBannedUserIds,
2732
}))
@@ -248,6 +253,10 @@ describe('preprocessExecution logPreprocessingErrors option', () => {
248253
})
249254

250255
describe('preprocessExecution suppressRetryableFailureLogs option', () => {
256+
beforeEach(() => {
257+
vi.clearAllMocks()
258+
})
259+
251260
const baseOptions = {
252261
workflowId: 'workflow-1',
253262
userId: 'owner-1',
@@ -267,7 +276,7 @@ describe('preprocessExecution suppressRetryableFailureLogs option', () => {
267276
}
268277

269278
it('skips the failure row for a retryable infrastructure failure', async () => {
270-
workflowAuthzMockFns.mockGetActiveWorkflowRecord.mockRejectedValueOnce(
279+
workflowAuthzMockFns.mockGetActiveWorkflowRecord.mockRejectedValue(
271280
Object.assign(new Error('write CONNECT_TIMEOUT'), { code: 'CONNECT_TIMEOUT' })
272281
)
273282
const loggingSession = makeLoggingSession()
@@ -308,8 +317,25 @@ describe('preprocessExecution suppressRetryableFailureLogs option', () => {
308317
expect(loggingSession.safeStart).toHaveBeenCalled()
309318
})
310319

320+
it('retries the workflow fetch before surfacing a transient failure', async () => {
321+
workflowAuthzMockFns.mockGetActiveWorkflowRecord.mockRejectedValue(
322+
Object.assign(new Error('read ECONNRESET'), { code: 'ECONNRESET' })
323+
)
324+
325+
const result = await preprocessExecution({
326+
...baseOptions,
327+
loggingSession: makeLoggingSession() as any,
328+
})
329+
330+
expect(workflowAuthzMockFns.mockGetActiveWorkflowRecord).toHaveBeenCalledTimes(3)
331+
expect(result).toMatchObject({
332+
success: false,
333+
error: { message: 'Internal error while fetching workflow', retryable: true },
334+
})
335+
})
336+
311337
it('records retryable failures when the option is absent', async () => {
312-
workflowAuthzMockFns.mockGetActiveWorkflowRecord.mockRejectedValueOnce(
338+
workflowAuthzMockFns.mockGetActiveWorkflowRecord.mockRejectedValue(
313339
Object.assign(new Error('write CONNECT_TIMEOUT'), { code: 'CONNECT_TIMEOUT' })
314340
)
315341
const loggingSession = makeLoggingSession()

apps/sim/lib/execution/preprocessing.ts

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import {
3535
} from '@/lib/core/execution-limits/metrics'
3636
import { RateLimiter } from '@/lib/core/rate-limiter/rate-limiter'
3737
import type { SubscriptionPlan } from '@/lib/core/rate-limiter/types'
38+
import { withDatabaseReadRetry } from '@/lib/db/read-retry'
3839
import { LoggingSession, type SessionStartParams } from '@/lib/logs/execution/logging-session'
3940
import type { CoreTriggerType } from '@/stores/logs/filters/types'
4041

@@ -236,7 +237,9 @@ export async function preprocessExecution(
236237
let workflowRecord: WorkflowRecord | null = prefetchedWorkflowRecord ?? null
237238
if (!workflowRecord) {
238239
try {
239-
workflowRecord = await getActiveWorkflowRecord(workflowId)
240+
workflowRecord = await withDatabaseReadRetry(() => getActiveWorkflowRecord(workflowId), {
241+
label: 'getActiveWorkflowRecord',
242+
})
240243

241244
if (!workflowRecord) {
242245
logger.warn(`[${requestId}] Workflow not found: ${workflowId}`)
@@ -297,7 +300,9 @@ export async function preprocessExecution(
297300
},
298301
}
299302
} else {
300-
const activeWorkflow = await getActiveWorkflowRecord(workflowId)
303+
const activeWorkflow = await withDatabaseReadRetry(() => getActiveWorkflowRecord(workflowId), {
304+
label: 'getActiveWorkflowRecord',
305+
})
301306
if (!activeWorkflow) {
302307
logger.warn(`[${requestId}] Workflow archived before execution started: ${workflowId}`)
303308
return {
@@ -365,7 +370,10 @@ export async function preprocessExecution(
365370
}
366371

367372
if (!actorUserId) {
368-
billingAttribution = await resolveSystemBillingAttribution(workspaceId)
373+
billingAttribution = await withDatabaseReadRetry(
374+
() => resolveSystemBillingAttribution(workspaceId),
375+
{ label: 'resolveSystemBillingAttribution' }
376+
)
369377
actorUserId = billingAttribution.actorUserId
370378
logger.info(`[${requestId}] Using atomically resolved system actor and payer`, {
371379
actorUserId,
@@ -402,7 +410,11 @@ export async function preprocessExecution(
402410
}
403411

404412
if (!billingAttribution) {
405-
billingAttribution = await resolveBillingAttribution({ actorUserId, workspaceId })
413+
const attributionInput = { actorUserId, workspaceId }
414+
billingAttribution = await withDatabaseReadRetry(
415+
() => resolveBillingAttribution(attributionInput),
416+
{ label: 'resolveBillingAttribution' }
417+
)
406418
}
407419
} catch (error) {
408420
logger.error(`[${requestId}] Error resolving billing attribution`, { error, workflowId })
@@ -487,7 +499,10 @@ export async function preprocessExecution(
487499
banCandidateIds.push(userId)
488500
}
489501
try {
490-
const bannedUserIds = await getActivelyBannedUserIds(banCandidateIds)
502+
const bannedUserIds = await withDatabaseReadRetry(
503+
() => getActivelyBannedUserIds(banCandidateIds),
504+
{ label: 'getActivelyBannedUserIds' }
505+
)
491506
if (bannedUserIds.length > 0) {
492507
logger.warn(`[${requestId}] Execution blocked: banned account`, {
493508
workflowId,
@@ -558,7 +573,10 @@ export async function preprocessExecution(
558573
if (skipUsageLimits) return { failure: null, snapshot: null }
559574
let snapshot: UsageSnapshot | null = null
560575
try {
561-
const usageCheck = await checkAttributedUsageLimits(billingAttribution)
576+
const usageCheck = await withDatabaseReadRetry(
577+
() => checkAttributedUsageLimits(billingAttribution),
578+
{ label: 'checkAttributedUsageLimits' }
579+
)
562580
snapshot = usageCheck.payerUsage
563581
? {
564582
...usageCheck.payerUsage,

apps/sim/lib/workflows/executor/execution-core.test.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -318,7 +318,8 @@ describe('executeWorkflowCore terminal finalization sequencing', () => {
318318
loggingSession: loggingSession as any,
319319
})
320320

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

323324
expect(callOrder).toContain('load-workflow:start')
324325
expect(callOrder).toContain('load-env:start')

apps/sim/lib/workflows/executor/execution-core.ts

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import {
1919
getTimeoutErrorMessage,
2020
isTimeoutAbortReason,
2121
} from '@/lib/core/execution-limits'
22+
import { withDatabaseReadRetry } from '@/lib/db/read-retry'
2223
import { getExecutionEnvironment } from '@/lib/environment/utils'
2324
import { clearExecutionCancellation } from '@/lib/execution/cancellation'
2425
import { warmLargeValueRefs } from '@/lib/execution/payloads/hydration'
@@ -381,7 +382,11 @@ export async function executeWorkflowCore(
381382
options: ExecuteWorkflowCoreOptions
382383
): Promise<ExecutionResult> {
383384
const workspaceId = options.snapshot.metadata.workspaceId
384-
const rows = workspaceId ? await getCustomBlockRowsForWorkspace(workspaceId) : []
385+
const rows = workspaceId
386+
? await withDatabaseReadRetry(() => getCustomBlockRowsForWorkspace(workspaceId), {
387+
label: 'getCustomBlockRowsForWorkspace',
388+
})
389+
: []
385390
return withCustomBlockOverlay(rows, () => executeWorkflowCoreImpl(options))
386391
}
387392

@@ -560,8 +565,11 @@ async function executeWorkflowCoreImpl(
560565
}
561566

562567
const [workflowState, env] = await Promise.all([
563-
loadWorkflowState(),
564-
getExecutionEnvironment(personalEnvUserId, workspaceEnvUserId, providedWorkspaceId),
568+
withDatabaseReadRetry(loadWorkflowState, { label: 'loadWorkflowState' }),
569+
withDatabaseReadRetry(
570+
() => getExecutionEnvironment(personalEnvUserId, workspaceEnvUserId, providedWorkspaceId),
571+
{ label: 'getExecutionEnvironment' }
572+
),
565573
])
566574

567575
const { blocks, loops, parallels } = workflowState
@@ -847,12 +855,16 @@ async function executeWorkflowCoreImpl(
847855
// stage (below) and the block-outputs stage (threaded into the executor).
848856
// Stored rules are the source of truth; absence yields the disabled default
849857
// with one indexed lookup and no masking cost for non-PII organizations.
850-
const [row] = await db
851-
.select({ orgSettings: organization.dataRetentionSettings })
852-
.from(workspace)
853-
.leftJoin(organization, eq(organization.id, workspace.organizationId))
854-
.where(eq(workspace.id, providedWorkspaceId))
855-
.limit(1)
858+
const [row] = await withDatabaseReadRetry(
859+
() =>
860+
db
861+
.select({ orgSettings: organization.dataRetentionSettings })
862+
.from(workspace)
863+
.leftJoin(organization, eq(organization.id, workspace.organizationId))
864+
.where(eq(workspace.id, providedWorkspaceId))
865+
.limit(1),
866+
{ label: 'resolvePiiRedactionPolicy' }
867+
)
856868
const piiRedaction: EffectivePiiRedaction = resolveEffectivePiiRedaction({
857869
orgSettings: row?.orgSettings,
858870
workspaceId: providedWorkspaceId,
@@ -933,7 +945,10 @@ async function executeWorkflowCoreImpl(
933945
(block) => block.id === resolvedTriggerBlockId
934946
)
935947
if (entryBlock && isRunMetadataEnabled(entryBlock)) {
936-
const runIdentity = await resolveStartBlockRunIdentity(metadata.principal)
948+
const runIdentity = await withDatabaseReadRetry(
949+
() => resolveStartBlockRunIdentity(metadata.principal),
950+
{ label: 'resolveStartBlockRunIdentity' }
951+
)
937952
startRunMetadata = {
938953
...runIdentity,
939954
workspaceId: providedWorkspaceId,

0 commit comments

Comments
 (0)