Skip to content

Commit 5b2a564

Browse files
committed
fix(webhooks): never replay completed poll work or mask a retryable fan-out failure
A poller stops its batch on a deterministic refusal only while nothing in the batch has completed; once an item has run, the refusal is an ordinary item failure, so the poller saves its completed work exactly as before and no completed event can replay after the idempotency window. In a multi-target delivery a missing block's no-retry 404 no longer stands in for a target that failed and needs the sender to retry. A source's Retry-After is counted from its answer rather than the poll's start. The RSS backoff keys are cleared by RSS's own state write, so other pollers' success path is unchanged, and two fields nothing reads are dropped.
1 parent 57ae81b commit 5b2a564

17 files changed

Lines changed: 110 additions & 42 deletions

‎apps/sim/app/api/webhooks/trigger/[path]/route.test.ts‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -742,6 +742,49 @@ describe('Webhook Trigger API Route', () => {
742742
expect(response.status).toBe(503)
743743
})
744744

745+
it('answers with a failing target rather than a missing block so the sender retries', async () => {
746+
testData.webhooks.push(
747+
{
748+
id: 'missing-block-webhook',
749+
provider: 'generic',
750+
path: 'mixed-path',
751+
isActive: true,
752+
providerConfig: {},
753+
workflowId: 'test-workflow-id',
754+
},
755+
{
756+
id: 'failing-webhook',
757+
provider: 'generic',
758+
path: 'mixed-path',
759+
isActive: true,
760+
providerConfig: {},
761+
workflowId: 'test-workflow-id',
762+
}
763+
)
764+
dispatchResolvedWebhookTargetMock
765+
.mockResolvedValueOnce({
766+
outcome: 'ignored',
767+
reason: 'block-missing',
768+
response: new NextResponse('Trigger block not found in deployment', {
769+
status: 404,
770+
headers: { 'x-slack-no-retry': '1' },
771+
}),
772+
})
773+
.mockResolvedValueOnce({
774+
outcome: 'failed',
775+
reason: 'queue-failed',
776+
response: new NextResponse(null, { status: 500 }),
777+
})
778+
779+
const response = await POST(
780+
createMockRequest('POST', { event: 'x' }),
781+
createRouteContext({ path: 'mixed-path' })
782+
)
783+
784+
expect(response.status).toBe(500)
785+
expect(response.headers.get('x-slack-no-retry')).toBeNull()
786+
})
787+
745788
it('tells Slack not to redeliver a POST to a path with no webhook', async () => {
746789
const req = createMockRequest('POST', { type: 'event_callback' })
747790

‎apps/sim/app/api/webhooks/trigger/[path]/route.ts‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,8 @@ async function handleWebhookDelivery(
265265
*/
266266
const responses: NextResponse[] = []
267267
const failures: NextResponse[] = []
268+
/** Kept apart so a missing block's no-retry 404 never stands in for a target that must retry. */
269+
const blockMissingResponses: NextResponse[] = []
268270
let hasDroppedTarget = false
269271
for (const dispatchResult of legacySlackDispatchResults) {
270272
if (dispatchResult.outcome === 'failed') {
@@ -348,7 +350,11 @@ async function handleWebhookDelivery(
348350
`[${requestId}] Webhook dispatch failed for ${foundWebhook.id}, continuing to next`,
349351
{ reason: dispatchResult.reason, status: dispatchResult.response.status }
350352
)
351-
failures.push(dispatchResult.response)
353+
if (dispatchResult.outcome === 'failed') {
354+
failures.push(dispatchResult.response)
355+
} else {
356+
blockMissingResponses.push(dispatchResult.response)
357+
}
352358
continue
353359
}
354360
return dispatchResult.response
@@ -361,6 +367,9 @@ async function handleWebhookDelivery(
361367
if (failures.length > 0) {
362368
return failures[0]
363369
}
370+
if (blockMissingResponses.length > 0) {
371+
return blockMissingResponses[0]
372+
}
364373
if (hasDroppedTarget) {
365374
return new NextResponse(null, { status: 200 })
366375
}

‎apps/sim/lib/execution/preprocessing.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1039,7 +1039,7 @@ describe('preprocessExecution admission rejection codes and blocked-run log thro
10391039
expect(result).toMatchObject({ success: false, error: expected })
10401040
})
10411041

1042-
it('leaves an unreadable usage ledger untagged and retryable', async () => {
1042+
it('leaves an unreadable usage ledger untagged so unattended senders retry', async () => {
10431043
mockCheckAttributedUsageLimits.mockResolvedValue({
10441044
isExceeded: true,
10451045
reason: 'usage_unavailable',
@@ -1049,7 +1049,7 @@ describe('preprocessExecution admission rejection codes and blocked-run log thro
10491049

10501050
const result = await refuse('workflow-1')
10511051

1052-
expect(result).toMatchObject({ success: false, error: { statusCode: 402, retryable: true } })
1052+
expect(result).toMatchObject({ success: false, error: { statusCode: 402 } })
10531053
expect(result.success === false && result.error.code).toBeUndefined()
10541054
})
10551055

‎apps/sim/lib/execution/preprocessing.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -767,7 +767,7 @@ export async function preprocessExecution(
767767
statusCode: 402,
768768
// An unreadable ledger fails closed; that is no verdict on the payer, so senders retry.
769769
...(usageCheck.reason === 'usage_unavailable'
770-
? { retryable: true }
770+
? {}
771771
: { code: ADMISSION_REJECTION_CODE.USAGE_LIMIT_EXCEEDED }),
772772
},
773773
},

‎apps/sim/lib/webhooks/polling/gmail.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -568,7 +568,7 @@ async function processEmails(
568568
)
569569
processedCount++
570570
} catch (error) {
571-
if (error instanceof PollAdmissionRefusedError) throw error
571+
if (error instanceof PollAdmissionRefusedError && processedCount === 0) throw error
572572
const errorMessage = getErrorMessage(error, 'Unknown error')
573573
logger.error(`[${requestId}] Error processing email ${email.id}:`, errorMessage)
574574
failedCount++

‎apps/sim/lib/webhooks/polling/google-calendar.test.ts‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,4 +98,15 @@ describe('Google Calendar polling when execution admission refuses events', () =
9898
expect(cursorUpdates).toEqual([])
9999
expect(mockMarkFailed).not.toHaveBeenCalled()
100100
})
101+
102+
it('saves the completed work as before when a refusal follows a completed event', async () => {
103+
mockProcessEvent.mockResolvedValueOnce({ success: true, executionId: 'execution-1' })
104+
105+
expect(await googleCalendarPollingHandler.pollWebhook(context())).not.toBe('skipped')
106+
107+
const cursorUpdates = mockUpdateConfig.mock.calls.filter(
108+
([, update]) => 'lastCheckedTimestamp' in (update as Record<string, unknown>)
109+
)
110+
expect(cursorUpdates).toHaveLength(1)
111+
})
101112
})

‎apps/sim/lib/webhooks/polling/google-calendar.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -354,7 +354,7 @@ async function processEvents(
354354
)
355355
processedCount++
356356
} catch (error) {
357-
if (error instanceof PollAdmissionRefusedError) throw error
357+
if (error instanceof PollAdmissionRefusedError && processedCount === 0) throw error
358358
const errorMessage = getErrorMessage(error, 'Unknown error')
359359
logger.error(`[${requestId}] Error processing event ${event.id}:`, errorMessage)
360360
failedCount++

‎apps/sim/lib/webhooks/polling/google-drive.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -421,7 +421,7 @@ async function processChanges(
421421
)
422422
processedCount++
423423
} catch (error) {
424-
if (error instanceof PollAdmissionRefusedError) throw error
424+
if (error instanceof PollAdmissionRefusedError && processedCount === 0) throw error
425425
const errorMessage = getErrorMessage(error, 'Unknown error')
426426
logger.error(
427427
`[${requestId}] Error processing change for file ${change.fileId}:`,

‎apps/sim/lib/webhooks/polling/google-sheets.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -451,7 +451,7 @@ async function processRows(
451451
)
452452
processedCount++
453453
} catch (error) {
454-
if (error instanceof PollAdmissionRefusedError) throw error
454+
if (error instanceof PollAdmissionRefusedError && processedCount === 0) throw error
455455
const errorMessage = getErrorMessage(error, 'Unknown error')
456456
logger.error(`[${requestId}] Error processing row ${rowNumber}:`, errorMessage)
457457
failedCount++

‎apps/sim/lib/webhooks/polling/hubspot.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -467,7 +467,7 @@ async function pollListMembership(
467467
)
468468
processedCount++
469469
} catch (error) {
470-
if (error instanceof PollAdmissionRefusedError) throw error
470+
if (error instanceof PollAdmissionRefusedError && processedCount === 0) throw error
471471
failedCount++
472472
logger.error(
473473
`[${requestId}] Error processing HubSpot list membership ${member.recordId}:`,
@@ -854,7 +854,7 @@ async function processRecords(
854854
snapshot.values.set(record.id, propertyValue ?? null)
855855
}
856856
} catch (error) {
857-
if (error instanceof PollAdmissionRefusedError) throw error
857+
if (error instanceof PollAdmissionRefusedError && processedCount === 0) throw error
858858
failedCount++
859859
cursorFrozen = true
860860
logger.error(

0 commit comments

Comments
 (0)