improvement(polling): fix correctness and efficiency across all polling handlers#4067
improvement(polling): fix correctness and efficiency across all polling handlers#4067waleedlatif1 wants to merge 7 commits intostagingfrom
Conversation
…ng handlers - Gmail: paginate history API, add historyTypes filter, differentiate 403/429, fetch fresh historyId on fallback to break 404 retry loop - Outlook: follow @odata.nextLink pagination, use fetchWithRetry for all Graph calls, fix $top alignment, skip folder filter on partial resolution failure, remove Content-Type from GET requests - RSS: add conditional GET (ETag/If-None-Match), raise GUID cap to 500, fix 304 ETag capture per RFC 9111, align GUID tracking with idempotency fallback key - IMAP: single connection reuse, UIDVALIDITY tracking per mailbox, advance UID only on successful fetch, fix messageFlagsAdd range type, remove cross-mailbox legacy UID fallback - Dispatch polling via trigger.dev task with per-provider concurrency key; fall back to synchronous Redis-locked polling for self-hosted
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview The Polling handlers were tightened to reduce missed/duplicate work and handle large inboxes and cache semantics: Gmail now paginates the History API and treats 403/429 as retryable failures (with a profile-based Reviewed by Cursor Bugbot for commit 18f5323. Configure here. |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: RSS GUID fallback diverges between tracking and idempotency
- Added the same truthiness guard (item.title && item.pubDate) to the idempotency key computation at line 295-298 to match the tracking and filtering logic.
Or push these changes by commenting:
@cursor push 79c99ddceb
Preview (79c99ddceb)
diff --git a/apps/sim/lib/webhooks/polling/rss.ts b/apps/sim/lib/webhooks/polling/rss.ts
--- a/apps/sim/lib/webhooks/polling/rss.ts
+++ b/apps/sim/lib/webhooks/polling/rss.ts
@@ -292,7 +292,10 @@
for (const item of items) {
try {
- const itemGuid = item.guid || item.link || `${item.title}-${item.pubDate}`
+ const itemGuid =
+ item.guid ||
+ item.link ||
+ (item.title && item.pubDate ? `${item.title}-${item.pubDate}` : '')
await pollingIdempotency.executeWithIdempotency(
'rss',This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.
Greptile SummaryThis PR comprehensively fixes correctness and efficiency issues across all four polling handlers (Gmail, Outlook, IMAP, RSS) and modernises the dispatch layer by routing polling jobs through trigger.dev when available, with a Redis-locked synchronous fallback for self-hosted deployments. Key improvements include: Gmail History API pagination with Confidence Score: 5/5Safe to merge — all remaining findings are P2 style/architecture suggestions that do not affect runtime correctness. The bug fixes (IMAP connection reuse, messageFlagsAdd range type, Gmail 403/429 escalation, Outlook pagination, RSS 304 handling) are all correct. The only flagged issue is a cross-domain import of fetchWithRetry from the knowledge module, which is a structural concern but does not cause any runtime defect. apps/sim/lib/webhooks/polling/outlook.ts — cross-domain fetchWithRetry import worth addressing in a follow-up.
|
| Filename | Overview |
|---|---|
| apps/sim/lib/webhooks/polling/outlook.ts | Adds @odata.nextLink pagination, fetchWithRetry (imported from knowledge domain — cross-domain coupling), removes Content-Type from GETs, and skips folder filter on partial well-known folder resolution failure. |
| apps/sim/lib/webhooks/polling/imap.ts | Major refactor: single ImapFlow connection shared between fetch and mark-as-read, UIDVALIDITY tracking per mailbox, UID advance inside fetch loop, fixed messageFlagsAdd range type from object to number. |
| apps/sim/lib/webhooks/polling/gmail.ts | History API now uses do…while pagination with historyTypes=messageAdded filter, throws on 403/429, and fetches a fresh historyId from the profile API on zero-result fallback to break the 404 retry loop. |
| apps/sim/lib/webhooks/polling/rss.ts | Adds conditional GET (ETag/If-None-Match, Last-Modified/If-Modified-Since), handles 304 correctly, raises GUID dedup cap from 100 to 500, and aligns fallback GUID key with idempotency key. |
| apps/sim/app/api/webhooks/poll/[provider]/route.ts | Dispatches polling to trigger.dev when enabled (with per-provider concurrencyKey), gracefully falls back to Redis-locked synchronous polling when trigger.dev fails or is absent. |
| apps/sim/background/provider-polling.ts | New trigger.dev task wrapping pollProvider; 5-minute maxDuration, single retry, concurrencyLimit=1 per provider via concurrencyKey. |
| apps/sim/app/api/schedules/execute/route.ts | Trivial cleanup: converts dynamic import of getWorkflowById to a static top-level import. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Cron triggers GET /poll/provider] --> B{verifyCronAuth}
B -- fail --> Z[401 Unauthorized]
B -- pass --> C{isTriggerDevEnabled?}
C -- yes --> D[providerPolling.trigger with per-provider concurrency]
D -- success --> E[200 dispatched - async run begins]
D -- error --> F[Fall through to sync path]
C -- no --> F
F --> G[acquireLock provider-polling-lock]
G -- not acquired --> H[202 skipped - poll already running]
G -- acquired --> I[pollProvider synchronously]
I --> J[releaseLock]
J --> K[200 completed]
D -.-> L[Trigger.dev task runs pollProvider]
L --> M[Gmail / Outlook / IMAP / RSS handler]
Reviews (6): Last reviewed commit: "fix(imap): preserve fresh UID after UIDV..." | Re-trigger Greptile
|
@greptile |
|
@cursor review |
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 7c7c95d. Configure here.
|
@greptile |
|
@cursor review |
|
@greptile |
|
@cursor review |
|
@greptile |
|
@cursor review |
|
@greptile |
|
@cursor review |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 18f5323. Configure here.
| nextUrl = | ||
| allEmails.length < maxEmails ? (data['@odata.nextLink'] as string | undefined) : undefined | ||
|
|
||
| if (pageEmails.length === 0) break |
There was a problem hiding this comment.
Outlook pagination never fetches beyond first page
Medium Severity
The $top parameter is set to maxEmails, so the first Graph API page already returns up to maxEmails items. The while loop condition allEmails.length < maxEmails then immediately evaluates false after the first page fills up, making the @odata.nextLink pagination dead code in that scenario. When Graph returns fewer items per page than $top (which is common), pagination works correctly. But when maxEmailsPerPoll is at or below Graph's default page size (typically ~10–50), the loop terminates after one page even if there are more matching emails, silently truncating results — the exact behavior the PR aims to fix.
Reviewed by Cursor Bugbot for commit 18f5323. Configure here.



Summary
Type of Change
Testing
Tested manually
Checklist