Uh oh!
There was an error while loading. Please reload this page.
refactor(polling): consolidate polling services into provider handler pattern - #4035
Conversation
… pattern Eliminate self-POST anti-pattern and extract shared boilerplate from 4 polling services into a clean handler registry mirroring lib/webhooks/providers/. - Add processPolledWebhookEvent() to processor.ts for direct in-process webhook execution, removing HTTP round-trips that caused Lambda 403/timeout errors - Extract shared utilities (markWebhookFailed/Success, fetchActiveWebhooks, runWithConcurrency, resolveOAuthCredential, updateWebhookProviderConfig) - Create PollingProviderHandler interface with per-provider implementations - Consolidate 4 identical route files into single dynamic [provider] route - Standardize concurrency to 10 across all providers - No infra changes needed — Helm cron paths resolve via dynamic route Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
PR SummaryMedium Risk Overview Consolidates API endpoints by replacing separate Eliminates the polling self-POST pattern by adding Reviewed by Cursor Bugbot for commit 4a1000c. Configure here. |
The latest updates on your projects. Learn more about Vercel for GitHub. |
Greptile SummaryThis PR is a well-scoped structural refactor that consolidates four near-identical polling services (Gmail, Outlook, IMAP, RSS) into a unified handler pattern under Key changes:
Confidence Score: 5/5Safe to merge — all prior P1 concerns are resolved, no new correctness or security issues introduced. All previously raised P1 findings (archived webhook filtering, auth-before-enumeration, idempotency import path) are addressed. Remaining findings are P2: a discarded preprocessing error detail, duplicated local interfaces in processor.ts, and verbose logger type annotations in private helpers. None affect runtime correctness. apps/sim/lib/webhooks/processor.ts — minor observability gap in preprocessResult error handling and interface duplication.
|
| Filename | Overview |
|---|---|
| apps/sim/app/api/webhooks/poll/[provider]/route.ts | Consolidates 4 identical route files into one dynamic route; auth check correctly precedes provider validation; lock key and TTL preserved correctly. |
| apps/sim/lib/webhooks/polling/utils.ts | Extracts shared boilerplate (markWebhookFailed/Success, fetchActiveWebhooks with archivedAt guards, runWithConcurrency, resolveOAuthCredential, updateWebhookProviderConfig) that was previously duplicated across all 4 providers. |
| apps/sim/lib/webhooks/polling/orchestrator.ts | Clean entry point delegating to handler registry and shared concurrency runner; correct per-webhook requestId scoping. |
| apps/sim/lib/webhooks/processor.ts | Adds processPolledWebhookEvent() to bypass self-POST; preprocessResult.error is discarded without logging; PolledWebhookRecord/PolledWorkflowRecord duplicate types in polling/types.ts. |
| apps/sim/lib/webhooks/polling/gmail.ts | Ports Gmail polling logic faithfully; uses ReturnType<typeof import('@sim/logger').createLogger> instead of Logger type in private helpers — minor style inconsistency. |
| apps/sim/lib/webhooks/polling/types.ts | Clean strategy interface for PollingProviderHandler; WebhookRecord/WorkflowRecord types well-defined but not reused in processor.ts. |
| apps/sim/lib/webhooks/polling/registry.ts | Simple handler lookup with VALID_POLLING_PROVIDERS derived from registry keys — adding a new provider only requires one entry here. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
CRON["Helm Cron\n/api/webhooks/poll/{provider}"]
ROUTE["[provider]/route.ts\nverifyCronAuth → VALID_POLLING_PROVIDERS\nacquireLock"]
ORCHESTRATOR["orchestrator.ts\npollProvider()"]
REGISTRY["registry.ts\ngetPollingHandler()"]
UTILS_FETCH["utils.ts\nfetchActiveWebhooks()"]
CONCURRENCY["utils.ts\nrunWithConcurrency(CONCURRENCY=10)"]
GMAIL["gmail.ts\ngmailPollingHandler"]
OUTLOOK["outlook.ts\noutlookPollingHandler"]
IMAP["imap.ts\nimapPollingHandler"]
RSS["rss.ts\nrssPollingHandler"]
IDEMPOTENCY["pollingIdempotency\n(per item)"]
PROCESSOR["processor.ts\nprocessPolledWebhookEvent()"]
ADMIT["tryAdmit() ticket"]
PREPROCESS["checkWebhookPreprocessing()"]
DISPATCH["BullMQ / inline job queue\nwebhook-execution"]
CRON --> ROUTE
ROUTE --> ORCHESTRATOR
ORCHESTRATOR --> REGISTRY
ORCHESTRATOR --> UTILS_FETCH
UTILS_FETCH --> CONCURRENCY
CONCURRENCY --> GMAIL & OUTLOOK & IMAP & RSS
GMAIL & OUTLOOK & IMAP & RSS --> IDEMPOTENCY
IDEMPOTENCY --> PROCESSOR
PROCESSOR --> ADMIT
ADMIT --> PREPROCESS
PREPROCESS --> DISPATCH
Reviews (2): Last reviewed commit: "fix(polling): use literal for maxDuratio..." | Re-trigger Greptile
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
…e casts - Widen processPolledWebhookEvent body param to accept object, eliminating `as unknown as Record<string, unknown>` double casts in all 4 handlers - Extract LOCK_TTL_SECONDS constant in route, tying maxDuration and lock TTL to a single value Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add archivedAt filters to fetchActiveWebhooks query, matching findWebhookAndWorkflow in processor.ts to prevent polling archived webhooks/workflows - Move provider validation after auth check to prevent provider enumeration by unauthenticated callers - Fix inconsistent pollingIdempotency import path in outlook.ts to match other handlers Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
waleedlatif1
commented
Apr 8, 2026
waleedlatif1
commented
Apr 8, 2026
@cursor review |
Next.js requires segment config exports to be statically analyzable literals. Using a variable reference caused build failure. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 4a1000c. Configure here.
Summary
processPolledWebhookEvent()toprocessor.tsfor direct in-process webhook execution, removing HTTP round-trips that caused Lambda 403/timeout errors under loadmarkWebhookFailed/Success,fetchActiveWebhooks,runWithConcurrency,resolveOAuthCredential,updateWebhookProviderConfig— previously duplicated across all 4 services (~30% of each file)[provider]route. Helm cron paths (/api/webhooks/poll/gmail, etc.) resolve unchanged via Next.js dynamic routingCONCURRENCY = 10(IMAP was previously 5)New structure (mirrors
lib/webhooks/providers/)No infra changes needed
/api/webhooks/poll/{gmail,outlook,imap,rss}— dynamic route matches identically${provider}-polling-lock) produce the same strings as beforemaxDuration = 180,dynamic = 'force-dynamic',verifyCronAuth— all preservedTest plan
bun run type-checkpasses (only pre-existing Athena errors)grep -r "polling-service" apps/sim/returns nothing/api/webhooks/poll/gmailetc. and returns 200