Skip to content

fix: bound inbound email listener polling and add rate-limit retry#248

Merged
felipefreitag merged 11 commits into
mainfrom
fix/bounded-poll-with-retry-f4b3
May 8, 2026
Merged

fix: bound inbound email listener polling and add rate-limit retry#248
felipefreitag merged 11 commits into
mainfrom
fix/bounded-poll-with-retry-f4b3

Conversation

@bukinoshita

@bukinoshita bukinoshita commented Apr 9, 2026

Copy link
Copy Markdown
Member

Summary by cubic

Bound the inbound email listener’s pagination and added automatic 429 retry to prevent request storms and lost progress during inbox bursts.

  • Bug Fixes

    • Cap pages per poll to 5 to avoid unbounded receiving.list() requests.
    • Checkpoint seen IDs after each page to preserve progress on mid-poll errors.
    • Retry 429s with withRetry using retry-after or 1s/2s/4s backoff.
    • Display emails oldest first.
  • Refactors

    • Added src/lib/with-retry.ts reusable helper (retries up to 3 times).
    • Replaced the while-loop with a recursive fetchPages flow using const arrow functions.

Written for commit 3d64b3d. Summary will update on new commits.

cursoragent and others added 3 commits April 9, 2026 18:06
Extract retry logic for rate_limit_exceeded (429) errors into a
standalone utility. Retries up to 3 times using the retry-after
header when available, falling back to exponential backoff (1s, 2s, 4s).

Co-authored-by: Bu Kinoshita <bukinoshita@users.noreply.github.com>
- Cap pages fetched per poll to MAX_PAGES_PER_POLL (5) so large
  email backlogs don't cause unbounded API request storms.
- Checkpoint seenIds after each page fetch so mid-poll errors
  preserve progress instead of discarding already-fetched emails.
- Wrap receiving.list() calls with withRetry to handle 429
  rate-limit responses with automatic backoff.
- Convert to functional style with const arrow functions and
  recursive fetchPages replacing the imperative while loop.

Closes BU-638

Co-authored-by: Bu Kinoshita <bukinoshita@users.noreply.github.com>
- Add with-retry.test.ts covering success, non-retryable errors,
  rate-limit retries with backoff, max retry exhaustion, and
  retry-after header parsing.
- Add listen tests for multi-page pagination, page cap enforcement,
  incremental checkpointing on mid-poll errors, rate-limit retry
  integration, and chronological email display order.
- Switch from mockClear to mockReset with resetModules to prevent
  mock state leaking between tests.

Co-authored-by: Bu Kinoshita <bukinoshita@users.noreply.github.com>
@bukinoshita

Copy link
Copy Markdown
Member Author

@cubic-dev-ai review this PR?

@cubic-dev-ai

cubic-dev-ai Bot commented Apr 9, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR?

@bukinoshita I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 4 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="tests/lib/with-retry.test.ts">

<violation number="1" location="tests/lib/with-retry.test.ts:33">
P2: Restore fake timers in a `finally` block (or shared cleanup). As written, a failure before `vi.useRealTimers()` will leak fake timers into later tests.</violation>
</file>

<file name="src/lib/with-retry.ts">

<violation number="1" location="src/lib/with-retry.ts:17">
P1: Handle HTTP-date `Retry-After` values as well as numeric seconds, or 429 retries can still fire earlier than the server allows.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.

Comment thread src/lib/with-retry.ts
Comment thread tests/lib/with-retry.test.ts
@bukinoshita bukinoshita marked this pull request as ready for review April 14, 2026 12:25
bukinoshita and others added 7 commits April 14, 2026 09:25
… withSpinner

Move parseRetryDelay, DEFAULT_RETRY_DELAYS, MAX_RETRIES, sleep, and the
SdkResponse<T> type into with-retry.ts as the single source of truth.
withSpinner delegates the retry loop to withRetry via an onRateLimited
callback that updates the spinner message during backoff.

HTTP-date support in parseRetryDelay (previously listen-only) now applies
to every command via withSpinner.

Behavior change: the spinner no longer reverts to the loading message
between retry sleep and next attempt — it stays on "Rate limited..."
through the next request, which is more honest UX.
…more

The MAX_PAGES_PER_POLL=5 cap (added earlier in this branch) was returning
hasMore: true silently — the caller never read the field, so a backlog
larger than 500 emails per poll would lose visibility with no signal.

- Surface page_cap_reached as a yellow stderr warning in interactive mode
  and as a JSON record (one-line NDJSON) in JSON mode.
- Bail in fetchPages when a page returns has_more: true with empty data
  (cursor would be undefined, causing a re-fetch from the head).
- Comment why MAX_PAGES_PER_POLL exists.

Tests: warning is emitted on cap-hit; next poll picks up emails arriving
after the cap; empty-page edge case terminates after one extra call.
Move seenIds.add out of fetchPages and into poll. Two reasons:

1. LRU eviction order: the bounded set on main puts most-recently-touched
   IDs at the tail. Per-page add ran in newest-first-per-page order, which
   left newest IDs near the LRU head — they'd be evicted first. Now poll
   reverses result.emails to chronological and adds in that order, so the
   newest emails land at the tail.

2. Mid-poll error checkpointing: fetchPages already returns partial emails
   when a page errors. Adding to seenIds in poll (after the fetch returns
   but before the error is reported up the loop) preserves progress without
   the per-page add logic. Same outcome, simpler structure.

Tests:
- Replace the misnamed "checkpoints progress incrementally" test with one
  that actually exercises multi-page checkpointing across polls (2 successful
  pages → mid-poll error → next poll dedupes both emails).
- Tighten the rate_limit retry test to pin call count before advancing the
  retry timer, catching regressions that ignore retry-after.
The first merge of main auto-resolved these two files to their pre-fb674af
content, accidentally reverting "fix: add new domain statuses (#287)" and
"feat: add custom click tracking subdomain (#273)". Neither change belongs
in this PR — restore main's versions.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 2 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/commands/domains/update.ts">

<violation number="1">
P2: Check `trackingSubdomain !== undefined` before assigning to the payload, so explicitly provided values are preserved.

(Based on your team's feedback about Commander.js option presence checks using `!== undefined`.) [FEEDBACK_USED]</violation>

<violation number="2">
P2: Use `trackingSubdomain === undefined` instead of a truthy check in the no-change guard, so provided (but possibly empty) values are not misclassified as absent.

(Based on your team's feedback about Commander.js option presence checks using `!== undefined`.) [FEEDBACK_USED]</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.

@felipefreitag felipefreitag merged commit 174fbbf into main May 8, 2026
7 checks passed
@felipefreitag felipefreitag deleted the fix/bounded-poll-with-retry-f4b3 branch May 8, 2026 19:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants