Skip to content

Commit 012d3e7

Browse files
committed
fix(webhooks): keep the pre-lookup steps in their original order
Hoisting the GET verification probe above the challenge handlers inverted the order this route has always used. A challenge is the more specific answer — a provider echoing a token it chose beats a generic "reachable" 200 — so when a path has both pending at once, inverting them answers the wrong one. Runs the challenge offer for every method before admission instead, which the delivery path was repeating anyway, so the sequence is restored and each step still happens once.
1 parent 6b93100 commit 012d3e7

1 file changed

Lines changed: 28 additions & 23 deletions

File tree

  • apps/sim/app/api/webhooks/trigger/[path]

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

Lines changed: 28 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -38,25 +38,37 @@ type WebhookTriggerContract =
3838
| typeof webhookTriggerDeleteContract
3939

4040
/**
41-
* Shared delivery entry point. Parses the path once, sheds load, and hands off — so a request is
42-
* parsed, challenged and looked up exactly once no matter which method it arrived on.
41+
* Shared delivery entry point, running the steps that need no body — and therefore no load-shed
42+
* ticket — before admission, in the order the `GET` route has always run them:
43+
*
44+
* 1. resolve the path,
45+
* 2. offer the request to the provider challenge handlers,
46+
* 3. on `GET`, answer a setup-time verification probe for a path that has no webhook row yet,
47+
* 4. take a ticket and hand off to the delivery path.
48+
*
49+
* Steps 2 and 3 keep their relative order because a challenge is the more specific answer: a
50+
* provider echoing a token it chose beats a generic "reachable" 200 when a path somehow has both
51+
* pending at once.
4352
*/
44-
function defineDeliveryRoute(
45-
contract: WebhookTriggerContract,
46-
beforeAdmission?: (
47-
request: NextRequest,
48-
requestId: string,
49-
path: string
50-
) => Promise<NextResponse | null>
51-
) {
53+
function defineDeliveryRoute(contract: WebhookTriggerContract, probeBeforeLookup = false) {
5254
return withRouteHandler(async (request: NextRequest, context: RouteContext) => {
5355
const requestId = generateRequestId()
5456
const parsed = await parseRequest(contract, request, context)
5557
if (!parsed.success) return parsed.response
5658
const { path } = parsed.data.params
5759

58-
const earlyResponse = await beforeAdmission?.(request, requestId, path)
59-
if (earlyResponse) return earlyResponse
60+
const challenge = await handleProviderChallenges({}, request, requestId, path)
61+
if (challenge) return challenge
62+
63+
if (probeBeforeLookup) {
64+
const verification = await handlePreLookupWebhookVerification(
65+
request.method,
66+
undefined,
67+
requestId,
68+
path
69+
)
70+
if (verification) return verification
71+
}
6072

6173
const ticket = tryAdmit()
6274
if (!ticket) {
@@ -72,13 +84,11 @@ function defineDeliveryRoute(
7284
}
7385

7486
/**
75-
* `GET` alone runs the pre-lookup verification probe before the webhook lookup, because a
76-
* provider validating a URL it has not been given a webhook for can only be answered there.
77-
* `handleWebhookDelivery` runs the same check for every method once the lookup comes back empty.
87+
* `GET` alone probes before the lookup, because a provider validating a URL it has not been given
88+
* a webhook for can only be answered there. `handleWebhookDelivery` runs the same check for every
89+
* method once the lookup comes back empty.
7890
*/
79-
export const GET = defineDeliveryRoute(webhookTriggerGetContract, (request, requestId, path) =>
80-
handlePreLookupWebhookVerification(request.method, undefined, requestId, path)
81-
)
91+
export const GET = defineDeliveryRoute(webhookTriggerGetContract, true)
8292

8393
export const POST = defineDeliveryRoute(webhookTriggerPostContract)
8494

@@ -127,11 +137,6 @@ async function handleWebhookDelivery(
127137
? Number(slackRequestTimestamp) * 1000
128138
: undefined
129139

130-
const earlyChallenge = await handleProviderChallenges({}, request, requestId, path)
131-
if (earlyChallenge) {
132-
return earlyChallenge
133-
}
134-
135140
const parseResult = await parseWebhookBody(request, requestId)
136141

137142
// Check if parseWebhookBody returned an error response

0 commit comments

Comments
 (0)