Skip to content

Commit fce177e

Browse files
waleedlatif1claude
authored andcommitted
refactor(webhooks): extract provider-specific logic into handler registry (#3973)
* refactor(webhooks): extract provider-specific logic into handler registry * fix(webhooks): address PR review feedback - Restore original fall-through behavior for generic requireAuth with no token - Replace `any` params with proper types in processor helper functions - Restore array-aware initializer in processTriggerFileOutputs * fix(webhooks): fix build error from union type indexing in processTriggerFileOutputs Cast array initializer to Record<string, unknown> to allow string indexing while preserving array runtime semantics for the return value. * fix(webhooks): return 401 when requireAuth is true but no token configured If a user explicitly sets requireAuth: true, they expect auth to be enforced. Returning 401 when no token is configured is the correct behavior — this is an intentional improvement over the original code which silently allowed unauthenticated access in this case. * refactor(webhooks): move signature validators into provider handler files Co-locate each validate*Signature function with its provider handler, eliminating the circular dependency where handlers imported back from utils.server.ts. validateJiraSignature is exported from jira.ts for shared use by confluence.ts. * refactor(webhooks): move challenge handlers into provider files Move handleWhatsAppVerification to providers/whatsapp.ts and handleSlackChallenge to providers/slack.ts. Update processor.ts imports to point to provider files. * refactor(webhooks): move fetchAndProcessAirtablePayloads into airtable handler Co-locate the ~400-line Airtable payload processing function with its provider handler. Remove AirtableChange interface from utils.server.ts. * refactor(webhooks): extract polling config functions into polling-config.ts Move configureGmailPolling, configureOutlookPolling, configureRssPolling, and configureImapPolling out of utils.server.ts into a dedicated module. Update imports in deploy.ts and webhooks/route.ts. * refactor(webhooks): decompose formatWebhookInput into per-provider formatInput methods Move all provider-specific input formatting from the monolithic formatWebhookInput switch statement into each provider's handler file. Delete formatWebhookInput and all its helper functions (fetchWithDNSPinning, formatTeamsGraphNotification, Slack file helpers, convertSquareBracketsToTwiML) from utils.server.ts. Create new handler files for gmail, outlook, rss, imap, and calendly providers. Update webhook-execution.ts to use handler.formatInput as the primary path with raw body passthrough as fallback. utils.server.ts reduced from ~1600 lines to ~370 lines containing only credential-sync functions. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor(webhooks): decompose provider-subscriptions into handler registry pattern Move all provider-specific subscription create/delete logic from the monolithic provider-subscriptions.ts into individual provider handler files via new createSubscription/deleteSubscription methods on WebhookProviderHandler. Replace the two massive if-else dispatch chains (11 branches each) with simple registry lookups via getProviderHandler(). provider-subscriptions.ts reduced from 2,337 lines to 128 lines (orchestration only). Also migrate polling configuration (gmail, outlook, rss, imap) into provider handlers via configurePolling() method, and challenge/verification handling (slack, whatsapp, teams) via handleChallenge() method. Delete polling-config.ts. Create new handler files for fathom and lemlist providers. Extract shared subscription utilities into subscription-utils.ts. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(webhooks): fix attio build error, restore imap field, remove demarcation comments - Cast `body` to `Record<string, unknown>` in attio formatInput to fix type error with extractor functions - Restore `rejectUnauthorized` field in imap configurePolling for parity - Remove `// ---` section demarcation comments from route.ts and airtable.ts - Update add-trigger skill to reflect handler-based architecture Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(webhooks): remove unused imports from utils.server.ts after rebase Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(webhooks): remove duplicate generic file processing from webhook-execution The generic provider's processInputFiles handler already handles file[] field processing via the handler.processInputFiles call. The hardcoded block from staging was incorrectly preserved during rebase, causing double processing. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(webhooks): validate auth token is set when requireAuth is enabled at deploy time Rejects deployment with a clear error message if a generic webhook trigger has requireAuth enabled but no authentication token configured, rather than letting requests fail with 401 at runtime. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(webhooks): remove unintended rejectUnauthorized field from IMAP polling config The refactored IMAP handler added a rejectUnauthorized field that was not present in the original configureImapPolling function. This would default to true for all existing IMAP webhooks, potentially breaking connections to servers with self-signed certificates. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(webhooks): replace crypto.randomUUID() with generateId() in ashby handler Per project coding standards, use generateId() from @/lib/core/utils/uuid instead of crypto.randomUUID() directly. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor(webhooks): standardize logger names and remove any types from providers - Standardize logger names to WebhookProvider:X pattern across 6 providers (fathom, gmail, imap, lemlist, outlook, rss) - Replace all `any` types in airtable handler with proper types: - Add AirtableTableChanges interface for API response typing - Change function params from `any` to `Record<string, unknown>` - Change AirtableChange fields from Record<string, any> to Record<string, unknown> - Change all catch blocks from `error: any` to `error: unknown` - Change input object from `any` to `Record<string, unknown>` Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor(webhooks): remove remaining any types from deploy.ts Replace 3 `catch (error: any)` with `catch (error: unknown)` and 1 `Record<string, any>` with `Record<string, unknown>`. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent c5c5743 commit fce177e

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

47 files changed

+6620
-6402
lines changed

.claude/commands/add-trigger.md

Lines changed: 295 additions & 177 deletions
Large diffs are not rendered by default.

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

Lines changed: 38 additions & 112 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,9 @@ import {
1616
createExternalWebhookSubscription,
1717
shouldRecreateExternalWebhookSubscription,
1818
} from '@/lib/webhooks/provider-subscriptions'
19+
import { getProviderHandler } from '@/lib/webhooks/providers'
1920
import { mergeNonUserFields } from '@/lib/webhooks/utils'
20-
import {
21-
configureGmailPolling,
22-
configureOutlookPolling,
23-
configureRssPolling,
24-
syncWebhooksForCredentialSet,
25-
} from '@/lib/webhooks/utils.server'
21+
import { syncWebhooksForCredentialSet } from '@/lib/webhooks/utils.server'
2622
import { authorizeWorkflowByWorkspacePermission } from '@/lib/workflows/utils'
2723
import { extractCredentialSetId, isCredentialSetValue } from '@/executor/constants'
2824

@@ -348,7 +344,6 @@ export async function POST(request: NextRequest) {
348344
workflowRecord.workspaceId || undefined
349345
)
350346

351-
// --- Credential Set Handling ---
352347
// For credential sets, we fan out to create one webhook per credential at save time.
353348
// This applies to all OAuth-based triggers, not just polling ones.
354349
// Check for credentialSetId directly (frontend may already extract it) or credential set value in credential fields
@@ -402,24 +397,24 @@ export async function POST(request: NextRequest) {
402397
)
403398
}
404399

405-
const needsConfiguration = provider === 'gmail' || provider === 'outlook'
400+
const providerHandler = getProviderHandler(provider)
406401

407-
if (needsConfiguration) {
408-
const configureFunc =
409-
provider === 'gmail' ? configureGmailPolling : configureOutlookPolling
402+
if (providerHandler.configurePolling) {
410403
const configureErrors: string[] = []
411404

412405
for (const wh of syncResult.webhooks) {
413406
if (wh.isNew) {
414-
// Fetch the webhook data for configuration
415407
const webhookRows = await db
416408
.select()
417409
.from(webhook)
418410
.where(and(eq(webhook.id, wh.id), isNull(webhook.archivedAt)))
419411
.limit(1)
420412

421413
if (webhookRows.length > 0) {
422-
const success = await configureFunc(webhookRows[0], requestId)
414+
const success = await providerHandler.configurePolling({
415+
webhook: webhookRows[0],
416+
requestId,
417+
})
423418
if (!success) {
424419
configureErrors.push(
425420
`Failed to configure webhook for credential ${wh.credentialId}`
@@ -436,7 +431,6 @@ export async function POST(request: NextRequest) {
436431
configureErrors.length > 0 &&
437432
configureErrors.length === syncResult.webhooks.length
438433
) {
439-
// All configurations failed - roll back
440434
logger.error(`[${requestId}] All webhook configurations failed, rolling back`)
441435
for (const wh of syncResult.webhooks) {
442436
await db.delete(webhook).where(eq(webhook.id, wh.id))
@@ -488,8 +482,6 @@ export async function POST(request: NextRequest) {
488482
}
489483
}
490484
}
491-
// --- End Credential Set Handling ---
492-
493485
let externalSubscriptionCreated = false
494486
const createTempWebhookData = (providerConfigOverride = resolvedProviderConfig) => ({
495487
id: targetWebhookId || generateShortId(),
@@ -629,115 +621,49 @@ export async function POST(request: NextRequest) {
629621
}
630622
}
631623

632-
// --- Gmail/Outlook webhook setup (these don't require external subscriptions, configure after DB save) ---
633-
if (savedWebhook && provider === 'gmail') {
634-
logger.info(`[${requestId}] Gmail provider detected. Setting up Gmail webhook configuration.`)
635-
try {
636-
const success = await configureGmailPolling(savedWebhook, requestId)
637-
638-
if (!success) {
639-
logger.error(`[${requestId}] Failed to configure Gmail polling, rolling back webhook`)
640-
await revertSavedWebhook(savedWebhook, existingWebhook, requestId)
641-
return NextResponse.json(
642-
{
643-
error: 'Failed to configure Gmail polling',
644-
details: 'Please check your Gmail account permissions and try again',
645-
},
646-
{ status: 500 }
647-
)
648-
}
649-
650-
logger.info(`[${requestId}] Successfully configured Gmail polling`)
651-
} catch (err) {
652-
logger.error(
653-
`[${requestId}] Error setting up Gmail webhook configuration, rolling back webhook`,
654-
err
655-
)
656-
await revertSavedWebhook(savedWebhook, existingWebhook, requestId)
657-
return NextResponse.json(
658-
{
659-
error: 'Failed to configure Gmail webhook',
660-
details: err instanceof Error ? err.message : 'Unknown error',
661-
},
662-
{ status: 500 }
624+
if (savedWebhook) {
625+
const pollingHandler = getProviderHandler(provider)
626+
if (pollingHandler.configurePolling) {
627+
logger.info(
628+
`[${requestId}] ${provider} provider detected. Setting up polling configuration.`
663629
)
664-
}
665-
}
666-
// --- End Gmail specific logic ---
630+
try {
631+
const success = await pollingHandler.configurePolling({
632+
webhook: savedWebhook,
633+
requestId,
634+
})
667635

668-
// --- Outlook webhook setup ---
669-
if (savedWebhook && provider === 'outlook') {
670-
logger.info(
671-
`[${requestId}] Outlook provider detected. Setting up Outlook webhook configuration.`
672-
)
673-
try {
674-
const success = await configureOutlookPolling(savedWebhook, requestId)
636+
if (!success) {
637+
logger.error(
638+
`[${requestId}] Failed to configure ${provider} polling, rolling back webhook`
639+
)
640+
await revertSavedWebhook(savedWebhook, existingWebhook, requestId)
641+
return NextResponse.json(
642+
{
643+
error: `Failed to configure ${provider} polling`,
644+
details: 'Please check your account permissions and try again',
645+
},
646+
{ status: 500 }
647+
)
648+
}
675649

676-
if (!success) {
677-
logger.error(`[${requestId}] Failed to configure Outlook polling, rolling back webhook`)
678-
await revertSavedWebhook(savedWebhook, existingWebhook, requestId)
679-
return NextResponse.json(
680-
{
681-
error: 'Failed to configure Outlook polling',
682-
details: 'Please check your Outlook account permissions and try again',
683-
},
684-
{ status: 500 }
650+
logger.info(`[${requestId}] Successfully configured ${provider} polling`)
651+
} catch (err) {
652+
logger.error(
653+
`[${requestId}] Error setting up ${provider} webhook configuration, rolling back webhook`,
654+
err
685655
)
686-
}
687-
688-
logger.info(`[${requestId}] Successfully configured Outlook polling`)
689-
} catch (err) {
690-
logger.error(
691-
`[${requestId}] Error setting up Outlook webhook configuration, rolling back webhook`,
692-
err
693-
)
694-
await revertSavedWebhook(savedWebhook, existingWebhook, requestId)
695-
return NextResponse.json(
696-
{
697-
error: 'Failed to configure Outlook webhook',
698-
details: err instanceof Error ? err.message : 'Unknown error',
699-
},
700-
{ status: 500 }
701-
)
702-
}
703-
}
704-
// --- End Outlook specific logic ---
705-
706-
// --- RSS webhook setup ---
707-
if (savedWebhook && provider === 'rss') {
708-
logger.info(`[${requestId}] RSS provider detected. Setting up RSS webhook configuration.`)
709-
try {
710-
const success = await configureRssPolling(savedWebhook, requestId)
711-
712-
if (!success) {
713-
logger.error(`[${requestId}] Failed to configure RSS polling, rolling back webhook`)
714656
await revertSavedWebhook(savedWebhook, existingWebhook, requestId)
715657
return NextResponse.json(
716658
{
717-
error: 'Failed to configure RSS polling',
718-
details: 'Please try again',
659+
error: `Failed to configure ${provider} webhook`,
660+
details: err instanceof Error ? err.message : 'Unknown error',
719661
},
720662
{ status: 500 }
721663
)
722664
}
723-
724-
logger.info(`[${requestId}] Successfully configured RSS polling`)
725-
} catch (err) {
726-
logger.error(
727-
`[${requestId}] Error setting up RSS webhook configuration, rolling back webhook`,
728-
err
729-
)
730-
await revertSavedWebhook(savedWebhook, existingWebhook, requestId)
731-
return NextResponse.json(
732-
{
733-
error: 'Failed to configure RSS webhook',
734-
details: err instanceof Error ? err.message : 'Unknown error',
735-
},
736-
{ status: 500 }
737-
)
738665
}
739666
}
740-
// --- End RSS specific logic ---
741667

742668
if (!targetWebhookId && savedWebhook) {
743669
try {

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

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,6 @@ const {
9797
handleSlackChallengeMock,
9898
processWhatsAppDeduplicationMock,
9999
processGenericDeduplicationMock,
100-
fetchAndProcessAirtablePayloadsMock,
101100
processWebhookMock,
102101
executeMock,
103102
getWorkspaceBilledAccountUserIdMock,
@@ -109,7 +108,6 @@ const {
109108
handleSlackChallengeMock: vi.fn().mockReturnValue(null),
110109
processWhatsAppDeduplicationMock: vi.fn().mockResolvedValue(null),
111110
processGenericDeduplicationMock: vi.fn().mockResolvedValue(null),
112-
fetchAndProcessAirtablePayloadsMock: vi.fn().mockResolvedValue(undefined),
113111
processWebhookMock: vi.fn().mockResolvedValue(new Response('Webhook processed', { status: 200 })),
114112
executeMock: vi.fn().mockResolvedValue({
115113
success: true,
@@ -156,10 +154,8 @@ vi.mock('@/background/logs-webhook-delivery', () => ({
156154
vi.mock('@/lib/webhooks/utils', () => ({
157155
handleWhatsAppVerification: handleWhatsAppVerificationMock,
158156
handleSlackChallenge: handleSlackChallengeMock,
159-
verifyProviderWebhook: vi.fn().mockReturnValue(null),
160157
processWhatsAppDeduplication: processWhatsAppDeduplicationMock,
161158
processGenericDeduplication: processGenericDeduplicationMock,
162-
fetchAndProcessAirtablePayloads: fetchAndProcessAirtablePayloadsMock,
163159
processWebhook: processWebhookMock,
164160
}))
165161

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ async function handleWebhookPost(
8787
if (webhooksForPath.length === 0) {
8888
const verificationResponse = await handlePreLookupWebhookVerification(
8989
request.method,
90-
body,
90+
body as Record<string, unknown> | undefined,
9191
requestId,
9292
path
9393
)

0 commit comments

Comments
 (0)