Skip to content

Commit 4747afb

Browse files
waleedlatif1claude
andauthored
fix(execution): retry transient database failures during execution setup (#7681)
* 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 * test(execution): type the logging-session factory instead of casting Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BYi7yz8qo98ziQWZmRpqb8 --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent d2e1927 commit 4747afb

5 files changed

Lines changed: 95 additions & 23 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: 37 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,18 @@
55
import { loggingSessionMock, workflowAuthzMockFns } from '@sim/testing'
66
import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest'
77
import { ADMISSION_ERROR_CODE } from '@/lib/core/admission/transient-failure'
8+
import type { LoggingSession } from '@/lib/logs/execution/logging-session'
89

910
const {
11+
mockSleep,
1012
mockCheckAttributedUsageLimits,
1113
mockCheckRateLimit,
1214
mockGetActivelyBannedUserIds,
1315
mockReserveExecutionSlot,
1416
mockResolveBillingAttribution,
1517
mockResolveSystemBillingAttribution,
1618
} = vi.hoisted(() => ({
19+
mockSleep: vi.fn().mockResolvedValue(undefined),
1720
mockCheckAttributedUsageLimits: vi.fn(),
1821
mockCheckRateLimit: vi.fn(),
1922
mockGetActivelyBannedUserIds: vi.fn().mockResolvedValue([]),
@@ -22,6 +25,9 @@ const {
2225
mockResolveSystemBillingAttribution: vi.fn(),
2326
}))
2427

28+
vi.mock('@sim/utils/helpers', () => ({
29+
sleep: mockSleep,
30+
}))
2531
vi.mock('@/lib/auth/ban', () => ({
2632
getActivelyBannedUserIds: mockGetActivelyBannedUserIds,
2733
}))
@@ -248,6 +254,10 @@ describe('preprocessExecution logPreprocessingErrors option', () => {
248254
})
249255

250256
describe('preprocessExecution suppressRetryableFailureLogs option', () => {
257+
beforeEach(() => {
258+
vi.clearAllMocks()
259+
})
260+
251261
const baseOptions = {
252262
workflowId: 'workflow-1',
253263
userId: 'owner-1',
@@ -266,16 +276,21 @@ describe('preprocessExecution suppressRetryableFailureLogs option', () => {
266276
}
267277
}
268278

279+
/** Preprocessing only reaches `safeStart`/`safeCompleteWithError`, so the mock stands in for the full session. */
280+
function asLoggingSession(session: ReturnType<typeof makeLoggingSession>): LoggingSession {
281+
return session as unknown as LoggingSession
282+
}
283+
269284
it('skips the failure row for a retryable infrastructure failure', async () => {
270-
workflowAuthzMockFns.mockGetActiveWorkflowRecord.mockRejectedValueOnce(
285+
workflowAuthzMockFns.mockGetActiveWorkflowRecord.mockRejectedValue(
271286
Object.assign(new Error('write CONNECT_TIMEOUT'), { code: 'CONNECT_TIMEOUT' })
272287
)
273288
const loggingSession = makeLoggingSession()
274289

275290
const result = await preprocessExecution({
276291
...baseOptions,
277292
suppressRetryableFailureLogs: true,
278-
loggingSession: loggingSession as any,
293+
loggingSession: asLoggingSession(loggingSession),
279294
})
280295

281296
expect(result).toMatchObject({
@@ -298,7 +313,7 @@ describe('preprocessExecution suppressRetryableFailureLogs option', () => {
298313
const result = await preprocessExecution({
299314
...baseOptions,
300315
suppressRetryableFailureLogs: true,
301-
loggingSession: loggingSession as any,
316+
loggingSession: asLoggingSession(loggingSession),
302317
})
303318

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

326+
it('retries the workflow fetch before surfacing a transient failure', async () => {
327+
workflowAuthzMockFns.mockGetActiveWorkflowRecord.mockRejectedValue(
328+
Object.assign(new Error('read ECONNRESET'), { code: 'ECONNRESET' })
329+
)
330+
331+
const result = await preprocessExecution({
332+
...baseOptions,
333+
loggingSession: asLoggingSession(makeLoggingSession()),
334+
})
335+
336+
expect(workflowAuthzMockFns.mockGetActiveWorkflowRecord).toHaveBeenCalledTimes(3)
337+
expect(result).toMatchObject({
338+
success: false,
339+
error: { message: 'Internal error while fetching workflow', retryable: true },
340+
})
341+
})
342+
311343
it('records retryable failures when the option is absent', async () => {
312-
workflowAuthzMockFns.mockGetActiveWorkflowRecord.mockRejectedValueOnce(
344+
workflowAuthzMockFns.mockGetActiveWorkflowRecord.mockRejectedValue(
313345
Object.assign(new Error('write CONNECT_TIMEOUT'), { code: 'CONNECT_TIMEOUT' })
314346
)
315347
const loggingSession = makeLoggingSession()
316348

317349
const result = await preprocessExecution({
318350
...baseOptions,
319-
loggingSession: loggingSession as any,
351+
loggingSession: asLoggingSession(loggingSession),
320352
})
321353

322354
expect(result).toMatchObject({

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)