-
Notifications
You must be signed in to change notification settings - Fork 5.5k
Quickbooks - Enhanced Rate Limit Detection & Handling #18098
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub. 3 Skipped Deployments
|
WalkthroughPatch increments versions across many QuickBooks actions and sources. Updates retry/backoff behavior: expands rate-limit detection (429, 503, QuickBooks codes 3200/10001), respects Retry-After, caps exponential backoff, and increases initial backoff from 1500ms to 2000ms. Package version bumped. One action (search-vendors) alters an error message. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Utils as retryWithExponentialBackoff
participant QB as QuickBooks API
Client->>Utils: invoke(fn, { retries, backoff })
activate Utils
Utils->>QB: call API
QB-->>Utils: response or error
alt success
Utils-->>Client: result
else rate limit (HTTP 429/503 or QB code 3200/10001)
Utils->>Utils: read Retry-After or use backoff
Utils->>Utils: wait (cap next backoff ≤ 60s)
Utils->>QB: retry API
QB-->>Utils: result or error
Utils-->>Client: result or throw
else other errors
Utils-->>Client: throw error
end
deactivate Utils
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🔭 Outside diff range comments (2)
components/quickbooks/actions/sparse-update-invoice/sparse-update-invoice.mjs (1)
123-125
: Bug: Currency prop name mismatch (currency vs currencyRefValue) leads to CurrencyRef never setProps define
currency
(via propDefinition), but run() referencesthis.currencyRefValue
. This results in CurrencyRef being omitted.Apply this fix:
- CurrencyRef: this.currencyRefValue && { - value: this.currencyRefValue, - }, + CurrencyRef: this.currency && { + value: this.currency, + },If other actions standardize on
currencyRefValue
, alternatively rename the prop here for consistency, but the above change is minimal and correct.components/quickbooks/actions/get-invoice/get-invoice.mjs (1)
1-36
: Rate-limit updates verified, but missing$
context in several QuickBooks client callsI confirmed:
- INITIAL_BACKOFF_MILLISECONDS is set to 2000.
- Common utils handle 429 and Retry-After (case-insensitive).
- Backoff cap at 60000 ms is present.
However, the verification script flagged these calls that don’t pass the
$
context and need updating:• components/quickbooks/sources/common/base.mjs (paginate call)
• components/quickbooks/actions/create-sales-receipt/create-sales-receipt.mjs (getPropOptions)
• components/quickbooks/actions/update-estimate/update-estimate.mjs (getPropOptions)
• components/quickbooks/actions/update-invoice/update-invoice.mjs (getPropOptions)
• components/quickbooks/actions/sparse-update-invoice/sparse-update-invoice.mjs (getPropOptions)
• components/quickbooks/actions/create-bill/create-bill.mjs (getPropOptions)
• components/quickbooks/actions/create-purchase/create-purchase.mjs (getPropOptions)
• components/quickbooks/actions/create-purchase-order/create-purchase-order.mjs (getPropOptions)
• components/quickbooks/actions/create-invoice/create-invoice.mjs (getPropOptions)
• components/quickbooks/actions/create-estimate/create-estimate.mjs (getPropOptions)Please update each of these
this.quickbooks.*({ … })
calls to include the$
property (e.g.,await this.quickbooks.getPropOptions({ $, page, resource: "...", mapper: …, });) so that retry logic and rate-limit handling are applied consistently.
🧹 Nitpick comments (16)
components/quickbooks/actions/sparse-update-invoice/sparse-update-invoice.mjs (1)
115-115
: Optional: Ensure Line array exists before pushingIf
invoice.Line
is undefined, the optional chaining prevents push but later you passLine: invoice.Line
. Consider normalizing to an array when adding new lines.For example:
- if (lines?.length) invoice.Line?.push(...lines); + if (lines?.length) { + invoice.Line = Array.isArray(invoice.Line) ? invoice.Line : []; + invoice.Line.push(...lines); + }components/quickbooks/actions/search-purchases/search-purchases.mjs (1)
38-55
: Nit: Avoid double spaces in query assemblyCurrent template can yield double spaces (e.g.,
select * from Purchase WHERE ...
). Not harmful, but easy to tidy by conditionally joining tokens.You could refactor to:
- const whereClause = this.whereClause - ? ` WHERE ${this.whereClause}` - : ""; - const orderClause = this.orderClause - ? ` ORDERBY ${this.orderClause}` - : ""; - const startPosition = this.startPosition - ? ` STARTPOSITION ${this.startPosition}` - : ""; - const maxResults = this.maxResults - ? ` MAXRESULTS ${this.maxResults}` - : ""; - const query = `select * from Purchase ${whereClause}${orderClause}${startPosition}${maxResults}`; + const parts = [ + "select * from Purchase", + this.whereClause && `WHERE ${this.whereClause}`, + this.orderClause && `ORDERBY ${this.orderClause}`, + this.startPosition && `STARTPOSITION ${this.startPosition}`, + this.maxResults && `MAXRESULTS ${this.maxResults}`, + ].filter(Boolean); + const query = parts.join(" ");components/quickbooks/actions/update-item/update-item.mjs (1)
280-284
: Remove unusedparentRefName
check (dead path / confusion)
this.parentRefName
is referenced but no prop is defined for it. This creates a dead/ambiguous path and can mislead future maintainers.Apply this diff:
- if (this.parentRefValue || this.parentRefName) { + if (this.parentRefValue) { data["ParentRef"] = { value: this.parentRefValue, }; }components/quickbooks/actions/create-estimate/create-estimate.mjs (1)
289-291
: Normalize summary export key (summary
vs$summary
)Other actions export
summary
(no$
), while this action exports$summary
. For consistency across QuickBooks actions, prefer one convention (most files usesummary
).Apply this diff:
- $.export("$summary", `Successfully created estimate with ID ${response.Estimate.Id}`); + $.export("summary", `Successfully created estimate with ID ${response.Estimate.Id}`);components/quickbooks/actions/get-bill/get-bill.mjs (1)
29-33
: Optional: Guard summary export against unexpected response shapes.If the client ever returns a non-standard payload on partial failures, this could throw. Optional chaining avoids that without changing behavior.
- if (response) { - $.export("summary", `Successfully retrieved bill with ID ${response.Bill.Id}`); - } + if (response?.Bill?.Id) { + $.export("summary", `Successfully retrieved bill with ID ${response.Bill.Id}`); + } else if (response) { + $.export("summary", "Successfully retrieved bill"); + }components/quickbooks/actions/search-accounts/search-accounts.mjs (1)
24-31
: Optional: Use a numeric type for Start Position.QuickBooks expects a number for STARTPOSITION. If supported by the component framework, declaring this as an integer strengthens validation.
- startPosition: { - description: "The starting count of the response for pagination.", - label: "Start Position", - optional: true, - type: "string", - }, + startPosition: { + description: "The starting count of the response for pagination.", + label: "Start Position", + optional: true, + type: "integer", + },components/quickbooks/actions/get-my-company/get-my-company.mjs (1)
17-19
: Optional: Guard summary export.Mirrors the pattern suggested elsewhere to prevent errors on unexpected response shapes.
- if (response) { - $.export("summary", `Successfully retrieved company with ID ${response.CompanyInfo.Id}`); - } + if (response?.CompanyInfo?.Id) { + $.export("summary", `Successfully retrieved company with ID ${response.CompanyInfo.Id}`); + } else if (response) { + $.export("summary", "Successfully retrieved company"); + }components/quickbooks/actions/get-invoice/get-invoice.mjs (1)
29-31
: Optional: Safer summary export.Prevents a potential TypeError if the payload lacks the expected shape.
- if (response) { - $.export("summary", `Successfully retrieved invoice with ID ${response.Invoice.Id}`); - } + if (response?.Invoice?.Id) { + $.export("summary", `Successfully retrieved invoice with ID ${response.Invoice.Id}`); + } else if (response) { + $.export("summary", "Successfully retrieved invoice"); + }components/quickbooks/actions/search-vendors/search-vendors.mjs (2)
39-41
: Fix misleading error message: remove stray includeClause reference.The action doesn’t define or use includeClause. The error should reference only whereClause.
Apply this diff:
- throw new ConfigurationError("Must provide includeClause, whereClause parameters."); + throw new ConfigurationError("Must provide whereClause parameter.");
55-63
: Optional: surface query string in debug logs when rate-limited.Given the PR’s focus on improved rate-limit handling, consider optionally including the constructed query in debug logs upon 429 handling to help users diagnose throttled queries. Assuming the centralized retry logic already logs response metadata, adding the query when $ is in debug mode would complement that.
components/quickbooks/actions/create-invoice/create-invoice.mjs (1)
160-162
: Clarify configuration error message for line items.The validation accepts either numLineItems (builder flow) or lineItemsAsObjects (JSON flow). Update the error to reflect both accepted inputs to reduce user confusion.
- throw new ConfigurationError("Must provide lineItems, and customerRefValue parameters."); + throw new ConfigurationError("Must provide customerRefValue and either numLineItems or lineItems.");components/quickbooks/actions/get-sales-receipt/get-sales-receipt.mjs (1)
8-8
: Version bump looks good; minor ID handling nitConsider normalizing how IDs are read from prop values, matching other actions (e.g., coalescing
.value
when the prop is an option object). This avoids edge cases when the UI returns an object rather than a raw string.Example adjustment (outside this line range):
const response = await this.quickbooks.getSalesReceipt({ $, salesReceiptId: this.salesReceiptId?.value ?? this.salesReceiptId, });components/quickbooks/actions/search-invoices/search-invoices.mjs (1)
8-8
: Version metadata update — LGTM; optional validation for pagination inputsLooks good. Optionally, add lightweight validation/coercion for
startPosition
andmaxResults
to reduce QBO query errors when users pass non-numeric values.Example (outside this line range):
const startPos = this.startPosition != null ? parseInt(this.startPosition, 10) : undefined; if (startPos !== undefined && Number.isNaN(startPos)) { throw new ConfigurationError("startPosition must be a number."); } const max = this.maxResults != null ? parseInt(this.maxResults, 10) : undefined; if (max !== undefined && Number.isNaN(max)) { throw new ConfigurationError("maxResults must be a number."); } const startPosition = startPos ? ` STARTPOSITION ${startPos}` : ""; const maxResults = max ? ` MAXRESULTS ${max}` : "";components/quickbooks/common/constants.mjs (1)
3-3
: Centralize retry-related constants for consistencyRaising
INITIAL_BACKOFF_MILLISECONDS
to 2000 ms is reasonable. To prevent magic numbers from diverging, let’s centralize the backoff cap and retry criteria:• In
components/quickbooks/common/constants.mjs
, add alongside existing exports:export const MAX_BACKOFF_MILLISECONDS = 60000; // cap for exponential backoff export const RETRYABLE_STATUS_CODES = [429, 503]; // HTTP statuses to retry export const QUICKBOOKS_RATE_LIMIT_ERROR_CODES = [ // QBO error codes to treat as rate limits "3200", "10001", ];• In
components/quickbooks/common/utils.mjs
, replace hard-coded values:
- Line 72:
// from Math.min(backoff * 2, 60000) // to Math.min(backoff * 2, MAX_BACKOFF_MILLISECONDS)- Lines 59–61:
// from status === 429 || status === 503 || errorCode === "3200" || errorCode === "10001" // to RETRYABLE_STATUS_CODES.includes(status) || QUICKBOOKS_RATE_LIMIT_ERROR_CODES.includes(errorCode)This ensures all retry logic references a single source of truth.
components/quickbooks/common/utils.mjs (2)
70-73
: Optional: add jitter to backoff to reduce thundering herdWhen Retry-After isn’t provided, many clients will retry at the same intervals. Adding jitter to the next backoff mitigates synchronized retries. Keep exact Retry-After when provided.
Apply this diff:
- await new Promise((resolve) => setTimeout(resolve, delay)); - return retryWithExponentialBackoff(requestFn, retries - 1, Math.min(backoff * 2, 60000)); + await new Promise((resolve) => setTimeout(resolve, delay)); + const nextBackoffBase = Math.min(backoff * 2, 60000); + // Preserve server guidance strictly; only jitter when we're self-calculating. + const nextBackoff = retryAfter + ? nextBackoffBase + : Math.floor(nextBackoffBase * (0.5 + Math.random() * 0.5)); // 50–100% of base + return retryWithExponentialBackoff(requestFn, retries - 1, nextBackoff);
70-70
: Nit: include status and QuickBooks error code in the warningAdds valuable context for debugging without increasing noise.
If you adopt the errorCodeStr change above, you can apply:
- console.warn(`QuickBooks rate limit exceeded. Retrying in ${delay}ms... (${retries} retries left)`); + console.warn(`QuickBooks rate limit detected (status=${status}, qbCode=${errorCodeStr ?? "n/a"}). Retrying in ${delay}ms... (${retries} retries left)`);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (51)
components/quickbooks/actions/create-ap-aging-report/create-ap-aging-report.mjs
(1 hunks)components/quickbooks/actions/create-bill/create-bill.mjs
(1 hunks)components/quickbooks/actions/create-customer/create-customer.mjs
(1 hunks)components/quickbooks/actions/create-estimate/create-estimate.mjs
(1 hunks)components/quickbooks/actions/create-invoice/create-invoice.mjs
(1 hunks)components/quickbooks/actions/create-payment/create-payment.mjs
(1 hunks)components/quickbooks/actions/create-pl-report/create-pl-report.mjs
(1 hunks)components/quickbooks/actions/create-purchase-order/create-purchase-order.mjs
(1 hunks)components/quickbooks/actions/create-purchase/create-purchase.mjs
(1 hunks)components/quickbooks/actions/create-sales-receipt/create-sales-receipt.mjs
(1 hunks)components/quickbooks/actions/delete-purchase/delete-purchase.mjs
(1 hunks)components/quickbooks/actions/get-bill/get-bill.mjs
(1 hunks)components/quickbooks/actions/get-customer/get-customer.mjs
(1 hunks)components/quickbooks/actions/get-invoice/get-invoice.mjs
(1 hunks)components/quickbooks/actions/get-my-company/get-my-company.mjs
(1 hunks)components/quickbooks/actions/get-payment/get-payment.mjs
(1 hunks)components/quickbooks/actions/get-purchase-order/get-purchase-order.mjs
(1 hunks)components/quickbooks/actions/get-purchase/get-purchase.mjs
(1 hunks)components/quickbooks/actions/get-sales-receipt/get-sales-receipt.mjs
(1 hunks)components/quickbooks/actions/get-time-activity/get-time-activity.mjs
(1 hunks)components/quickbooks/actions/search-accounts/search-accounts.mjs
(1 hunks)components/quickbooks/actions/search-customers/search-customers.mjs
(1 hunks)components/quickbooks/actions/search-invoices/search-invoices.mjs
(1 hunks)components/quickbooks/actions/search-items/search-items.mjs
(1 hunks)components/quickbooks/actions/search-products/search-products.mjs
(1 hunks)components/quickbooks/actions/search-purchases/search-purchases.mjs
(1 hunks)components/quickbooks/actions/search-query/search-query.mjs
(1 hunks)components/quickbooks/actions/search-services/search-services.mjs
(1 hunks)components/quickbooks/actions/search-time-activities/search-time-activities.mjs
(1 hunks)components/quickbooks/actions/search-vendors/search-vendors.mjs
(1 hunks)components/quickbooks/actions/send-estimate/send-estimate.mjs
(1 hunks)components/quickbooks/actions/send-invoice/send-invoice.mjs
(1 hunks)components/quickbooks/actions/sparse-update-invoice/sparse-update-invoice.mjs
(1 hunks)components/quickbooks/actions/update-customer/update-customer.mjs
(1 hunks)components/quickbooks/actions/update-estimate/update-estimate.mjs
(1 hunks)components/quickbooks/actions/update-invoice/update-invoice.mjs
(1 hunks)components/quickbooks/actions/update-item/update-item.mjs
(1 hunks)components/quickbooks/actions/void-invoice/void-invoice.mjs
(1 hunks)components/quickbooks/common/constants.mjs
(1 hunks)components/quickbooks/common/utils.mjs
(1 hunks)components/quickbooks/package.json
(1 hunks)components/quickbooks/sources/new-customer-created/new-customer-created.mjs
(1 hunks)components/quickbooks/sources/new-customer-updated/new-customer-updated.mjs
(1 hunks)components/quickbooks/sources/new-employee-created/new-employee-created.mjs
(1 hunks)components/quickbooks/sources/new-employee-updated/new-employee-updated.mjs
(1 hunks)components/quickbooks/sources/new-invoice-created/new-invoice-created.mjs
(1 hunks)components/quickbooks/sources/new-invoice-updated/new-invoice-updated.mjs
(1 hunks)components/quickbooks/sources/new-item-created/new-item-created.mjs
(1 hunks)components/quickbooks/sources/new-item-updated/new-item-updated.mjs
(1 hunks)components/quickbooks/sources/new-purchase-created/new-purchase-created.mjs
(1 hunks)components/quickbooks/sources/new-purchase-updated/new-purchase-updated.mjs
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
components/quickbooks/common/utils.mjs (2)
components/quickbooks/quickbooks.app.mjs (1)
requestFn
(565-578)components/quickbooks/common/constants.mjs (4)
MAX_RETRIES
(2-2)MAX_RETRIES
(2-2)INITIAL_BACKOFF_MILLISECONDS
(3-3)INITIAL_BACKOFF_MILLISECONDS
(3-3)
⏰ 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). (3)
- GitHub Check: Lint Code Base
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
🔇 Additional comments (56)
components/quickbooks/actions/create-purchase/create-purchase.mjs (2)
9-9
: Version bump aligns with retry/backoff rolloutNo functional changes here. The metadata update is consistent with the broader QuickBooks package updates in this PR.
117-129
: Enhanced rate-limit retry logic in createPurchase confirmedThe
createPurchase
action routes through the shared_makeRequest
helper, which wraps requests inretryWithExponentialBackoff
with 429/503 detection and Retry-After support:
- common/utils.mjs (lines 48–74):
· ExportsretryWithExponentialBackoff
· Detects status 429/503 andError…code === "3200"
· Applies exponential backoff (initial 2 s, capped at 60 s) and honorsRetry-After
- quickbooks.app.mjs (lines 1–5, 577–580):
· ImportsretryWithExponentialBackoff
· Implements_makeRequest(opts)
by constructingrequestFn
and returningawait retryWithExponentialBackoff(requestFn)
- create-purchase.mjs (lines 117–129):
· Callsthis.quickbooks.createPurchase
, which delegates to_makeRequest
All QuickBooks client calls, including
createPurchase
, now leverage the updated retry/backoff logic.components/quickbooks/actions/sparse-update-invoice/sparse-update-invoice.mjs (1)
9-9
: Version bump only — consistent with package-wide updatesNo logic changes. Version increment appears to be part of the coordinated release.
components/quickbooks/actions/search-purchases/search-purchases.mjs (1)
7-7
: Version bump looks goodNo functional code touched. This aligns with the coordinated QuickBooks integration release.
components/quickbooks/actions/delete-purchase/delete-purchase.mjs (2)
7-7
: Version update approvedNo logic changes in this action. Version bump is consistent with the broader QuickBooks updates.
21-35
: Sanity-check: ensure upstream calls use enhanced retry/backoffGiven rate-limit objectives, please confirm
getPurchase
anddeletePurchase
calls funnel through the updated retry logic.You can re-use the script shared in the create-purchase comment; it checks these methods within quickbooks.app.mjs and the presence of retry handling in common utils.
components/quickbooks/actions/get-payment/get-payment.mjs (2)
8-8
: Version metadata bump acknowledgedNo functional changes. Consistent with the integration-wide release.
24-31
: Confirm getPayment path benefits from new 429/Retry-After handlingNon-blocking: Ensure
this.quickbooks.getPayment
internally uses the enhanced backoff/retry, so rate-limited reads auto-retry.Use the earlier verification script to confirm
getPayment
implementation references the shared retry/backoff utility.components/quickbooks/actions/search-time-activities/search-time-activities.mjs (1)
8-8
: Version bump only — LGTMNo behavioral changes; the action continues to route calls through the app client, which will pick up the enhanced retry/backoff logic from common utils.
components/quickbooks/actions/send-invoice/send-invoice.mjs (1)
7-7
: Version bump only — LGTMNo runtime logic changes; outbound requests still go through the QuickBooks app client where the updated rate-limit handling applies.
components/quickbooks/sources/new-employee-created/new-employee-created.mjs (1)
9-9
: Version bump only — LGTMNo functional changes to the polling/query logic; will benefit from shared retry/backoff improvements at the app/request layer.
components/quickbooks/actions/create-bill/create-bill.mjs (1)
9-9
: Version bump only — LGTMNo changes to validation or request construction; request continues to flow through the shared client with improved rate-limit handling.
components/quickbooks/actions/update-item/update-item.mjs (2)
8-8
: Version bump only — LGTMThe metadata bump to 0.0.5 looks good and aligns with the PR’s global retry/backoff enhancements.
286-289
: All retry/backoff enhancements are in place for QuickBooks callsI verified that the shared client implements the requested enhancements:
- components/quickbooks/common/constants.mjs
• INITIAL_BACKOFF_MILLISECONDS is set to 2000- components/quickbooks/common/utils.mjs
• retryWithExponentialBackoff detects HTTP 429/503 and QuickBooks error codes 3200/10001
• Honors theRetry-After
header when present, otherwise uses exponential backoff
• Backoff is capped at 60000msNo further action needed here.
components/quickbooks/actions/create-pl-report/create-pl-report.mjs (2)
10-10
: Version bump only — LGTMNo functional changes; version bump to 0.0.6 is consistent with the shared backoff updates.
119-138
: Sanity-check retries on report endpointsReport endpoints are frequent rate-limit candidates. Ensure the shared client’s enhanced retry/backoff logic applies here without additional per-action changes.
components/quickbooks/actions/create-ap-aging-report/create-ap-aging-report.mjs (1)
8-8
: Version bump only — LGTMMetadata update to 0.0.6 looks good. No logic changes detected.
components/quickbooks/actions/create-payment/create-payment.mjs (1)
7-7
: Version bump only — LGTMBumps to 0.0.10 with no functional changes. Consistent with PR scope.
components/quickbooks/actions/create-estimate/create-estimate.mjs (1)
12-12
: Version bump only — LGTMMetadata update to 0.0.3 is consistent with global changes.
components/quickbooks/actions/get-bill/get-bill.mjs (2)
8-8
: Version bump aligns with shared retry/rate-limit enhancements.Metadata-only change looks good and is consistent with the PR’s intent to centralize rate-limit handling in shared utils.
24-27
: Good: Passing$
to the client ensures consistent logging/retry context.This allows shared utils to log and apply retry/backoff behavior per request. No changes needed here.
components/quickbooks/sources/new-customer-created/new-customer-created.mjs (1)
9-9
: Version bump is appropriate; behavior remains delegated to common base.No logic changes needed here given rate-limit handling lives in shared utils and base methods.
components/quickbooks/actions/search-accounts/search-accounts.mjs (1)
8-8
: Version bump only; OK to ship.Fits the broad metadata update across actions in this PR.
components/quickbooks/actions/get-my-company/get-my-company.mjs (1)
7-7
: Version bump only; consistent with package and shared utils updates.No code changes required in this action for the rate-limit improvements.
components/quickbooks/actions/get-invoice/get-invoice.mjs (2)
8-8
: Version bump is fine; functionality remains unchanged.Matches the PR pattern of deferring rate-limit logic to shared utilities.
24-27
: Good:$
context is forwarded to the client.This supports shared retry/backoff logic and logging.
components/quickbooks/actions/search-products/search-products.mjs (1)
8-8
: ✅ Verification Complete: Global Rate-Limit and Backoff Updates Present, Version Bump Correct
- components/quickbooks/common/constants.mjs uses
INITIAL_BACKOFF_MILLISECONDS = 2000
- components/quickbooks/common/utils.mjs detects status 429/503, error codes 3200/10001, and handles
Retry-After
- The new
version: "0.1.11"
in search-products.mjs aligns with other actions updated to 0.1.11LGTM—no further changes needed.
components/quickbooks/actions/create-sales-receipt/create-sales-receipt.mjs (1)
9-9
: Version bump to 0.0.10 — metadata-only change. LGTM.No runtime or error-handling changes in this action; shared retry/backoff updates will apply via the app client.
components/quickbooks/actions/void-invoice/void-invoice.mjs (1)
8-8
: Version bump to 0.0.3 — looks good.No functional diffs; version aligns this action with the shared rate-limit/retry improvements.
components/quickbooks/actions/create-customer/create-customer.mjs (1)
8-8
: Version bump to 0.1.11 — approved.No logic changes; consistent with package-wide updates.
components/quickbooks/sources/new-item-updated/new-item-updated.mjs (1)
9-9
: Version bump to 0.0.8 — approved.No source behavior changes; shared backoff/rate-limit handling applies via the QuickBooks client.
components/quickbooks/actions/get-customer/get-customer.mjs (1)
8-8
: Approve version bump to 0.3.11 and rate-limit logic update
All checks passed:
- constants.mjs:
INITIAL_BACKOFF_MILLISECONDS = 2000
- utils.mjs: 429/503/Retry-After/3200/10001 handling & backoff cap at 60000 ms
- components/quickbooks/package.json: version “0.7.1”
- get-customer.mjs: version “0.3.11”
No further action required.
components/quickbooks/actions/search-customers/search-customers.mjs (1)
8-8
: Version bump to 0.1.11 — LGTM.No logic changes. Since this action queries via the shared QuickBooks client, it will benefit from the enhanced 429/Retry-After handling once published.
components/quickbooks/actions/search-query/search-query.mjs (1)
8-8
: Version bump to 0.1.11 — LGTM.No functional changes in the action; safe metadata update aligned with the package-wide rate limit improvements.
components/quickbooks/actions/update-estimate/update-estimate.mjs (1)
12-12
: Version bump to 0.0.3 — LGTM.Action logic unchanged. Publishing with the updated common utils should improve resilience to throttling during update calls.
components/quickbooks/actions/update-invoice/update-invoice.mjs (1)
12-12
: Version bump to 0.0.3 — LGTM.No behavior changes here. The shared retry/backoff enhancements will apply to both the get and update calls used by this action once released.
components/quickbooks/actions/search-vendors/search-vendors.mjs (1)
8-8
: LGTM: version bump only (no functional changes).This aligns with the broader QuickBooks package bump and enhanced rate-limit handling centralized in common utilities.
components/quickbooks/actions/create-invoice/create-invoice.mjs (1)
9-9
: LGTM: version bump only (no functional changes).No run-logic changes; version aligns with the package update and centralized retry/backoff improvements.
components/quickbooks/sources/new-purchase-updated/new-purchase-updated.mjs (1)
9-9
: LGTM: version bump only.No behavioral changes to polling/query generation. This remains compatible with the centralized retry logic.
components/quickbooks/actions/get-time-activity/get-time-activity.mjs (1)
8-8
: LGTM: version bump only (no functional changes).The action continues to defer HTTP behavior to the QuickBooks app layer where the new rate-limit handling resides.
components/quickbooks/actions/search-services/search-services.mjs (1)
8-8
: LGTM: version bump only (no functional changes).Consistent with the rest of the actions’ metadata updates.
components/quickbooks/sources/new-customer-updated/new-customer-updated.mjs (1)
9-9
: Version bump aligns with the rate-limit handling release. LGTM.No behavioral changes here; the version bump is consistent with the package-wide update and the enhanced retry/backoff improvements elsewhere.
components/quickbooks/actions/update-customer/update-customer.mjs (1)
8-8
: Version metadata update looks good.This action will benefit from the centralized QuickBooks client/backoff improvements without any local changes. No issues spotted.
components/quickbooks/sources/new-employee-updated/new-employee-updated.mjs (1)
9-9
: Version bump approved.No logic changes; consistent with the broader QuickBooks release set.
components/quickbooks/actions/create-purchase-order/create-purchase-order.mjs (1)
12-12
: Version bump approved.Action logic unchanged and will inherit the updated retry/rate-limit behavior from the shared client.
components/quickbooks/actions/search-items/search-items.mjs (2)
8-8
: Version bump approved.No runtime or interface changes; aligns with the package-wide update for rate-limit handling.
8-8
: Rate-limit handling successfully verified
All enhancements are correctly wired through the QuickBooks client:
- common/utils.mjs detects HTTP 429/503 and QuickBooks error codes 3200/10001 and reads the
Retry-After
header.- common/constants.mjs defines
INITIAL_BACKOFF_MILLISECONDS = 2000
.- retryWithExponentialBackoff doubles the delay and caps it at 60000 ms.
- quickbooks.app.mjs methods (
query
,updateCustomer
,createPurchaseOrder
, etc.) import and call the sharedretryWithExponentialBackoff
wrapper around API requests.components/quickbooks/actions/get-purchase/get-purchase.mjs (1)
8-8
: Version bump correctly picks up shared backoff & rate-limit enhancements — LGTMVerified in the shared QuickBooks modules:
- components/quickbooks/common/constants.mjs:
INITIAL_BACKOFF_MILLISECONDS
is set to2000
.- components/quickbooks/common/utils.mjs: rate-limit errors include HTTP 429/503 and QuickBooks codes “3200”/“10001”, honor
Retry-After
, and cap exponential backoff at60000ms
.components/quickbooks/sources/new-purchase-created/new-purchase-created.mjs (1)
9-9
: Version metadata update only — LGTMNo functional changes. This keeps the source aligned with the shared retry/backoff improvements.
components/quickbooks/sources/new-item-created/new-item-created.mjs (1)
9-9
: Version metadata update only — LGTMNo behavioral changes; consistent with the PR’s scope.
components/quickbooks/sources/new-invoice-updated/new-invoice-updated.mjs (1)
9-9
: Version bump aligns this source with enhanced shared rate-limit handling.Incrementing the version is appropriate to republish the source so it benefits from the updated backoff/rate-limit logic in common utils.
components/quickbooks/actions/get-purchase-order/get-purchase-order.mjs (2)
8-8
: Version bump looks good.Keeps the action in sync with the shared retry/backoff improvements and ensures republish.
30-32
: No change: retain"summary"
export key for QuickBooks actions
The export key for summaries in QuickBooks actions is overwhelmingly"summary"
(without the leading$
). Only a small subset of actions currently use"$summary"
, so switching this file alone would introduce more inconsistency. If you’d like to standardize on"$summary"
, please open a dedicated refactoring PR to update all QuickBooks actions at once.Likely an incorrect or invalid review comment.
components/quickbooks/sources/new-invoice-created/new-invoice-created.mjs (1)
9-9
: Version bump LGTM.Ensures this source repackages with the updated shared rate-limit/backoff behavior.
components/quickbooks/actions/send-estimate/send-estimate.mjs (1)
7-7
: Version bump LGTM.Good to republish this action with the enhanced shared retry/backoff logic.
components/quickbooks/common/utils.mjs (1)
49-52
: LGTM on signature formattingSplitting parameters across lines improves readability; no functional change. Defaults align with constants.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
components/quickbooks/common/utils.mjs (1)
56-66
: Good catch: normalizing QuickBooks error codes avoids missed rate-limit detectionConverting the QuickBooks error code to a string before comparison prevents mismatches when the API returns numeric codes. The broadened detection (429, 503, "3200", "10001") looks solid.
To double-check assumptions about QuickBooks-specific codes and header formats, please run this web query:
QuickBooks Online API rate limiting: confirm whether error codes 3200 and 10001 are official rate-limit/throttle signals, and whether Retry-After is seconds or an HTTP-date.
🧹 Nitpick comments (1)
components/quickbooks/common/utils.mjs (1)
49-51
: Signature formatting is fine; consider adding JSDoc for intent and typesMinor nit: documenting the contract for
requestFn
and the units/behavior forretries
/backoff
will make this utility safer to reuse.Example JSDoc to place above the function:
/** * Retries an async request function with exponential backoff. * * @param {() => Promise<any>} requestFn - Function that performs the request; must throw on failure. * @param {number} [retries=MAX_RETRIES] - Remaining retry attempts. * @param {number} [backoff=INITIAL_BACKOFF_MILLISECONDS] - Current backoff delay in milliseconds. * @returns {Promise<any>} */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
components/quickbooks/actions/search-vendors/search-vendors.mjs
(2 hunks)components/quickbooks/common/utils.mjs
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- components/quickbooks/actions/search-vendors/search-vendors.mjs
🧰 Additional context used
🧬 Code Graph Analysis (1)
components/quickbooks/common/utils.mjs (2)
components/quickbooks/quickbooks.app.mjs (1)
requestFn
(565-578)components/quickbooks/common/constants.mjs (4)
MAX_RETRIES
(2-2)MAX_RETRIES
(2-2)INITIAL_BACKOFF_MILLISECONDS
(3-3)INITIAL_BACKOFF_MILLISECONDS
(3-3)
⏰ 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: Verify TypeScript components
- GitHub Check: Publish TypeScript components
- GitHub Check: pnpm publish
- GitHub Check: Lint Code Base
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @michelle0927, LGTM! Ready for QA!
Resolves #18041
Will create a follow-up PR for
quickbooks_sandbox
after this is published.Summary by CodeRabbit
Bug Fixes
Chores