Skip to content

Commit 11cd349

Browse files
committed
fix(execution): complete async and resume jobs on marked workflow user failures
The workflow-execution and resume-execution Trigger.dev tasks re-threw every execution error, so a user's own workflow failure (their function code raising, a condition expression that does not parse) failed the run and paged #eng-errors. The existing finalized-by-core guard in workflow-execution also ran before core had recorded the failure, so it never applied. Failures stay faults by default. A job completes with success: false only when core recorded the failure and it was positively marked as the workflow's own at its source (markWorkflowUserFailure): - the Function route's 422 for a runtime or compile error in user/custom-tool code, carried across executeTool's flattening as ToolResponse.workflowUserFailure - a condition expression that threw or that the sandbox could not parse - the HTTP block receiving a 4xx from the URL the workflow called - a missing required field, now a WorkflowValidationError attributed to its block Child workflows carry the mark through their cause chain. Anything unmarked, including database and other platform errors inside blocks, still faults. /api/jobs and the queue-job status fallback project the completed failure result back to failed. Webhook and schedule execution are unchanged.
1 parent 9cf2d31 commit 11cd349

24 files changed

Lines changed: 700 additions & 21 deletions

‎apps/sim/app/api/jobs/[jobId]/route.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { checkHybridAuth } from '@/lib/auth/hybrid'
88
import { getJobQueue } from '@/lib/core/async-jobs'
99
import { generateRequestId } from '@/lib/core/utils/request'
1010
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
11+
import { projectWorkflowJobOutcome } from '@/lib/workflows/executor/job-outcome'
1112
import { createErrorResponse } from '@/app/api/workflows/utils'
1213

1314
const logger = createLogger('TaskStatusAPI')
@@ -66,15 +67,16 @@ export const GET = withRouteHandler(
6667
return createErrorResponse('Access denied', 403)
6768
}
6869

70+
const outcome = projectWorkflowJobOutcome(job)
6971
const response: Record<string, unknown> = {
7072
success: true,
7173
taskId,
72-
status: job.status,
74+
status: outcome.status,
7375
metadata: job.metadata,
7476
}
7577

7678
if (job.output !== undefined) response.output = job.output
77-
if (job.error !== undefined) response.error = job.error
79+
if (outcome.error !== undefined) response.error = outcome.error
7880

7981
return NextResponse.json(response)
8082
} catch (error: unknown) {

‎apps/sim/background/async-preprocessing-correlation.test.ts‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -26,15 +26,13 @@ const {
2626
mockExecuteWorkflowCore,
2727
mockExecutionSnapshot,
2828
mockWasExecutionFinalizedByCore,
29-
mockHasExecutionResult,
3029
mockIsWorkflowTimedOut,
3130
mockGetScheduleTimeValues,
3231
mockGetSubBlockValue,
3332
} = vi.hoisted(() => ({
3433
mockExecuteWorkflowCore: vi.fn(),
3534
mockExecutionSnapshot: vi.fn(),
3635
mockWasExecutionFinalizedByCore: vi.fn(),
37-
mockHasExecutionResult: vi.fn(),
3836
mockIsWorkflowTimedOut: vi.fn(() => false),
3937
mockGetScheduleTimeValues: vi.fn(),
4038
mockGetSubBlockValue: vi.fn(),
@@ -74,10 +72,6 @@ vi.mock('@/executor/execution/snapshot', () => ({
7472
ExecutionSnapshot: mockExecutionSnapshot,
7573
}))
7674

77-
vi.mock('@/executor/utils/errors', () => ({
78-
hasExecutionResult: mockHasExecutionResult,
79-
}))
80-
8175
import { executeScheduleJob } from './schedule-execution'
8276
import { executeWorkflowJob } from './workflow-execution'
8377

@@ -125,7 +119,6 @@ const principal = {
125119
describe('async preprocessing correlation threading', () => {
126120
beforeEach(() => {
127121
mockWasExecutionFinalizedByCore.mockReturnValue(false)
128-
mockHasExecutionResult.mockReturnValue(false)
129122
mockIsWorkflowTimedOut.mockReturnValue(false)
130123
resetDbChainMock()
131124
dbChainMockFns.limit.mockResolvedValue([
@@ -379,7 +372,6 @@ describe('async preprocessing correlation threading', () => {
379372
executionTimeout: {},
380373
})
381374
mockExecuteWorkflowCore.mockRejectedValueOnce(rawError)
382-
mockHasExecutionResult.mockImplementation((error) => error === rawError)
383375
mockWasExecutionFinalizedByCore.mockReturnValue(true)
384376

385377
await expect(
@@ -395,7 +387,8 @@ describe('async preprocessing correlation threading', () => {
395387
})
396388
).rejects.toBe(rawError)
397389

398-
expect(loggingSessionMockFns.mockWaitForPostExecution).not.toHaveBeenCalled()
390+
// Core finalizes after throwing, so the task must settle that work before deciding.
391+
expect(loggingSessionMockFns.mockWaitForPostExecution).toHaveBeenCalled()
399392
expect(mockWasExecutionFinalizedByCore).toHaveBeenCalledWith(rawError, 'execution-finalized')
400393
expect(loggingSessionMockFns.mockSafeCompleteWithError).not.toHaveBeenCalled()
401394
})

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

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,13 @@ vi.mock('@/executor/execution/snapshot', () => ({
2929
}))
3030

3131
import { executeResumeJob, type ResumeExecutionPayload } from '@/background/resume-execution'
32+
import { buildBlockExecutionError, markWorkflowUserFailure } from '@/executor/utils/errors'
33+
import type { SerializedBlock } from '@/serializer/types'
34+
35+
const planPanels = {
36+
id: 'plan-panels',
37+
metadata: { id: 'function', name: 'planPanels' },
38+
} as SerializedBlock
3239

3340
const { mockFindCellContextByExecutionId } = tableWorkflowColumnsMockFns
3441
const {
@@ -104,6 +111,39 @@ describe('executeResumeJob terminal errors', () => {
104111
expect(rawError.message).toContain(secret)
105112
})
106113

114+
it('completes the job when the resumed workflow failed with a recorded workflow user failure', async () => {
115+
const blockError = Object.assign(
116+
buildBlockExecutionError({
117+
block: planPanels,
118+
error: markWorkflowUserFailure(new Error("ValueError: kind ''")),
119+
}),
120+
{ executionFinalizedByCore: true }
121+
)
122+
mockStartResumeExecution.mockRejectedValue(blockError)
123+
124+
await expect(executeResumeJob(payload)).resolves.toMatchObject({
125+
success: false,
126+
workflowId: 'workflow-1',
127+
executionId: 'resume-execution-1',
128+
parentExecutionId: 'parent-execution-1',
129+
status: 'failed',
130+
error: blockError.message,
131+
})
132+
})
133+
134+
it('faults the job on an unmarked block failure even when core recorded it', async () => {
135+
const blockError = Object.assign(
136+
buildBlockExecutionError({
137+
block: planPanels,
138+
error: new TypeError('rows.flatMap is not a function'),
139+
}),
140+
{ executionFinalizedByCore: true }
141+
)
142+
mockStartResumeExecution.mockRejectedValue(blockError)
143+
144+
await expect(executeResumeJob(payload)).rejects.toBe(blockError)
145+
})
146+
107147
it('starts a legacy attempt deadline before deserializing the full snapshot', async () => {
108148
mockStartResumeExecution.mockResolvedValue({
109149
success: true,

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,10 @@ import {
2323
createResumeAttemptTimeoutController,
2424
PauseResumeManager,
2525
} from '@/lib/workflows/executor/human-in-the-loop-manager'
26+
import {
27+
buildWorkflowJobFailureResult,
28+
classifySettledWorkflowJobFailure,
29+
} from '@/lib/workflows/executor/job-failure'
2630
import { RESUME_EXECUTION_CONCURRENCY_LIMIT } from '@/background/concurrency-limits'
2731
import { ExecutionSnapshot } from '@/executor/execution/snapshot'
2832
import type { SerializedSnapshot } from '@/executor/types'
@@ -230,6 +234,15 @@ export async function executeResumeJob(payload: ResumeExecutionPayload, signal?:
230234
workflowId,
231235
})
232236
)
237+
// The resumed run executes under its parent's id, and the manager settles
238+
// its post-execution work before re-throwing.
239+
if (classifySettledWorkflowJobFailure(error, parentExecutionId) === 'workflow_failure') {
240+
return {
241+
...buildWorkflowJobFailureResult({ error, workflowId, executionId: resumeExecutionId }),
242+
parentExecutionId,
243+
status: 'failed' as const,
244+
}
245+
}
233246
throw error
234247
} finally {
235248
timeoutController?.cleanup()
Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import {
5+
executionPreprocessingMock,
6+
executionPreprocessingMockFns,
7+
LoggingSessionMock,
8+
loggingSessionMock,
9+
loggingSessionMockFns,
10+
} from '@sim/testing'
11+
import {
12+
executionLimitsMock,
13+
executionLimitsMockFns,
14+
} from '@sim/testing/mocks/execution-limits.mock'
15+
import { beforeEach, describe, expect, it, vi } from 'vitest'
16+
17+
const { mockExecuteWorkflowCore, mockWasExecutionFinalizedByCore } = vi.hoisted(() => ({
18+
mockExecuteWorkflowCore: vi.fn(),
19+
mockWasExecutionFinalizedByCore: vi.fn(),
20+
}))
21+
22+
vi.mock('@/lib/execution/preprocessing', () => executionPreprocessingMock)
23+
vi.mock('@/lib/logs/execution/logging-session', () => loggingSessionMock)
24+
vi.mock('@/lib/core/execution-limits', () => executionLimitsMock)
25+
vi.mock('@/lib/workflows/executor/execution-core', () => ({
26+
executeWorkflowCore: mockExecuteWorkflowCore,
27+
wasExecutionFinalizedByCore: mockWasExecutionFinalizedByCore,
28+
}))
29+
vi.mock('@/lib/workflows/executor/pause-persistence', () => ({
30+
handlePostExecutionPauseState: vi.fn(),
31+
}))
32+
vi.mock('@/lib/logs/execution/trace-spans/trace-spans', () => ({
33+
buildTraceSpans: vi.fn(() => ({ traceSpans: [] })),
34+
}))
35+
vi.mock('@/executor/execution/snapshot', () => ({ ExecutionSnapshot: vi.fn() }))
36+
vi.mock('@/lib/uploads/utils/user-file-base64.server', () => ({
37+
cleanupExecutionBase64Cache: vi.fn(async () => {}),
38+
}))
39+
40+
import * as usageReservation from '@/lib/billing/calculations/usage-reservation'
41+
import { executeWorkflowJob, type WorkflowExecutionPayload } from '@/background/workflow-execution'
42+
import { buildBlockExecutionError, markWorkflowUserFailure } from '@/executor/utils/errors'
43+
import type { SerializedBlock } from '@/serializer/types'
44+
45+
const billingAttribution = {
46+
actorUserId: 'user-1',
47+
workspaceId: 'workspace-1',
48+
organizationId: null,
49+
billedAccountUserId: 'user-1',
50+
billingEntity: { type: 'user' as const, id: 'user-1' },
51+
billingPeriod: { start: '2026-09-01T00:00:00.000Z', end: '2026-10-01T00:00:00.000Z' },
52+
payerSubscription: null,
53+
}
54+
55+
const payload: WorkflowExecutionPayload = {
56+
workflowId: 'workflow-1',
57+
principal: {
58+
version: 1,
59+
principal: {
60+
kind: 'system',
61+
serviceId: 'internal',
62+
workspaceId: 'workspace-1',
63+
workflowId: 'workflow-1',
64+
},
65+
},
66+
userId: 'user-1',
67+
billingAttribution,
68+
workspaceId: 'workspace-1',
69+
executionId: 'execution-1',
70+
requestId: 'request-1',
71+
triggerType: 'api',
72+
}
73+
74+
const planPanels = {
75+
id: 'plan-panels',
76+
metadata: { id: 'function', name: 'planPanels' },
77+
} as SerializedBlock
78+
79+
describe('executeWorkflowJob fault vs workflow failure', () => {
80+
beforeEach(() => {
81+
vi.spyOn(usageReservation, 'refreshExecutionSlotExpiry').mockResolvedValue(true)
82+
vi.spyOn(usageReservation, 'releaseExecutionSlot').mockResolvedValue(undefined)
83+
executionLimitsMockFns.mockCreateTimeoutAbortController.mockImplementation(() => ({
84+
signal: new AbortController().signal,
85+
cleanup: vi.fn(),
86+
abort: vi.fn(),
87+
isTimedOut: () => false,
88+
timeoutMs: 120_000,
89+
}))
90+
LoggingSessionMock.mockImplementation(function LoggingSession() {
91+
return {
92+
safeCompleteWithError: loggingSessionMockFns.mockSafeCompleteWithError,
93+
waitForPostExecution: loggingSessionMockFns.mockWaitForPostExecution,
94+
markAsFailed: loggingSessionMockFns.mockMarkAsFailed,
95+
setExecutionDeadlineAt: loggingSessionMockFns.mockSetExecutionDeadlineAt,
96+
projectDiagnosticError: loggingSessionMockFns.mockProjectDiagnosticError,
97+
}
98+
})
99+
executionPreprocessingMockFns.mockPreprocessExecution.mockResolvedValue({
100+
success: true,
101+
actorUserId: 'user-1',
102+
billingAttribution,
103+
workflowRecord: {
104+
id: 'workflow-1',
105+
workspaceId: 'workspace-1',
106+
userId: 'user-1',
107+
variables: {},
108+
},
109+
})
110+
})
111+
112+
it('completes the job when a workflow user failure is recorded only after core throws', async () => {
113+
const blockError = buildBlockExecutionError({
114+
block: planPanels,
115+
error: markWorkflowUserFailure(
116+
new Error("ValueError: Doctrine has no sheet layout for kind ''")
117+
),
118+
})
119+
let recorded = false
120+
mockExecuteWorkflowCore.mockRejectedValue(blockError)
121+
loggingSessionMockFns.mockWaitForPostExecution.mockImplementation(async () => {
122+
recorded = true
123+
})
124+
mockWasExecutionFinalizedByCore.mockImplementation(() => recorded)
125+
126+
const result = await executeWorkflowJob(payload)
127+
128+
expect(result).toMatchObject({
129+
success: false,
130+
workflowId: 'workflow-1',
131+
executionId: 'execution-1',
132+
error: blockError.message,
133+
})
134+
expect(loggingSessionMockFns.mockSafeCompleteWithError).not.toHaveBeenCalled()
135+
})
136+
137+
it('faults the job on an unmarked block failure even when core recorded it', async () => {
138+
const internalError = buildBlockExecutionError({
139+
block: planPanels,
140+
error: new Error('An internal error occurred while running this block'),
141+
})
142+
mockExecuteWorkflowCore.mockRejectedValue(internalError)
143+
mockWasExecutionFinalizedByCore.mockReturnValue(true)
144+
145+
await expect(executeWorkflowJob(payload)).rejects.toBe(internalError)
146+
expect(loggingSessionMockFns.mockSafeCompleteWithError).not.toHaveBeenCalled()
147+
})
148+
149+
it('faults the job when core never recorded the failure', async () => {
150+
const setupError = new Error('Workflow state not found')
151+
mockExecuteWorkflowCore.mockRejectedValue(setupError)
152+
mockWasExecutionFinalizedByCore.mockReturnValue(false)
153+
154+
await expect(executeWorkflowJob(payload)).rejects.toBe(setupError)
155+
expect(loggingSessionMockFns.mockSafeCompleteWithError).toHaveBeenCalled()
156+
})
157+
})

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,10 @@ import {
3434
executeWorkflowCore,
3535
wasExecutionFinalizedByCore,
3636
} from '@/lib/workflows/executor/execution-core'
37+
import {
38+
buildWorkflowJobFailureResult,
39+
classifyWorkflowJobFailure,
40+
} from '@/lib/workflows/executor/job-failure'
3741
import { handlePostExecutionPauseState } from '@/lib/workflows/executor/pause-persistence'
3842
import { WORKFLOW_EXECUTION_CONCURRENCY_LIMIT } from '@/background/concurrency-limits'
3943
import { ExecutionSnapshot } from '@/executor/execution/snapshot'
@@ -313,6 +317,13 @@ export async function executeWorkflowJob(
313317

314318
if (error instanceof ExecutionTimeoutError) throw error
315319

320+
const failure = await classifyWorkflowJobFailure({ error, executionId, loggingSession })
321+
if (failure === 'workflow_failure') {
322+
return {
323+
...buildWorkflowJobFailureResult({ error, workflowId, executionId }),
324+
metadata: payload.metadata,
325+
}
326+
}
316327
if (wasExecutionFinalizedByCore(error, executionId)) {
317328
throw error
318329
}

‎apps/sim/executor/constants.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,7 @@ export const DEFAULTS = {
216216
export const HTTP = {
217217
STATUS: {
218218
OK: 200,
219+
BAD_REQUEST: 400,
219220
FORBIDDEN: 403,
220221
NOT_FOUND: 404,
221222
TOO_MANY_REQUESTS: 429,

‎apps/sim/executor/handlers/api/api-handler.test.ts‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { beforeEach, describe, expect, it, type Mock, vi } from 'vitest'
55
import { BlockType } from '@/executor/constants'
66
import { ApiBlockHandler } from '@/executor/handlers/api/api-handler'
77
import type { ExecutionContext } from '@/executor/types'
8+
import { isWorkflowUserFailure } from '@/executor/utils/errors'
89
import type { SerializedBlock } from '@/serializer/types'
910
import { executeTool } from '@/tools'
1011
import type { ToolConfig } from '@/tools/types'
@@ -125,4 +126,22 @@ describe('ApiBlockHandler', () => {
125126
)
126127
expect(mockExecuteTool).toHaveBeenCalled()
127128
})
129+
130+
it.each([
131+
{ output: { status: 400, statusText: 'Bad Request' }, expected: true },
132+
{ output: { status: 503, statusText: 'Service Unavailable' }, expected: false },
133+
{ output: {}, expected: false },
134+
])(
135+
'marks the failure as the workflow user failure only for a 4xx from the requested URL ($output.status)',
136+
async ({ output, expected }) => {
137+
mockExecuteTool.mockResolvedValue({ success: false, output, error: 'Request failed' })
138+
139+
const thrown = await handler
140+
.execute(mockContext, mockBlock, { url: 'https://example.com/hook', method: 'POST' })
141+
.catch((error: unknown) => error)
142+
143+
expect(thrown).toBeInstanceOf(Error)
144+
expect(isWorkflowUserFailure(thrown)).toBe(expected)
145+
}
146+
)
128147
})

0 commit comments

Comments
 (0)