Skip to content

Commit 3567deb

Browse files
fix(slack): retry failed webhook deliveries
1 parent 1e60ab3 commit 3567deb

8 files changed

Lines changed: 107 additions & 18 deletions

File tree

apps/sim/app/api/webhooks/slack/custom/[credentialId]/route.ts

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
dispatchSlackCustomBotCredential,
99
verifySlackCustomBotCredentialRequest,
1010
} from '@/lib/webhooks/slack-custom-ingress'
11+
import { getSlackDispatchResponse } from '@/lib/webhooks/slack-dispatch'
1112

1213
export const dynamic = 'force-dynamic'
1314
export const runtime = 'nodejs'
@@ -74,13 +75,5 @@ async function handleSlackCustomBotWebhook(
7475
requestId,
7576
receivedAt,
7677
})
77-
const acknowledged = dispatchResults.some(
78-
(result) => result.outcome !== 'failed' && result.reason !== 'block-missing'
79-
)
80-
if (!acknowledged) {
81-
const failure = dispatchResults.find((result) => result.outcome === 'failed')
82-
if (failure) return failure.response
83-
}
84-
85-
return new NextResponse(null, { status: 200 })
78+
return getSlackDispatchResponse(dispatchResults)
8679
}

apps/sim/app/api/webhooks/slack/route.test.ts

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ function webhook(id: string) {
4343

4444
async function run(body: Record<string, unknown>) {
4545
mockParseWebhookBody.mockResolvedValue({ body, rawBody: JSON.stringify(body) })
46-
await POST(makeRequest())
46+
return POST(makeRequest())
4747
}
4848

4949
const messageBody = {
@@ -83,6 +83,18 @@ describe('Slack app webhook route', () => {
8383
expect(mockDispatchResolvedWebhookTarget).toHaveBeenCalledTimes(1)
8484
})
8585

86+
it('returns a retryable failure when no target queues', async () => {
87+
mockDispatchResolvedWebhookTarget.mockResolvedValue({
88+
outcome: 'failed',
89+
response: new Response(null, { status: 500 }),
90+
reason: 'queue-failed',
91+
})
92+
93+
const response = await run(messageBody)
94+
95+
expect(response.status).toBe(500)
96+
})
97+
8698
it('routes via Slack Connect authorizations and dedups overlapping webhooks', async () => {
8799
// Two candidate teams (outer + authorization) that resolve to overlapping webhooks.
88100
mockFindWebhooksByRoutingKey.mockImplementation(async (teamId: string) =>

apps/sim/app/api/webhooks/slack/route.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { generateRequestId } from '@/lib/core/utils/request'
66
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
77
import { findWebhooksByRoutingKey, parseWebhookBody } from '@/lib/webhooks/processor'
88
import { handleSlackChallenge, verifySlackRequestSignature } from '@/lib/webhooks/providers/slack'
9-
import { dispatchSlackWebhooks } from '@/lib/webhooks/slack-dispatch'
9+
import { dispatchSlackWebhooks, getSlackDispatchResponse } from '@/lib/webhooks/slack-dispatch'
1010

1111
const logger = createLogger('SlackAppWebhookAPI')
1212

@@ -106,7 +106,11 @@ async function handleSlackAppWebhook(request: NextRequest): Promise<NextResponse
106106
return new NextResponse(null, { status: 200 })
107107
}
108108

109-
await dispatchSlackWebhooks(webhooks, { body, request, requestId, receivedAt })
110-
111-
return new NextResponse(null, { status: 200 })
109+
const dispatchResults = await dispatchSlackWebhooks(webhooks, {
110+
body,
111+
request,
112+
requestId,
113+
receivedAt,
114+
})
115+
return getSlackDispatchResponse(dispatchResults)
112116
}

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import {
2727
getLegacySlackCustomBotCredentialId,
2828
verifySlackCustomBotCredentialRequest,
2929
} from '@/lib/webhooks/slack-custom-ingress'
30+
import { getSlackDispatchFailureResponse } from '@/lib/webhooks/slack-dispatch'
3031

3132
const logger = createLogger('WebhookTriggerAPI')
3233
const MAX_LEGACY_SLACK_CREDENTIALS_PER_PATH = 25
@@ -263,7 +264,7 @@ async function handleWebhookDelivery(
263264
let hasPermanentlyIgnoredLegacyTarget = false
264265
for (const dispatchResult of legacySlackDispatchResults) {
265266
if (dispatchResult.outcome === 'failed') {
266-
failures.push(dispatchResult.response)
267+
failures.push(getSlackDispatchFailureResponse(dispatchResult))
267268
continue
268269
}
269270
if (dispatchResult.reason === 'block-missing') {

apps/sim/lib/webhooks/providers/slack.test.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,12 @@ const ctx = (body: unknown) => ({
1717
const eventOf = (input: unknown) =>
1818
(input as { event: Record<string, unknown> }).event as Record<string, unknown>
1919

20+
describe('slackHandler responses', () => {
21+
it('returns a retryable failure when queue admission fails', () => {
22+
expect(slackHandler.formatQueueErrorResponse!().status).toBe(500)
23+
})
24+
})
25+
2026
describe('slackHandler formatInput - Events API', () => {
2127
it('maps an app_mention event', async () => {
2228
const { input } = await slackHandler.formatInput!(

apps/sim/lib/webhooks/providers/slack.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -860,7 +860,7 @@ export const slackHandler: WebhookProviderHandler = {
860860
},
861861

862862
formatQueueErrorResponse() {
863-
return new NextResponse(null, { status: 200 })
863+
return new NextResponse(null, { status: 500 })
864864
},
865865

866866
/**

apps/sim/lib/webhooks/slack-dispatch.test.ts

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,11 @@ vi.mock('@/lib/webhooks/providers/slack', () => ({
1717
}))
1818

1919
import { dispatchResolvedWebhookTarget } from '@/lib/webhooks/processor'
20-
import { dispatchSlackWebhooks } from '@/lib/webhooks/slack-dispatch'
20+
import {
21+
dispatchSlackWebhooks,
22+
getSlackDispatchFailureResponse,
23+
getSlackDispatchResponse,
24+
} from '@/lib/webhooks/slack-dispatch'
2125

2226
describe('dispatchSlackWebhooks', () => {
2327
it('dispatches at most ten targets concurrently and preserves result order', async () => {
@@ -68,4 +72,43 @@ describe('dispatchSlackWebhooks', () => {
6872
Array.from({ length: 12 }, (_, index) => 200 + index)
6973
)
7074
})
75+
76+
it('returns the failure when no Slack target is acknowledged', () => {
77+
const response = getSlackDispatchResponse([
78+
{
79+
outcome: 'failed',
80+
response: new NextResponse(null, { status: 500 }),
81+
reason: 'queue-failed',
82+
},
83+
])
84+
85+
expect(response.status).toBe(500)
86+
})
87+
88+
it('acknowledges a mixed fan-out when at least one Slack target queues', () => {
89+
const response = getSlackDispatchResponse([
90+
{
91+
outcome: 'failed',
92+
response: new NextResponse(null, { status: 500 }),
93+
reason: 'queue-failed',
94+
},
95+
{
96+
outcome: 'queued',
97+
response: new NextResponse(null, { status: 200 }),
98+
reason: 'queued',
99+
},
100+
])
101+
102+
expect(response.status).toBe(200)
103+
})
104+
105+
it('fails fast when a failed Slack dispatch carries a successful response', () => {
106+
expect(() =>
107+
getSlackDispatchFailureResponse({
108+
outcome: 'failed',
109+
response: new NextResponse(null, { status: 200 }),
110+
reason: 'queue-failed',
111+
})
112+
).toThrow('Failed Slack dispatch returned successful HTTP status 200')
113+
})
71114
})

apps/sim/lib/webhooks/slack-dispatch.ts

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { createLogger } from '@sim/logger'
2-
import type { NextRequest } from 'next/server'
2+
import { type NextRequest, NextResponse } from 'next/server'
33
import { mapWithConcurrency } from '@/lib/core/utils/concurrency'
44
import {
55
dispatchResolvedWebhookTarget,
@@ -18,6 +18,36 @@ interface DispatchSlackWebhooksOptions {
1818
receivedAt: number
1919
}
2020

21+
/** Returns the non-success response that tells Slack to retry a failed delivery. */
22+
export function getSlackDispatchFailureResponse(result: WebhookDispatchResult): NextResponse {
23+
if (result.outcome !== 'failed') {
24+
throw new Error(`Expected failed Slack dispatch, received ${result.outcome}`)
25+
}
26+
if (result.response.ok) {
27+
throw new Error(
28+
`Failed Slack dispatch returned successful HTTP status ${result.response.status}`
29+
)
30+
}
31+
return result.response
32+
}
33+
34+
/** Reduces a Slack fan-out to one provider acknowledgment or retry response. */
35+
export function getSlackDispatchResponse(results: WebhookDispatchResult[]): NextResponse {
36+
const acknowledged = results.some(
37+
(result) => result.outcome !== 'failed' && result.reason !== 'block-missing'
38+
)
39+
if (acknowledged) {
40+
return new NextResponse(null, { status: 200 })
41+
}
42+
43+
const failure = results.find((result) => result.outcome === 'failed')
44+
if (failure) {
45+
return getSlackDispatchFailureResponse(failure)
46+
}
47+
48+
return new NextResponse(null, { status: 200 })
49+
}
50+
2151
/**
2252
* Shared fan-out tail for the Slack ingest routes (native team-id route and the
2353
* custom-bot credential route): run each candidate webhook through the common

0 commit comments

Comments
 (0)