fix(execution): complete async and resume jobs on marked workflow user failures - #8380
waleedlatif1 wants to merge 1 commit into
Conversation
…r 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.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
There was a problem hiding this comment.
3 issues found across 24 files
Confidence score: 3/5
apps/sim/tools/index.tsclassifies remote-sandboxprovider_limitinfrastructure failures as workflow-user failures, which can report infrastructure failures under the wrong category — preserve the provider-limit infrastructure classification.apps/sim/background/resume-execution.tsreturns before writing the terminal cell state on handled workflow failures, leaving table cells and groups stuck in their pre-resume state — write the cell terminal error state before returning.apps/sim/executor/utils/errors.tsstops checking causes after ten links, so deeply nested child-workflow failures can be misclassified asjob_faultand trigger paging instead of completing as workflow failures — ensure the marker is found through deeper cause chains.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/sim/tools/index.ts">
<violation number="1" location="apps/sim/tools/index.ts:2805">
P1: This status check also marks remote-sandbox provider-limit failures as workflow-user failures. The sandbox contract identifies `providerFailure: 'provider_limit'` as an infrastructure outcome, but the Function route returns those errors as 422, causing this code to suppress paging for a platform fault. Preserve the provider-failure signal through the route and exclude it from user-fault marking.</violation>
</file>
<file name="apps/sim/background/resume-execution.ts">
<violation number="1" location="apps/sim/background/resume-execution.ts:239">
P1: This handled failure returns before the cell-context terminal writer runs. Marked workflow failures in table cells will therefore leave the cell/group stuck in its pre-resume state; write the cell terminal error state before returning this workflow-failure result.</violation>
</file>
<file name="apps/sim/executor/utils/errors.ts">
<violation number="1" location="apps/sim/executor/utils/errors.ts:161">
P2: `isWorkflowUserFailure` stops after `findCause` examines ten error links. A deeply nested child-workflow failure can therefore lose its marker and be classified as `job_fault`, paging instead of completing as a workflow failure; use a cycle-safe walk with a depth appropriate for the supported nesting limit.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
| { status: response.status, statusText: response.statusText, data: errorData }, | ||
| tool.errorExtractor | ||
| ) | ||
| throw isFunctionOperation && response.status === FUNCTION_USER_CODE_FAILURE_STATUS |
There was a problem hiding this comment.
P1: This status check also marks remote-sandbox provider-limit failures as workflow-user failures. The sandbox contract identifies providerFailure: 'provider_limit' as an infrastructure outcome, but the Function route returns those errors as 422, causing this code to suppress paging for a platform fault. Preserve the provider-failure signal through the route and exclude it from user-fault marking.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/sim/tools/index.ts, line 2805:
<comment>This status check also marks remote-sandbox provider-limit failures as workflow-user failures. The sandbox contract identifies `providerFailure: 'provider_limit'` as an infrastructure outcome, but the Function route returns those errors as 422, causing this code to suppress paging for a platform fault. Preserve the provider-failure signal through the route and exclude it from user-fault marking.</comment>
<file context>
@@ -2790,10 +2798,13 @@ async function executeDeclaredInternalOperation({
{ status: response.status, statusText: response.statusText, data: errorData },
tool.errorExtractor
)
+ throw isFunctionOperation && response.status === FUNCTION_USER_CODE_FAILURE_STATUS
+ ? markWorkflowUserFailure(error)
+ : error
</file context>
| ) | ||
| // The resumed run executes under its parent's id, and the manager settles | ||
| // its post-execution work before re-throwing. | ||
| if (classifySettledWorkflowJobFailure(error, parentExecutionId) === 'workflow_failure') { |
There was a problem hiding this comment.
P1: This handled failure returns before the cell-context terminal writer runs. Marked workflow failures in table cells will therefore leave the cell/group stuck in its pre-resume state; write the cell terminal error state before returning this workflow-failure result.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/sim/background/resume-execution.ts, line 239:
<comment>This handled failure returns before the cell-context terminal writer runs. Marked workflow failures in table cells will therefore leave the cell/group stuck in its pre-resume state; write the cell terminal error state before returning this workflow-failure result.</comment>
<file context>
@@ -230,6 +234,15 @@ export async function executeResumeJob(payload: ResumeExecutionPayload, signal?:
)
+ // The resumed run executes under its parent's id, and the manager settles
+ // its post-execution work before re-throwing.
+ if (classifySettledWorkflowJobFailure(error, parentExecutionId) === 'workflow_failure') {
+ return {
+ ...buildWorkflowJobFailureResult({ error, workflowId, executionId: resumeExecutionId }),
</file context>
| */ | ||
| export function isWorkflowUserFailure(error: unknown): boolean { | ||
| return ( | ||
| findCause( |
There was a problem hiding this comment.
P2: isWorkflowUserFailure stops after findCause examines ten error links. A deeply nested child-workflow failure can therefore lose its marker and be classified as job_fault, paging instead of completing as a workflow failure; use a cycle-safe walk with a depth appropriate for the supported nesting limit.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/sim/executor/utils/errors.ts, line 161:
<comment>`isWorkflowUserFailure` stops after `findCause` examines ten error links. A deeply nested child-workflow failure can therefore lose its marker and be classified as `job_fault`, paging instead of completing as a workflow failure; use a cycle-safe walk with a depth appropriate for the supported nesting limit.</comment>
<file context>
@@ -141,6 +141,33 @@ function isRecordedThrown(value: unknown): value is object {
+ */
+export function isWorkflowUserFailure(error: unknown): boolean {
+ return (
+ findCause(
+ error,
+ (value): value is Error =>
</file context>
|
| throw isFunctionOperation && response.status === FUNCTION_USER_CODE_FAILURE_STATUS | ||
| ? markWorkflowUserFailure(error) | ||
| : error |
There was a problem hiding this comment.
Sandbox failures marked as user errors
When a Function block requests sandbox files on a deployment without a remote sandbox, the Function route returns 422 because the filesystem is unavailable. This check marks that response as a user-code failure. Once core records it, the queue job completes instead of faulting and alerting engineering. A 422 needs to be identified as a user runtime or compile error before it receives this mark.
Knowledge Base Used: Agent execution and sandbox tasks
| expect(loggingSessionMockFns.mockWaitForPostExecution).toHaveBeenCalled() | ||
| expect(mockWasExecutionFinalizedByCore).toHaveBeenCalledWith(rawError, 'execution-finalized') | ||
| expect(loggingSessionMockFns.mockSafeCompleteWithError).not.toHaveBeenCalled() |
There was a problem hiding this comment.
These assertions check whether mocked functions were called. The repository’s testing directive prohibits mock-call assertions; the new workflow-execution test also uses them. Please verify the resulting behavior instead. This requirement must be satisfied before merging.
Context Used: CLAUDE.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
|
Converting to draft: review found that the Function route's 422 is not a trustworthy user-code signal (isolated-vm bootstrap/runtime-binding failures, |
Summary
workflow-executionandresume-executionTrigger.dev tasks re-threw every execution error, so a user's own workflow failure (their function code raising, a condition expression that doesn't parse) failed the run and paged #eng-errors. The existing finalized-by-core guard inworkflow-executionalso ran before core had recorded the failure, so it never appliedsuccess: falseonly when core recorded the failure and it was positively marked as the workflow's own at its source (markWorkflowUserFailureinexecutor/utils/errors.ts):executeTool's throw→result flattening asToolResponse.workflowUserFailureWorkflowValidationErrorattributed to its blockcausechain; custom blocks deliberately cut the chain at their trust boundary, so those still fault/api/jobs/{id}and the queue-job status fallback project the completed failure result back tofailed, so pollers still see a failed runretryFailures: true(re-throwing would let a redelivery re-run a partly executed workflow) and schedule pages on nothing today, so paging by default there would add noiseType of Change
Testing
lib/sim-search/live/application.test.tsthat fail identically on current staging without this changeChecklist