Skip to content

Commit ccce75f

Browse files
committed
fix(tables): continue the cascade after a completed resume whose later step threw
A completed run's cell is completed, so its downstream workflow groups still start; the failure is still rethrown. A pause that cannot be saved now throws a stable message with the underlying error on cause, so API callers never see internal persistence details.
1 parent 6be9ba0 commit ccce75f

4 files changed

Lines changed: 51 additions & 15 deletions

File tree

‎apps/sim/background/resume-execution.ts‎

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -178,13 +178,36 @@ export async function executeResumeJob(payload: ResumeExecutionPayload, signal?:
178178
cellContext.rowId,
179179
parentExecutionId,
180180
async () => {
181+
let completedBeforeFailure = false
181182
const result = await runResumeAndCellTerminal(
182183
payload,
183184
pausedExecution,
184185
writers,
185186
attemptSignal,
186-
attemptTimeoutController
187-
)
187+
attemptTimeoutController,
188+
() => {
189+
completedBeforeFailure = true
190+
}
191+
).catch(async (error: unknown) => {
192+
/**
193+
* The run completed and only a later step of the attempt threw, so its
194+
* cell is completed: continue the cascade as a completed run would, and
195+
* still surface the failure.
196+
*/
197+
if (completedBeforeFailure) {
198+
await continueCascadeAfterResume(cellContext, billingAttribution, attemptSignal).catch(
199+
(cascadeError: unknown) => {
200+
logger.error(
201+
'Failed to continue the cascade after a completed resume',
202+
projectResolvedSecretDiagnosticError(cascadeError, undefined, {
203+
resumeExecutionId,
204+
})
205+
)
206+
}
207+
)
208+
}
209+
throw error
210+
})
188211
if (result.status === 'paused' || result.status === 'cancelled') return result
189212
await continueCascadeAfterResume(cellContext, billingAttribution, attemptSignal)
190213
return result
@@ -392,7 +415,8 @@ async function runResumeAndCellTerminal(
392415
pausedExecution: Awaited<ReturnType<typeof PauseResumeManager.getPausedExecutionById>>,
393416
writers: CellWriters,
394417
signal: AbortSignal | undefined,
395-
timeoutController: ReturnType<typeof createTimeoutAbortController>
418+
timeoutController: ReturnType<typeof createTimeoutAbortController>,
419+
onCompletedBeforeFailure?: () => void
396420
): Promise<Awaited<ReturnType<typeof PauseResumeManager.startResumeExecution>>> {
397421
if (!pausedExecution) throw new Error('Paused execution missing — already nulled by caller')
398422
const result = await PauseResumeManager.startResumeExecution({
@@ -403,7 +427,10 @@ async function runResumeAndCellTerminal(
403427
resumeInput: payload.resumeInput,
404428
userId: payload.userId,
405429
onBlockComplete: writers.cellOnBlockComplete,
406-
onAttemptFailed: (outcome, error) => writeFailedResumeCellTerminal(writers, outcome, error),
430+
onAttemptFailed: async (outcome, error) => {
431+
if (outcome === 'execution_completed') onCompletedBeforeFailure?.()
432+
await writeFailedResumeCellTerminal(writers, outcome, error)
433+
},
407434
abortSignal: signal,
408435
})
409436

‎apps/sim/background/resume-governed-subject.test.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,16 @@ describe('resuming a paused table cell', () => {
264264
executionId: 'parent-execution-1',
265265
error: null,
266266
})
267+
expect(mocks.runRowCascadeLoop).toHaveBeenCalledTimes(1)
268+
}, 20_000)
269+
270+
it('does not continue the cascade when the resume failed the execution', async () => {
271+
const runFailure = new Error('Block failed')
272+
failResume('execution_failed', runFailure)
273+
274+
await expect(executeResumeJob(PAYLOAD)).rejects.toBe(runFailure)
275+
276+
expect(mocks.runRowCascadeLoop).not.toHaveBeenCalled()
267277
}, 20_000)
268278

269279
it('puts the cell back to paused when the pause stayed resumable', async () => {

‎apps/sim/lib/workflows/executor/human-in-the-loop-manager.test.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -369,9 +369,10 @@ describe('what a failed resume did to its paused execution', () => {
369369
pauseRun({ snapshotSeed: createSnapshotSeed(), persistError: new Error('lock timeout') })
370370
const { outcomes, args } = argsReportingOutcomes()
371371

372-
await expect(PauseResumeManager.startResumeExecution(args)).rejects.toThrow(
373-
'Failed to persist pause state: lock timeout'
374-
)
372+
await expect(PauseResumeManager.startResumeExecution(args)).rejects.toMatchObject({
373+
message: 'Failed to persist pause state',
374+
cause: new Error('lock timeout'),
375+
})
375376
expect(outcomes).toEqual(['execution_failed'])
376377
})
377378

‎apps/sim/lib/workflows/executor/human-in-the-loop-manager.ts‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -927,17 +927,18 @@ export class PauseResumeManager {
927927
if (result.status === 'paused') {
928928
/**
929929
* A pause that cannot be saved fails the execution. Fail the log with the
930-
* reason, then throw so the attempt settles as failed below.
930+
* reason, then throw so the attempt settles as failed below. The thrown
931+
* message stays stable; the underlying error rides on `cause`.
931932
*/
932933
const effectiveExecutionId = result.metadata?.executionId ?? resumeExecutionId
933-
const failPause = async (message: string, cause?: unknown): Promise<never> => {
934+
const failPause = async (reason: string, cause?: unknown): Promise<never> => {
934935
await LoggingSession.markExecutionAsFailed(
935936
effectiveExecutionId,
936-
message,
937+
cause === undefined ? reason : `${reason}: ${toError(cause).message}`,
937938
undefined,
938939
pausedExecution.workflowId
939940
)
940-
throw new Error(message, { cause })
941+
throw new Error(reason, { cause })
941942
}
942943
if (!result.snapshotSeed) {
943944
await failPause('Missing snapshot seed for paused execution')
@@ -952,10 +953,7 @@ export class PauseResumeManager {
952953
executorUserId: result.metadata?.userId,
953954
})
954955
} catch (pauseError) {
955-
await failPause(
956-
`Failed to persist pause state: ${toError(pauseError).message}`,
957-
pauseError
958-
)
956+
await failPause('Failed to persist pause state', pauseError)
959957
}
960958
}
961959
} else {

0 commit comments

Comments
 (0)