[FIX] Front New Conversation Tag rate limits #18857
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
WalkthroughUpdated many FrontApp action/source module version metadata; added retry-with-backoff for HTTP requests, Retry-After handling, and configurable delay between paginated requests; added async-retry dependency; and added a fallback for missing conversation subject in one source emitter. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as Caller
participant Paginate as paginate()
participant Retry as retryLogic
participant API as FrontAPI
Caller->>Paginate: Request pages (fn, params, delayMs)
Note over Paginate: isFirstPage = true
loop for each page
Paginate->>Retry: call fn(params)
Retry->>API: HTTP request
alt 2xx Success
API-->>Retry: response
Retry-->>Paginate: data
else 429 Rate limited
API-->>Retry: 429 + Retry-After?
Retry->>Retry: wait (Retry-After or backoff)
Retry->>API: retry request
API-->>Retry: response
Retry-->>Paginate: data
else 5xx Server error
Retry->>Retry: exponential backoff & retry
Retry->>API: retry
API-->>Retry: response
Retry-->>Paginate: data
else 4xx (non-429)
API-->>Retry: client error
Retry-->>Paginate: bail with error
end
Note over Paginate: if !isFirstPage and delayMs>0 -> wait delayMs
Paginate-->>Caller: yield page
Note over Paginate: isFirstPage = false
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (7)
components/frontapp/actions/create-message-template/create-message-template.mjs (2)
89-95: Normalize inboxIds once; simplify branching.
Minor clean‑up: since the prop isstring[], normalize and loop, avoiding the string/array split.- if (typeof inboxIds === "string") { - formData.append("inbox_ids", inboxIds); - } else if (Array.isArray(inboxIds)) { - for (const inboxId of inboxIds) { - formData.append("inbox_ids", inboxId); - } - } + const ids = Array.isArray(inboxIds) ? inboxIds : (inboxIds ? [inboxIds] : []); + for (const id of ids) formData.append("inbox_ids", id);
112-118: Avoid sending undefined fields in JSON payload.
Builddataconditionally to keep the request minimal and avoid accidental validation issues.- data = { - name, - subject, - body, - folder_id: folderId, - inbox_ids: inboxIds, - }; + data = { + name, + body, + ...(subject != null && { subject }), + ...(folderId != null && { folder_id: folderId }), + ...(Array.isArray(inboxIds) && inboxIds.length && { inbox_ids: inboxIds }), + };components/frontapp/sources/common/base.mjs (1)
31-31: Make per‑page delay configurable and add light jitter.Static 1.2s works but may be suboptimal across tenants. Recommend exposing a prop and adding small jitter to reduce herd effects.
Apply:
export default { props: { frontapp, db: "$.service.db", + pageDelayMs: { + type: "integer", + label: "Page Delay (ms)", + description: "Delay between paginated API calls.", + default: 1200, + optional: true, + min: 0, + }, timer: { type: "$.interface.timer", default: { intervalSeconds: DEFAULT_POLLING_SOURCE_TIMER_INTERVAL, }, }, }, ... const items = this.frontapp.paginate({ fn: this._getFunction(), params: this._getParams(lastTs), maxResults, - delayMs: 1200, // 1.2 second delay between pages to respect rate limits + delayMs: this.pageDelayMs, // delay between pages to respect rate limits });components/frontapp/sources/new-conversation-tag/new-conversation-tag.mjs (1)
9-9: Version bump LGTM; consider consistent ms timestamps to avoid reprocessing._emit uses floored seconds for
ts, but filters compareitem.emitted_at(ms). This can cause re-scans of same-second events (dedupe prevents double emit, but wastes calls).Apply:
- ts: (Math.floor(item.emitted_at / 1000)) * 1000, + ts: item.emitted_at, // keep ms precision to align with filterscomponents/frontapp/frontapp.app.mjs (3)
356-376: Avoid double wait: onRetry sleep + async‑retry backoff.You
awaitinsideonRetry(for Retry‑After) and async‑retry also applies its own backoff. This can stretch retries far beyond intent.
- Set
minTimeout: 0andfactor: 1to rely on your explicit waits for 429s, and small fixed backoff for others.- Optionally increase
retriesto 4–5 per PR objective.- const retryOpts = { - retries: 3, - maxTimeout: 30000, + const retryOpts = { + retries: 4, + minTimeout: 0, // rely on our explicit waits, esp. Retry-After + factor: 1, + maxTimeout: 30000, onRetry: async (err, attempt) => {
362-371: Honor Retry‑After HTTP‑date as well as seconds.Header can be a delay in seconds or an HTTP‑date. Currently only seconds are handled.
- if (status === 429) { - const retryAfter = err.response?.headers?.["retry-after"]; - if (retryAfter) { - retryAfterDelay = parseInt(retryAfter) * 1000; - console.log(`Rate limit exceeded. Waiting ${retryAfter} seconds before retry (attempt ${attempt}/${retryOpts.retries})`); - // Wait for the Retry-After period - await new Promise((resolve) => setTimeout(resolve, retryAfterDelay)); - } else { + if (status === 429) { + const ra = err.response?.headers?.["retry-after"]; + let delayMs = null; + if (ra) { + const seconds = Number(ra); + if (Number.isFinite(seconds)) { + delayMs = seconds * 1000; + } else { + const dateMs = Date.parse(ra); + if (!Number.isNaN(dateMs)) delayMs = Math.max(0, dateMs - Date.now()); + } + } + if (delayMs != null) { + console.log(`Rate limit exceeded. Waiting ${Math.ceil(delayMs/1000)}s before retry (attempt ${attempt}/${retryOpts.retries})`); + await new Promise((r) => setTimeout(r, delayMs)); + } else { console.log(`Rate limit exceeded. Using exponential backoff (attempt ${attempt}/${retryOpts.retries})`); } } else {
694-708: Add small random jitter to pagination delay (optional).Helps distribute load when many sources poll simultaneously.
- async *paginate({ - fn, params = {}, maxResults = null, delayMs = 1000, ...args + async *paginate({ + fn, params = {}, maxResults = null, delayMs = 1000, delayJitterMs = 0, ...args }) { ... - if (!isFirstPage && delayMs > 0) { - await new Promise((resolve) => setTimeout(resolve, delayMs)); + if (!isFirstPage && delayMs > 0) { + const jitter = delayJitterMs ? Math.floor(Math.random() * delayJitterMs) : 0; + await new Promise((resolve) => setTimeout(resolve, delayMs + jitter)); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (32)
components/frontapp/actions/add-comment/add-comment.mjs(1 hunks)components/frontapp/actions/archive-conversation/archive-conversation.mjs(1 hunks)components/frontapp/actions/assign-conversation/assign-conversation.mjs(1 hunks)components/frontapp/actions/create-draft-reply/create-draft-reply.mjs(1 hunks)components/frontapp/actions/create-draft/create-draft.mjs(1 hunks)components/frontapp/actions/create-inbox/create-inbox.mjs(1 hunks)components/frontapp/actions/create-message-template/create-message-template.mjs(1 hunks)components/frontapp/actions/create-message/create-message.mjs(1 hunks)components/frontapp/actions/delete-message-template/delete-message-template.mjs(1 hunks)components/frontapp/actions/get-comment/get-comment.mjs(1 hunks)components/frontapp/actions/get-conversation/get-conversation.mjs(1 hunks)components/frontapp/actions/get-message/get-message.mjs(1 hunks)components/frontapp/actions/get-teammate/get-teammate.mjs(1 hunks)components/frontapp/actions/import-message/import-message.mjs(1 hunks)components/frontapp/actions/list-comment-mentions/list-comment-mentions.mjs(1 hunks)components/frontapp/actions/list-comments/list-comments.mjs(1 hunks)components/frontapp/actions/list-conversations/list-conversations.mjs(1 hunks)components/frontapp/actions/list-message-templates/list-message-templates.mjs(1 hunks)components/frontapp/actions/list-teammates/list-teammates.mjs(1 hunks)components/frontapp/actions/receive-custom-messages/receive-custom-messages.mjs(1 hunks)components/frontapp/actions/reply-to-conversation/reply-to-conversation.mjs(1 hunks)components/frontapp/actions/send-new-message/send-new-message.mjs(1 hunks)components/frontapp/actions/tag-conversation/tag-conversation.mjs(1 hunks)components/frontapp/actions/update-conversation/update-conversation.mjs(1 hunks)components/frontapp/actions/update-teammate/update-teammate.mjs(1 hunks)components/frontapp/frontapp.app.mjs(3 hunks)components/frontapp/package.json(2 hunks)components/frontapp/sources/common/base.mjs(1 hunks)components/frontapp/sources/new-conversation-created/new-conversation-created.mjs(2 hunks)components/frontapp/sources/new-conversation-state-change/new-conversation-state-change.mjs(1 hunks)components/frontapp/sources/new-conversation-tag/new-conversation-tag.mjs(1 hunks)components/frontapp/sources/new-message-template-created/new-message-template-created.mjs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
components/frontapp/sources/new-conversation-created/new-conversation-created.mjs (1)
components/frontapp/actions/get-conversation/get-conversation.mjs (1)
conversation(31-34)
components/frontapp/frontapp.app.mjs (2)
components/gmail/gmail.app.mjs (2)
attempt(580-580)delayMs(590-590)components/frontapp/actions/get-message/get-message.mjs (1)
response(23-26)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: pnpm publish
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
- GitHub Check: Lint Code Base
🔇 Additional comments (30)
components/frontapp/actions/send-new-message/send-new-message.mjs (1)
8-8: Verify consistency between version bump and runtime behavior changes.The AI summary states there are "no changes to logic, props, or run-time behavior," yet the version is being bumped as part of a PR aimed at fixing rate-limiting issues through retry logic and exponential backoff. This suggests an inconsistency: either the underlying
frontapp.sendMessage()call (line 159) now includes retry logic (which would constitute a runtime behavior change), or the version bump is not justified.Please verify that:
- The
frontapp.sendMessage()method infrontapp.app.mjsincludes the expected retry logic for 429 and 5xx errors- If so, whether the version bump should be reflected as a patch version (0.2.10) rather than just metadata, given the improved resilience constitutes a user-facing behavioral change
components/frontapp/actions/create-message-template/create-message-template.mjs (1)
9-9: Version bump to 0.0.4 correctly enables rate-limit retries and backoff.The
frontapp.app.mjsmakeRequestmethod (lines 351–408) wraps all HTTP calls viaretry()with:
- 3 retries and 30s maxTimeout
- 429 status detection with Retry-After header extraction and exponential backoff
- 5xx error retry
- Headers properly merged and forwarded to the axios call
The
createMessageTemplateaction delegates throughmakeRequest, so it will automatically inherit this centralized retry logic.components/frontapp/actions/list-comment-mentions/list-comment-mentions.mjs (1)
7-7: Version bump approved.The version update aligns with the broader PR changes to FrontApp actions.
components/frontapp/actions/update-conversation/update-conversation.mjs (1)
8-8: Version bump approved.Consistent version increment for the coordinated FrontApp action release.
components/frontapp/actions/update-teammate/update-teammate.mjs (1)
7-7: Version bump approved.Aligned with coordinated FrontApp actions versioning update.
components/frontapp/actions/receive-custom-messages/receive-custom-messages.mjs (1)
8-8: Version bump approved.Consistent with the PR's coordinated action versioning strategy.
components/frontapp/actions/create-draft-reply/create-draft-reply.mjs (1)
8-8: Version bump approved.Aligned with FrontApp actions versioning update.
components/frontapp/actions/get-message/get-message.mjs (1)
7-7: Version bump approved.Consistent with coordinated FrontApp action versioning.
components/frontapp/actions/delete-message-template/delete-message-template.mjs (1)
7-7: Version bump approved.Aligned with the coordinated FrontApp action versioning release.
components/frontapp/actions/get-conversation/get-conversation.mjs (1)
7-7: Version bump approved.Consistent with the coordinated FrontApp action versioning update.
components/frontapp/actions/archive-conversation/archive-conversation.mjs (1)
7-7: LGTM! Version bump aligns with infrastructure improvements.The version increment reflects the updated retry logic and pagination pacing introduced in the underlying FrontApp app module.
components/frontapp/actions/create-draft/create-draft.mjs (1)
8-8: LGTM! Version bump aligns with infrastructure improvements.The version increment reflects the updated retry logic and pagination pacing introduced in the underlying FrontApp app module.
components/frontapp/actions/reply-to-conversation/reply-to-conversation.mjs (1)
8-8: LGTM! Version bump aligns with infrastructure improvements.The version increment reflects the updated retry logic and pagination pacing introduced in the underlying FrontApp app module.
components/frontapp/actions/assign-conversation/assign-conversation.mjs (1)
7-7: LGTM! Version bump aligns with infrastructure improvements.The version increment reflects the updated retry logic and pagination pacing introduced in the underlying FrontApp app module.
components/frontapp/actions/import-message/import-message.mjs (1)
8-8: LGTM! Version bump aligns with infrastructure improvements.The version increment reflects the updated retry logic and pagination pacing introduced in the underlying FrontApp app module.
components/frontapp/actions/get-comment/get-comment.mjs (1)
7-7: LGTM! Version bump aligns with infrastructure improvements.The version increment reflects the updated retry logic and pagination pacing introduced in the underlying FrontApp app module.
components/frontapp/actions/list-comments/list-comments.mjs (1)
7-7: LGTM! Version bump aligns with infrastructure improvements.The version increment reflects the updated retry logic and pagination pacing introduced in the underlying FrontApp app module.
components/frontapp/actions/list-teammates/list-teammates.mjs (1)
7-7: LGTM! Version bump aligns with infrastructure improvements.The version increment reflects the updated retry logic and pagination pacing introduced in the underlying FrontApp app module.
components/frontapp/actions/list-conversations/list-conversations.mjs (1)
7-7: LGTM: Routine version bump.Version metadata update aligns with the PR's coordinated release for rate-limiting improvements.
components/frontapp/actions/list-message-templates/list-message-templates.mjs (1)
7-7: LGTM: Version metadata update.components/frontapp/actions/add-comment/add-comment.mjs (1)
8-8: LGTM: Version metadata update.components/frontapp/actions/create-message/create-message.mjs (1)
7-7: LGTM: Version metadata update.components/frontapp/package.json (1)
14-14: LGTM: async-retry dependency supports PR objectives.The addition of
async-retryat version ^1.3.3 aligns with the PR's goals to implement exponential backoff for 429 and 5xx responses.components/frontapp/actions/get-teammate/get-teammate.mjs (1)
7-7: LGTM: Version metadata update.components/frontapp/actions/create-inbox/create-inbox.mjs (1)
7-7: LGTM: Version metadata update.components/frontapp/actions/tag-conversation/tag-conversation.mjs (1)
7-7: LGTM: Version metadata update.components/frontapp/sources/new-message-template-created/new-message-template-created.mjs (1)
9-9: Version bump looks good.No functional changes; safe to ship.
components/frontapp/sources/new-conversation-created/new-conversation-created.mjs (2)
26-26: Good fallback for missing subjects.
subject || idprevents undefined summaries and remains stable for dedupe.
9-9: Version bump ok.components/frontapp/frontapp.app.mjs (1)
2-2: Dependency import verified.
async-retryis declared incomponents/frontapp/package.jsonat version^1.3.3and locked inpnpm-lock.yaml. No action needed.
components/frontapp/sources/new-conversation-state-change/new-conversation-state-change.mjs
Show resolved
Hide resolved
2 participants
WHY
Resolves #18834
Summary by CodeRabbit
New Features
Bug Fixes
Chores