Skip to content

Commit 8c8ffc1

Browse files
fix(custom-blocks): drain child log finalization when the parent is cancelled
1 parent 891e02e commit 8c8ffc1

5 files changed

Lines changed: 152 additions & 2 deletions

File tree

apps/sim/executor/handlers/workflow/workflow-handler.test.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ const {
1818
mockGetCustomBlockAuthority,
1919
mockGetUserEmailById,
2020
mockAdmitCustomBlockChildExecution,
21+
mockTrackChildFinalization,
2122
mockBuildTraceSpans,
2223
mockSafeStart,
2324
mockSafeComplete,
@@ -33,6 +34,7 @@ const {
3334
mockGetCustomBlockAuthority: vi.fn(),
3435
mockGetUserEmailById: vi.fn(),
3536
mockAdmitCustomBlockChildExecution: vi.fn(),
37+
mockTrackChildFinalization: vi.fn(),
3638
mockBuildTraceSpans: vi.fn(),
3739
mockSafeStart: vi.fn(),
3840
mockSafeComplete: vi.fn(),
@@ -63,6 +65,7 @@ vi.mock('@/lib/logs/execution/trace-spans/trace-spans', () => ({
6365

6466
vi.mock('@/lib/workflows/custom-blocks/child-execution', () => ({
6567
admitCustomBlockChildExecution: mockAdmitCustomBlockChildExecution,
68+
trackChildFinalization: mockTrackChildFinalization,
6669
buildCustomBlockCorrelation: (params: Record<string, any>) =>
6770
params.invokerExecutionId
6871
? { source: 'custom_block', executionId: params.invokerExecutionId }
@@ -1247,6 +1250,24 @@ describe('WorkflowBlockHandler', () => {
12471250
expect(error.cause).toBeUndefined()
12481251
})
12491252

1253+
it('registers the finalization so a cancelled parent can still drain it', async () => {
1254+
await handler.execute(customBlockContext(), customBlock(), {})
1255+
1256+
expect(mockTrackChildFinalization).toHaveBeenCalledTimes(1)
1257+
const [invokerId, promise] = mockTrackChildFinalization.mock.calls[0]
1258+
expect(invokerId).toBe('parent-execution-id')
1259+
expect(promise).toBeInstanceOf(Promise)
1260+
})
1261+
1262+
it('registers the finalization on the failure path too', async () => {
1263+
mockExecutorExecute.mockRejectedValue(new Error('boom'))
1264+
1265+
await handler.execute(customBlockContext(), customBlock(), {}).catch(() => {})
1266+
1267+
expect(mockTrackChildFinalization).toHaveBeenCalledTimes(1)
1268+
expect(mockTrackChildFinalization.mock.calls[0][0]).toBe('parent-execution-id')
1269+
})
1270+
12501271
it('never leaks the source workflow name when the child returns success: false', async () => {
12511272
mockExecutorExecute.mockResolvedValue({
12521273
success: false,

apps/sim/executor/handlers/workflow/workflow-handler.ts

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
admitCustomBlockChildExecution,
1515
buildCustomBlockCorrelation,
1616
createChildCancellationSignal,
17+
trackChildFinalization,
1718
} from '@/lib/workflows/custom-blocks/child-execution'
1819
import { getCustomBlockAuthority } from '@/lib/workflows/custom-blocks/operations'
1920
import { extractInputFieldsFromBlocks } from '@/lib/workflows/input-format'
@@ -601,7 +602,10 @@ export class WorkflowBlockHandler implements BlockHandler {
601602
const duration = performance.now() - startTime
602603

603604
if (childSession && childSessionStarted) {
604-
await this.finalizeChildSession(childSession, executionResult, duration, childWorkflowInput)
605+
await this.trackFinalization(
606+
ctx,
607+
this.finalizeChildSession(childSession, executionResult, duration, childWorkflowInput)
608+
)
605609
childSessionFinalized = true
606610
}
607611

@@ -652,7 +656,7 @@ export class WorkflowBlockHandler implements BlockHandler {
652656
// The child's own log row records the real failure in the source workspace,
653657
// so the publisher sees what the consumer deliberately cannot.
654658
if (childSession && childSessionStarted && !childSessionFinalized) {
655-
await this.failChildSession(childSession, error)
659+
await this.trackFinalization(ctx, this.failChildSession(childSession, error))
656660
}
657661

658662
// A custom block is checked FIRST and unconditionally: errors this invocation
@@ -713,6 +717,22 @@ export class WorkflowBlockHandler implements BlockHandler {
713717
}
714718
}
715719

720+
/**
721+
* Awaits a child-session finalization while ALSO registering it against the
722+
* invoking run. A cancelled or timed-out parent engine returns without
723+
* draining its in-flight node promises, which would abandon this `await`
724+
* mid-write; the registration lets the invoking run's completion path finish
725+
* the durable write instead of leaving the child's row `running` for the
726+
* stale-execution reaper.
727+
*/
728+
private async trackFinalization(
729+
ctx: ExecutionContext,
730+
finalization: Promise<void>
731+
): Promise<void> {
732+
trackChildFinalization(ctx.executionId, finalization)
733+
await finalization
734+
}
735+
716736
/**
717737
* Completes a custom-block child's own logging session, so the publisher gets a
718738
* full run record — trace waterfall, duration, and ledger rows — in the source

apps/sim/lib/workflows/custom-blocks/child-execution.test.ts

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ import {
3232
buildCustomBlockCorrelation,
3333
CustomBlockAdmissionError,
3434
createChildCancellationSignal,
35+
trackChildFinalization,
36+
waitForChildFinalizations,
3537
} from '@/lib/workflows/custom-blocks/child-execution'
3638
import { isBoundarySafeError } from '@/executor/errors/boundary'
3739

@@ -242,3 +244,48 @@ describe('createChildCancellationSignal durable backstop', () => {
242244
expect(signal.aborted).toBe(false)
243245
})
244246
})
247+
248+
describe('child finalization tracking', () => {
249+
it('lets an invoking run await a finalization the engine abandoned', async () => {
250+
let resolveWrite: () => void = () => {}
251+
const durableWrite = new Promise<void>((resolve) => {
252+
resolveWrite = resolve
253+
})
254+
let finished = false
255+
const finalization = durableWrite.then(() => {
256+
finished = true
257+
})
258+
259+
trackChildFinalization('invoker-1', finalization)
260+
261+
// The engine returned without draining; the write is still in flight.
262+
expect(finished).toBe(false)
263+
264+
const drained = waitForChildFinalizations('invoker-1')
265+
resolveWrite()
266+
await drained
267+
268+
expect(finished).toBe(true)
269+
})
270+
271+
it('does not let a rejected finalization break the drain', async () => {
272+
trackChildFinalization('invoker-2', Promise.reject(new Error('db down')))
273+
274+
await expect(waitForChildFinalizations('invoker-2')).resolves.toBeUndefined()
275+
})
276+
277+
it('is a no-op for a run that registered nothing', async () => {
278+
await expect(waitForChildFinalizations('invoker-never')).resolves.toBeUndefined()
279+
await expect(waitForChildFinalizations(undefined)).resolves.toBeUndefined()
280+
})
281+
282+
it('releases its entry so a run that never drains cannot leak', async () => {
283+
const finalization = Promise.resolve()
284+
trackChildFinalization('invoker-3', finalization)
285+
await finalization
286+
await Promise.resolve()
287+
288+
// Draining after self-cleanup still resolves rather than hanging.
289+
await expect(waitForChildFinalizations('invoker-3')).resolves.toBeUndefined()
290+
})
291+
})

apps/sim/lib/workflows/custom-blocks/child-execution.ts

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,3 +142,60 @@ export async function createChildCancellationSignal(params: {
142142
},
143143
}
144144
}
145+
146+
/**
147+
* Child-session finalizations still in flight, keyed by the INVOKING run's
148+
* execution id.
149+
*
150+
* A cancelled or timed-out parent engine returns without draining its in-flight
151+
* node promises, so the custom-block handler's finalization would otherwise be
152+
* abandoned mid-write: the invoking run finishes, the worker tears down, and the
153+
* child's log row is left `running` until the stale-execution reaper sweeps it
154+
* minutes later. Registering here lets the run's own completion path await the
155+
* durable write instead of racing it.
156+
*
157+
* Only the immediate invoker is tracked. A finalization orphaned *below* another
158+
* abandoned child still falls back to the reaper.
159+
*/
160+
const pendingChildFinalizations = new Map<string, Set<Promise<unknown>>>()
161+
162+
/** Registers a child-session finalization against its invoking run. */
163+
export function trackChildFinalization(
164+
invokerExecutionId: string | undefined,
165+
promise: Promise<unknown>
166+
): void {
167+
if (!invokerExecutionId) return
168+
169+
let pending = pendingChildFinalizations.get(invokerExecutionId)
170+
if (!pending) {
171+
pending = new Set()
172+
pendingChildFinalizations.set(invokerExecutionId, pending)
173+
}
174+
pending.add(promise)
175+
176+
// Self-cleaning so an invoker that never drains cannot leak the entry. The
177+
// `catch` also marks the rejection handled — callers use `allSettled`.
178+
void promise
179+
.catch(() => {})
180+
.finally(() => {
181+
pending.delete(promise)
182+
if (pending.size === 0) pendingChildFinalizations.delete(invokerExecutionId)
183+
})
184+
}
185+
186+
/** Awaits every child-session finalization registered for this run. */
187+
export async function waitForChildFinalizations(
188+
invokerExecutionId: string | undefined
189+
): Promise<void> {
190+
if (!invokerExecutionId) return
191+
192+
const pending = pendingChildFinalizations.get(invokerExecutionId)
193+
if (!pending) return
194+
195+
// Re-check: a settling finalization can register nothing further, but a
196+
// nested child can still be added while we await.
197+
while (pending.size > 0) {
198+
await Promise.allSettled([...pending])
199+
}
200+
pendingChildFinalizations.delete(invokerExecutionId)
201+
}

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import { redactLargeValueRefsInValue } from '@/lib/logs/execution/pii-large-valu
2222
import { redactObjectStrings } from '@/lib/logs/execution/pii-redaction'
2323
import { buildTraceSpans } from '@/lib/logs/execution/trace-spans/trace-spans'
2424
import { getUserEmailById } from '@/lib/users/queries'
25+
import { waitForChildFinalizations } from '@/lib/workflows/custom-blocks/child-execution'
2526
import { getCustomBlockRowsForWorkspace } from '@/lib/workflows/custom-blocks/operations'
2627
import {
2728
loadDeployedWorkflowState,
@@ -375,6 +376,10 @@ async function executeWorkflowCoreImpl(
375376
while (pendingLifecycleCallbacks.size > 0) {
376377
await Promise.allSettled([...pendingLifecycleCallbacks])
377378
}
379+
// A custom block's child owns a separate log row whose terminal write the
380+
// engine does not drain on cancel/timeout — await it here so the row is not
381+
// left `running` for the reaper when this run finishes or the worker exits.
382+
await waitForChildFinalizations(executionId)
378383
}
379384

380385
try {

0 commit comments

Comments
 (0)