Skip to content

Commit 693340f

Browse files
committed
feat(executor): opt-in per-block retry for transient failures
1 parent 2ba2484 commit 693340f

8 files changed

Lines changed: 565 additions & 3 deletions

File tree

Lines changed: 191 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,191 @@
1+
/**
2+
* @vitest-environment node
3+
*
4+
* Retry wraps only the handler invocation, so a replay cannot duplicate output the
5+
* client has already seen and cannot re-run the deterministic post-processing.
6+
*/
7+
import { beforeEach, describe, expect, it, vi } from 'vitest'
8+
import { BlockType, EDGE } from '@/executor/constants'
9+
import type { DAGNode } from '@/executor/dag/builder'
10+
import { BlockExecutor } from '@/executor/execution/block-executor'
11+
import { ExecutionState } from '@/executor/execution/state'
12+
import type { BlockHandler, ExecutionContext } from '@/executor/types'
13+
import { VariableResolver } from '@/executor/variables/resolver'
14+
import type { SerializedBlock, SerializedWorkflow } from '@/serializer/types'
15+
16+
vi.mock('@/ee/access-control/utils/permission-check', () => ({
17+
validateBlockType: vi.fn(),
18+
}))
19+
20+
function createBlock(retry?: SerializedBlock['retry']): SerializedBlock {
21+
return {
22+
id: 'slack-block-1',
23+
metadata: { id: BlockType.FUNCTION, name: 'Post' },
24+
position: { x: 0, y: 0 },
25+
config: { tool: BlockType.FUNCTION, params: {} },
26+
inputs: {},
27+
outputs: {},
28+
enabled: true,
29+
...(retry ? { retry } : {}),
30+
}
31+
}
32+
33+
function createContext(state: ExecutionState, abortSignal?: AbortSignal): ExecutionContext {
34+
return {
35+
workflowId: 'workflow-1',
36+
workspaceId: 'workspace-1',
37+
executionId: 'execution-1',
38+
userId: 'user-1',
39+
blockStates: state.getBlockStates(),
40+
blockLogs: [],
41+
metadata: { requestId: 'request-1', duration: 0 },
42+
environmentVariables: {},
43+
workflowVariables: {},
44+
decisions: { router: new Map(), condition: new Map() },
45+
loopExecutions: new Map(),
46+
executedBlocks: new Set(),
47+
activeExecutionPath: new Set(),
48+
completedLoops: new Set(),
49+
abortSignal,
50+
} as ExecutionContext
51+
}
52+
53+
function createNode(block: SerializedBlock, withErrorPort = false): DAGNode {
54+
return {
55+
id: block.id,
56+
block,
57+
incomingEdges: new Set(),
58+
outgoingEdges: withErrorPort
59+
? new Map([['edge-1', { sourceHandle: EDGE.ERROR, target: 'downstream' }]])
60+
: new Map(),
61+
metadata: {},
62+
} as unknown as DAGNode
63+
}
64+
65+
function buildExecutor(block: SerializedBlock, handler: BlockHandler, state: ExecutionState) {
66+
const workflow: SerializedWorkflow = {
67+
version: '1',
68+
blocks: [block],
69+
connections: [],
70+
loops: {},
71+
parallels: {},
72+
}
73+
return new BlockExecutor(
74+
[handler],
75+
new VariableResolver(workflow, {}, state),
76+
{
77+
workspaceId: 'workspace-1',
78+
executionId: 'execution-1',
79+
userId: 'user-1',
80+
metadata: {
81+
requestId: 'request-1',
82+
executionId: 'execution-1',
83+
workflowId: 'workflow-1',
84+
workspaceId: 'workspace-1',
85+
userId: 'user-1',
86+
triggerType: 'manual',
87+
useDraftState: false,
88+
startTime: new Date().toISOString(),
89+
},
90+
},
91+
state
92+
)
93+
}
94+
95+
/** Bun's dropped-connection failure, the case that motivated this. */
96+
function socketClosed() {
97+
return new Error('The socket connection was closed unexpectedly.')
98+
}
99+
100+
describe('BlockExecutor retry', () => {
101+
beforeEach(() => vi.clearAllMocks())
102+
103+
it('does not retry when the builder has not opted in', async () => {
104+
const block = createBlock()
105+
const execute = vi.fn().mockRejectedValue(socketClosed())
106+
const state = new ExecutionState()
107+
const executor = buildExecutor(block, { canHandle: () => true, execute }, state)
108+
109+
await expect(executor.execute(createContext(state), createNode(block), block)).rejects.toThrow()
110+
expect(execute).toHaveBeenCalledTimes(1)
111+
})
112+
113+
it('replays a transient failure and succeeds on a later attempt', async () => {
114+
const block = createBlock({ maxAttempts: 3, waitMs: 0 })
115+
const execute = vi
116+
.fn()
117+
.mockRejectedValueOnce(socketClosed())
118+
.mockResolvedValueOnce({ ok: true })
119+
const state = new ExecutionState()
120+
const ctx = createContext(state)
121+
const executor = buildExecutor(block, { canHandle: () => true, execute }, state)
122+
123+
const output = await executor.execute(ctx, createNode(block), block)
124+
125+
expect(execute).toHaveBeenCalledTimes(2)
126+
expect(output).toMatchObject({ ok: true })
127+
expect(ctx.blockLogs[0]?.success).toBe(true)
128+
expect(ctx.blockLogs[0]?.attempts).toBe(2)
129+
})
130+
131+
it('stops at the configured attempt ceiling', async () => {
132+
const block = createBlock({ maxAttempts: 3, waitMs: 0 })
133+
const execute = vi.fn().mockRejectedValue(socketClosed())
134+
const state = new ExecutionState()
135+
const ctx = createContext(state)
136+
const executor = buildExecutor(block, { canHandle: () => true, execute }, state)
137+
138+
await expect(executor.execute(ctx, createNode(block), block)).rejects.toThrow()
139+
expect(execute).toHaveBeenCalledTimes(3)
140+
expect(ctx.blockLogs[0]?.attempts).toBe(3)
141+
})
142+
143+
/** A permanent failure must not spend the budget re-confirming itself. */
144+
it('does not replay a non-transient failure', async () => {
145+
const block = createBlock({ maxAttempts: 5, waitMs: 0 })
146+
const execute = vi.fn().mockRejectedValue(new Error('Invalid channel id'))
147+
const state = new ExecutionState()
148+
const executor = buildExecutor(block, { canHandle: () => true, execute }, state)
149+
150+
await expect(executor.execute(createContext(state), createNode(block), block)).rejects.toThrow(
151+
'Invalid channel id'
152+
)
153+
expect(execute).toHaveBeenCalledTimes(1)
154+
})
155+
156+
/** A run cancelled mid-flight must not start another attempt. */
157+
it('stops replaying once the run is cancelled', async () => {
158+
const block = createBlock({ maxAttempts: 5, waitMs: 0 })
159+
const controller = new AbortController()
160+
const execute = vi.fn().mockImplementation(async () => {
161+
controller.abort()
162+
throw socketClosed()
163+
})
164+
const state = new ExecutionState()
165+
const executor = buildExecutor(block, { canHandle: () => true, execute }, state)
166+
167+
await expect(
168+
executor.execute(createContext(state, controller.signal), createNode(block), block)
169+
).rejects.toThrow()
170+
expect(execute).toHaveBeenCalledTimes(1)
171+
})
172+
173+
/**
174+
* Retry and the error port compose: the port only sees the failure once the
175+
* attempt budget is spent, and the block still returns an error output rather
176+
* than throwing.
177+
*/
178+
it('hands an exhausted retry to the error port instead of throwing', async () => {
179+
const block = createBlock({ maxAttempts: 2, waitMs: 0 })
180+
const execute = vi.fn().mockRejectedValue(socketClosed())
181+
const state = new ExecutionState()
182+
const ctx = createContext(state)
183+
const executor = buildExecutor(block, { canHandle: () => true, execute }, state)
184+
185+
const output = await executor.execute(ctx, createNode(block, true), block)
186+
187+
expect(execute).toHaveBeenCalledTimes(2)
188+
expect(output.error).toContain('socket connection was closed')
189+
expect(ctx.blockLogs[0]?.errorHandled).toBe(true)
190+
})
191+
})

apps/sim/executor/execution/block-executor.ts

Lines changed: 65 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
import { createLogger, type Logger } from '@sim/logger'
22
import { toError } from '@sim/utils/errors'
3+
import { sleep } from '@sim/utils/helpers'
4+
import { backoffWithJitter } from '@sim/utils/retry'
35
import { isTimeoutAbortReason } from '@/lib/core/execution-limits/types'
46
import { redactApiKeys } from '@/lib/core/security/redaction'
57
import { normalizeStringArray } from '@/lib/core/utils/arrays'
@@ -25,6 +27,7 @@ import {
2527
} from '@/executor/constants'
2628
import type { DAGNode } from '@/executor/dag/builder'
2729
import { ChildWorkflowError } from '@/executor/errors/child-workflow-error'
30+
import { isRetryableBlockError, resolveBlockRetryPolicy } from '@/executor/execution/block-retry'
2831
import type {
2932
BlockStateWriter,
3033
ContextExtensions,
@@ -181,9 +184,20 @@ export class BlockExecutor {
181184

182185
let streamingPartialOutput: Record<string, any> | undefined
183186
try {
184-
const output = handler.executeWithNode
185-
? await handler.executeWithNode(ctx, block, resolvedInputs, nodeMetadata)
186-
: await handler.execute(ctx, block, resolvedInputs)
187+
/**
188+
* Only the handler call is retried, never the post-processing below it.
189+
*
190+
* For a streaming block the handler returns before any token is drained, so
191+
* a replay here cannot duplicate output the client has already seen — a
192+
* failure during the drain falls through to the catch untouched. The
193+
* redaction and compaction steps are deterministic and would fail again
194+
* identically, so replaying them would only burn the attempt budget.
195+
*/
196+
const output = await this.runHandlerWithRetry(ctx, node, block, blockLog, () =>
197+
handler.executeWithNode
198+
? handler.executeWithNode(ctx, block, resolvedInputs, nodeMetadata)
199+
: handler.execute(ctx, block, resolvedInputs)
200+
)
187201

188202
const isStreamingExecution =
189203
output && typeof output === 'object' && 'stream' in output && 'execution' in output
@@ -370,6 +384,54 @@ export class BlockExecutor {
370384
return this.blockHandlers.find((h) => h.canHandle(block))
371385
}
372386

387+
/**
388+
* Runs the block handler, replaying it while the failure looks transient.
389+
*
390+
* Returns the handler's value untouched on success, and rethrows the final
391+
* attempt's error on exhaustion so the caller's catch — and with it the error
392+
* port — behaves exactly as it does for a block that never retried.
393+
*/
394+
private async runHandlerWithRetry<T>(
395+
ctx: ExecutionContext,
396+
node: DAGNode,
397+
block: SerializedBlock,
398+
blockLog: BlockLog | undefined,
399+
invoke: () => Promise<T>
400+
): Promise<T> {
401+
const policy = resolveBlockRetryPolicy(block)
402+
if (!policy) return await invoke()
403+
404+
for (let attempt = 1; ; attempt++) {
405+
try {
406+
const output = await invoke()
407+
if (blockLog && attempt > 1) blockLog.attempts = attempt
408+
return output
409+
} catch (error) {
410+
const isFinalAttempt = attempt >= policy.maxAttempts
411+
/**
412+
* A run cancelled mid-backoff must not start another attempt, even when
413+
* the error itself looks retryable.
414+
*/
415+
const cancelled = ctx.abortSignal?.aborted === true
416+
if (isFinalAttempt || cancelled || !isRetryableBlockError(error)) {
417+
if (blockLog && attempt > 1) blockLog.attempts = attempt
418+
throw error
419+
}
420+
421+
const delayMs = backoffWithJitter(attempt, null, { baseMs: policy.waitMs })
422+
this.execLogger.warn('Block failed on a transient error; retrying', {
423+
blockId: node.id,
424+
blockType: block.metadata?.id,
425+
attempt,
426+
maxAttempts: policy.maxAttempts,
427+
delayMs,
428+
error: normalizeError(error),
429+
})
430+
await sleep(delayMs)
431+
}
432+
}
433+
}
434+
373435
private async handleBlockError(
374436
error: unknown,
375437
ctx: ExecutionContext,

0 commit comments

Comments
 (0)