Skip to content

Commit d5e6319

Browse files
committed
refactor(trigger): share the recorded-failure completion and skip the redundant error write on a timed-out recorded failure
1 parent 6431fc6 commit d5e6319

3 files changed

Lines changed: 17 additions & 19 deletions

File tree

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -426,6 +426,7 @@ describe('async preprocessing correlation threading', () => {
426426
mockWasExecutionFinalizedByCore.mockReturnValue(true)
427427

428428
await expect(runRecordedFailure('execution-recorded-deadline')).rejects.toBe(error)
429+
expect(loggingSessionMockFns.mockSafeCompleteWithError).not.toHaveBeenCalled()
429430
})
430431

431432
it('still fails the job when the run passed its deadline', async () => {
@@ -435,6 +436,7 @@ describe('async preprocessing correlation threading', () => {
435436
mockWasExecutionFinalizedByCore.mockReturnValue(true)
436437

437438
await expect(runRecordedFailure('execution-recorded-late')).rejects.toBe(error)
439+
expect(loggingSessionMockFns.mockSafeCompleteWithError).not.toHaveBeenCalled()
438440
})
439441
})
440442

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

Lines changed: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,13 @@ function isNonFaultPreprocessingRejection(rejection: PreprocessExecutionError):
119119
)
120120
}
121121

122+
/** Completes the job with a failure already in the execution log, keeping it on the run's span. */
123+
function completeWithRecordedFailure(workflowId: string, executionId: string, error: unknown) {
124+
const failure = toError(error)
125+
trace.getActiveSpan()?.recordException(failure)
126+
return buildRecordedJobFailureResult({ workflowId, executionId, error: failure.message })
127+
}
128+
122129
function requireLegacyWorkflowJobString(value: unknown, field: string): string {
123130
if (typeof value !== 'string' || !value.trim()) {
124131
throw new Error(`Legacy workflow job ${field} must be a non-empty string`)
@@ -333,16 +340,9 @@ export async function executeWorkflowJob(
333340

334341
// A failure core already recorded is the workflow's outcome, not a job fault; a run past
335342
// its deadline (checked after the await, which can outlast it) still fails the job.
336-
if (
337-
(await isRecordedWorkflowFailure(error, executionId, loggingSession)) &&
338-
!timeoutController.isTimedOut()
339-
) {
340-
trace.getActiveSpan()?.recordException(toError(error))
341-
return buildRecordedJobFailureResult({
342-
workflowId,
343-
executionId,
344-
error: toError(error).message,
345-
})
343+
if (await isRecordedWorkflowFailure(error, executionId, loggingSession)) {
344+
if (timeoutController.isTimedOut()) throw error
345+
return completeWithRecordedFailure(workflowId, executionId, error)
346346
}
347347

348348
const executionResult = hasExecutionResult(error) ? error.executionResult : undefined
@@ -365,12 +365,7 @@ export async function executeWorkflowJob(
365365
isNonFaultPreprocessingRejection(preprocessingRejection) &&
366366
loggingSession.hasCompleted()
367367
) {
368-
trace.getActiveSpan()?.recordException(toError(error))
369-
return buildRecordedJobFailureResult({
370-
workflowId,
371-
executionId,
372-
error: toError(error).message,
373-
})
368+
return completeWithRecordedFailure(workflowId, executionId, error)
374369
}
375370

376371
throw error

‎apps/sim/lib/core/async-jobs/recorded-failure.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,12 @@ import { isRecordLike } from '@sim/utils/object'
22
import type { JobType } from '@/lib/core/async-jobs/types'
33

44
/**
5-
* Job types whose runner reports a failure the run already recorded in its
5+
* The job type whose runner reports a failure the run already recorded in its
66
* execution log by completing with a {@link RecordedJobFailureResult} instead
77
* of throwing, so the queue does not fault (and alert on) a user or workflow
88
* error.
99
*/
10-
const RECORDED_FAILURE_JOB_TYPES: ReadonlySet<string> = new Set<JobType>(['workflow-execution'])
10+
const RECORDED_FAILURE_JOB_TYPE: JobType = 'workflow-execution'
1111

1212
/**
1313
* Bounds the error in UTF-8 bytes so the result stays inline under Trigger.dev's
@@ -35,6 +35,7 @@ interface RecordedJobFailureResult {
3535
executedAt: string
3636
}
3737

38+
/** The output a runner completes with when its failure is already in the execution log. */
3839
export function buildRecordedJobFailureResult(params: {
3940
workflowId: string
4041
executionId: string
@@ -56,6 +57,6 @@ export function buildRecordedJobFailureResult(params: {
5657
* runner threw.
5758
*/
5859
export function getRecordedJobFailureError(type: string, output: unknown): string | undefined {
59-
if (!RECORDED_FAILURE_JOB_TYPES.has(type) || !isRecordLike(output)) return undefined
60+
if (type !== RECORDED_FAILURE_JOB_TYPE || !isRecordLike(output)) return undefined
6061
return output.success === false && typeof output.error === 'string' ? output.error : undefined
6162
}

0 commit comments

Comments
 (0)