Skip to content

Commit 4c35a7b

Browse files
committed
fix(webhooks): requeue deliveries dropped by retryable setup infrastructure failures
1 parent edf07ec commit 4c35a7b

9 files changed

Lines changed: 642 additions & 64 deletions

File tree

apps/sim/background/webhook-execution.test.ts

Lines changed: 152 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -25,25 +25,32 @@ const {
2525
mockLoadDeploymentVersionState,
2626
mockGetProviderHandler,
2727
mockSetResolvedSecretTraceRegistry,
28-
} = vi.hoisted(() => ({
29-
mockResolveWebhookRecordProviderConfig: vi.fn(),
30-
mockExecuteWorkflowCore: vi.fn(),
31-
mockWasExecutionFinalizedByCore: vi.fn(),
32-
mockExecuteWithIdempotency: vi.fn(),
33-
mockRefreshExecutionSlotExpiry: vi.fn().mockResolvedValue(true),
34-
mockReleaseExecutionSlot: vi.fn(),
35-
mockGetProviderHandler: vi.fn(() => ({})),
36-
mockSetResolvedSecretTraceRegistry: vi.fn(),
37-
mockLoadDeploymentVersionState: vi.fn(
38-
async (_workflowId: string, deploymentVersionId: string) => ({
39-
blocks: {},
40-
edges: [],
41-
loops: {},
42-
parallels: {},
43-
deploymentVersionId,
44-
})
45-
),
46-
}))
28+
mockEnqueue,
29+
mockGetJobQueue,
30+
} = vi.hoisted(() => {
31+
const mockEnqueue = vi.fn()
32+
return {
33+
mockResolveWebhookRecordProviderConfig: vi.fn(),
34+
mockExecuteWorkflowCore: vi.fn(),
35+
mockWasExecutionFinalizedByCore: vi.fn(),
36+
mockExecuteWithIdempotency: vi.fn(),
37+
mockRefreshExecutionSlotExpiry: vi.fn().mockResolvedValue(true),
38+
mockReleaseExecutionSlot: vi.fn(),
39+
mockGetProviderHandler: vi.fn(() => ({})),
40+
mockSetResolvedSecretTraceRegistry: vi.fn(),
41+
mockLoadDeploymentVersionState: vi.fn(
42+
async (_workflowId: string, deploymentVersionId: string) => ({
43+
blocks: {},
44+
edges: [],
45+
loops: {},
46+
parallels: {},
47+
deploymentVersionId,
48+
})
49+
),
50+
mockEnqueue,
51+
mockGetJobQueue: vi.fn(async () => ({ enqueue: mockEnqueue })),
52+
}
53+
})
4754

4855
const mockGetEffectiveEnvironmentSnapshot =
4956
environmentUtilsMockFns.mockGetEffectiveEnvironmentSnapshot
@@ -105,6 +112,11 @@ vi.mock('@/lib/core/execution-limits', () => ({
105112
getExecutionDeadlineAt: vi.fn(() => new Date(Date.now() + 120_000)),
106113
getTimeoutErrorMessage: vi.fn(() => 'timed out'),
107114
RESERVATION_TTL_BUFFER_MS: 300_000,
115+
toTriggerMaxDurationSeconds: vi.fn(() => undefined),
116+
}))
117+
118+
vi.mock('@/lib/core/async-jobs', () => ({
119+
getJobQueue: mockGetJobQueue,
108120
}))
109121

110122
vi.mock('@/lib/workflows/executor/pause-persistence', () => ({
@@ -132,6 +144,7 @@ vi.mock('@/triggers', () => ({
132144
isTriggerValid: vi.fn(() => false),
133145
}))
134146

147+
import { isRetryableSetupError } from '@/lib/core/errors/retryable-infrastructure'
135148
import {
136149
executeWebhookJob,
137150
resolveWebhookExecutionProviderConfig,
@@ -242,6 +255,7 @@ describe('executeWebhookJob fault vs error handling', () => {
242255
}
243256
})
244257
mockGetProviderHandler.mockReturnValue({})
258+
mockEnqueue.mockResolvedValue('run_retry')
245259
mockExecuteWithIdempotency.mockImplementation(
246260
(_provider: string, _key: string, operation: () => Promise<unknown>) => operation()
247261
)
@@ -543,4 +557,123 @@ describe('executeWebhookJob fault vs error handling', () => {
543557

544558
expect(executionPreprocessingMockFns.mockPreprocessExecution).not.toHaveBeenCalled()
545559
})
560+
561+
it('requeues the delivery when preprocessing fails on retryable infrastructure', async () => {
562+
executionPreprocessingMockFns.mockPreprocessExecution.mockResolvedValueOnce({
563+
success: false,
564+
error: {
565+
message: 'Internal error while fetching workflow',
566+
statusCode: 500,
567+
retryable: true,
568+
cause: { code: 'CONNECT_TIMEOUT' },
569+
},
570+
})
571+
572+
const result = await executeWebhookJob(payload)
573+
574+
expect(result).toMatchObject({
575+
success: false,
576+
requeued: true,
577+
workflowId: 'workflow-1',
578+
executionId: 'execution-1',
579+
})
580+
expect(executionPreprocessingMockFns.mockPreprocessExecution).toHaveBeenCalledWith(
581+
expect.objectContaining({ suppressRetryableFailureLogs: true })
582+
)
583+
expect(mockEnqueue).toHaveBeenCalledTimes(1)
584+
const [jobType, retryPayload, options] = mockEnqueue.mock.calls[0]
585+
expect(jobType).toBe('webhook-execution')
586+
expect(retryPayload).toMatchObject({
587+
webhookId: 'webhook-1',
588+
workflowId: 'workflow-1',
589+
executionId: 'execution-1',
590+
requestId: 'request-1',
591+
infraRetryCount: 1,
592+
})
593+
expect(options.delayMs).toBeGreaterThan(0)
594+
// Database backend executes only through an in-process runner; trigger.dev ignores it.
595+
expect(options.runner).toBeTypeOf('function')
596+
expect(mockReleaseExecutionSlot).toHaveBeenCalledWith('execution-1')
597+
expect(mockExecuteWorkflowCore).not.toHaveBeenCalled()
598+
// No terminal failure row for an attempt that will be retried.
599+
expect(loggingSessionMockFns.mockSafeCompleteWithError).not.toHaveBeenCalled()
600+
})
601+
602+
it('requeues on retryable infrastructure errors thrown by setup reads', async () => {
603+
dbChainMockFns.limit.mockRejectedValueOnce(
604+
Object.assign(new Error('write CONNECT_TIMEOUT'), { code: 'CONNECT_TIMEOUT' })
605+
)
606+
607+
const result = await executeWebhookJob(payload)
608+
609+
expect(result).toMatchObject({ success: false, requeued: true })
610+
expect(mockEnqueue).toHaveBeenCalledTimes(1)
611+
expect(mockExecuteWorkflowCore).not.toHaveBeenCalled()
612+
expect(loggingSessionMockFns.mockSafeCompleteWithError).not.toHaveBeenCalled()
613+
})
614+
615+
it('faults the run without requeueing once the retry budget is exhausted', async () => {
616+
executionPreprocessingMockFns.mockPreprocessExecution.mockResolvedValueOnce({
617+
success: false,
618+
error: {
619+
message: 'Internal error while fetching workflow',
620+
statusCode: 500,
621+
retryable: true,
622+
},
623+
})
624+
625+
await expect(executeWebhookJob({ ...payload, infraRetryCount: 5 })).rejects.toSatisfy(
626+
(error: unknown) => isRetryableSetupError(error)
627+
)
628+
629+
expect(executionPreprocessingMockFns.mockPreprocessExecution).toHaveBeenCalledWith(
630+
expect.objectContaining({ suppressRetryableFailureLogs: false })
631+
)
632+
expect(mockEnqueue).not.toHaveBeenCalled()
633+
expect(mockReleaseExecutionSlot).toHaveBeenCalledWith('execution-1')
634+
})
635+
636+
it('does not requeue non-retryable preprocessing failures', async () => {
637+
executionPreprocessingMockFns.mockPreprocessExecution.mockResolvedValueOnce({
638+
success: false,
639+
error: { message: 'Usage limit exceeded', statusCode: 402 },
640+
})
641+
642+
await expect(executeWebhookJob(payload)).rejects.toSatisfy(
643+
(error: unknown) =>
644+
!isRetryableSetupError(error) && (error as Error).message === 'Usage limit exceeded'
645+
)
646+
647+
expect(mockEnqueue).not.toHaveBeenCalled()
648+
})
649+
650+
it('never reclassifies infrastructure errors after the workflow core started', async () => {
651+
const infraError = Object.assign(new Error('Connection terminated unexpectedly'), {
652+
code: 'CONNECTION_CLOSED',
653+
})
654+
mockExecuteWorkflowCore.mockRejectedValue(infraError)
655+
mockWasExecutionFinalizedByCore.mockReturnValue(false)
656+
657+
await expect(executeWebhookJob(payload)).rejects.toBe(infraError)
658+
659+
expect(mockEnqueue).not.toHaveBeenCalled()
660+
// Post-core failures keep recording the terminal row.
661+
expect(loggingSessionMockFns.mockSafeCompleteWithError).toHaveBeenCalled()
662+
})
663+
664+
it('faults the run when the requeue enqueue itself fails', async () => {
665+
executionPreprocessingMockFns.mockPreprocessExecution.mockResolvedValueOnce({
666+
success: false,
667+
error: {
668+
message: 'Internal error while fetching workflow',
669+
statusCode: 500,
670+
retryable: true,
671+
},
672+
})
673+
mockEnqueue.mockRejectedValueOnce(new Error('trigger api unavailable'))
674+
675+
await expect(executeWebhookJob(payload)).rejects.toThrow(
676+
'Internal error while fetching workflow'
677+
)
678+
})
546679
})

0 commit comments

Comments
 (0)