Repository navigation
Conversation
HubSpot limits each account that installs a marketplace OAuth app to 110 requests per 10 seconds, and answers a throttled call with a 429 whose body names the limit in `policyName`. The Requester met every such 429 with its fixed 1, 3, 10, 30, 60 and 180 second ladder, whatever the response said. Declare `static rateLimit` so the Requester can use what the response says: - A burst 429 (`TEN_SECONDLY_ROLLING`) returns a reason-only hint. A real `Retry-After` then wins, and the 10 second burst window is the fallback. - A `DAILY` 429 returns a one hour probe (`source: 'static'`). HubSpot resets the daily limit at midnight in the account's time zone, which the module does not know, so no UTC reset time is computed. One hour is over the in-process budget, so the Requester throws a `RateLimitError` with `reason: 'daily'` and `retryAt` and does not sleep. - Any other status is not a limit. - `userHints.daily.links` points at the API usage guidelines. `maxConcurrency` (10) is a pacing figure that HubSpot does not document. It is declared for the pacing phase and nothing reads it yet. The tests use recorded-style 429 responses and a stubbed fetch. They need a `@friggframework/core` that exports `RateLimitError` (friggframework/frigg#656). The dependency range is unchanged until the release that ships it is known. Refs ADR-049. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Salesforce counts the API calls of an org over a rolling 24 hours. An org over its limit gets a 403 whose error code is `REQUEST_LIMIT_EXCEEDED`, and no reset time. The module drives jsforce, not the Requester, and a jsforce error has no `statusCode` (`HttpApiError`: `name` equals `errorCode`), so the Requester never sees it and the queue worker gets an opaque error. `REQUEST_LIMIT_EXCEEDED` is also the error for too many concurrent requests that run 20 seconds or longer, which clears in seconds. Reporting it as a daily limit would put a sync on hold for an hour. - Declare `static rateLimit`: a per-org daily rolling window and a `classify` that recognises `REQUEST_LIMIT_EXCEEDED`. Salesforce documents no field that tells the two limits apart, so `classify` reads the message: one that contains "concurrent" is a concurrency limit with a 30 second wait, and any other message is a daily limit. The message text is not documented, so the default stays daily. With no reset time to read, a daily limit gets a static one hour probe. Both reasons link to the API request limits page. - Add the public `withLimits(call)`. On a failed call it runs `classifyRateLimit` and throws a `RateLimitError`, with the jsforce error kept as `cause`. It rethrows any other error unchanged. The data methods use it. A consumer that calls `api.conn` directly wraps its own calls with `api.withLimits(() => api.conn.query(soql))`, as the README says. - Before it throws, `withLimits` tells the delegate with `_notifyRateLimited`, as the Requester does when it throws a `RateLimitError`, so the integration can record a warning for the user. Core writes that warning only for a limit longer than its in-process cap, so the daily limit gets one and the 30 second concurrency limit does not. A core without that method skips the call. - `module` on the error is `_telemetryModuleLabel()`, the label the Requester puts on its own `RateLimitError`. An API instance has no `name`. `RateLimitError` and `classifyRateLimit` come from `@friggframework/core` (friggframework/frigg#656) and are read at runtime. The dependency range is unchanged. On a core that does not export both yet, `withLimits` rethrows the original jsforce error, so the module can be released before an app upgrades its core. The token and refresh code is untouched, and `conn.limitInfo` is not read yet. Refs ADR-049. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Pipedrive limits an OAuth app per token over a rolling 2 second window and sends `x-ratelimit-reset` with the throttled response. A recorded 429 also carries `Retry-After`. The Requester met every such 429 with its fixed 1, 3, 10, 30, 60 and 180 second ladder. Declare `static rateLimit`: - `scope: 'entity'` and a burst window of 80 requests per 2 seconds. That is the lowest OAuth allowance in the docs (Lite 80, Growth 160, Premium 400, Ultimate 480). Pipedrive reports the real figure in `x-ratelimit-limit`. - `parsers: ['retryAfter', 'resetHeaders']`, the two header families Pipedrive sends. `x-ratelimit-reset` is a wait in seconds. - No `classify`. A spent daily token budget answers with a 429 and no documented wait time. Unless the response carries one of those headers, it keeps the fixed ladder and then the redelivery it has today. Add `RateLimitPolicy` and the types it uses to the module's `frigg-core.d.ts` shim, copied from the core types (friggframework/frigg#656). The subclass declares its own static, so the shim's `OAuth2Requester` needs no change. Core reads `x-ratelimit-reset` by default, so the wait tests characterise the behaviour with recorded headers, and the declaration test pins the numbers. The tests need a `@friggframework/core` with rate-limit awareness. The dependency range is unchanged until the release that ships it is known. Refs ADR-049. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
358b809 to
1b8f325
Compare
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1b8f32580a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| RateLimitError, | ||
| classifyRateLimit, |
There was a problem hiding this comment.
A clean npm ci still resolves the committed core versions (next.89 for HubSpot, next.75 for Pipedrive, and next.110 for Salesforce), which predate RateLimitError, classifyRateLimit, and requester support for static rateLimit. Consequently the new Salesforce/HubSpot tests receive undefined for these exports, while the HubSpot and Pipedrive policies are silently ignored in production. Update each module's core dependency and the lockfile to the release that provides this API before landing the feature.
Useful? React with 👍 / 👎.
| class Api extends OAuth2Requester { | ||
| static rateLimit = { | ||
| scope: 'entity', | ||
| windows: [{ name: 'burst', limit: 110, perMs: 10_000 }], |
There was a problem hiding this comment.
For calls such as companySearch, searchDeals, and the other /search methods in this class, HubSpot applies a separate limit of five requests per second per account. Declaring only the general 110-per-10-second window permits ten concurrent search requests and uses the 10-second general fallback after the resulting 429, so search-heavy integrations will still be predictably throttled despite this policy. Add a search-specific limiter or otherwise enforce the five-per-second window for these endpoints.
Useful? React with 👍 / 👎.
1 participant
The HubSpot, Salesforce and Pipedrive modules now tell the Frigg Requester their provider's limits and how to read a throttled response. HubSpot and Pipedrive declare a
static rateLimit. Salesforce also getswithLimits, because its calls go through jsforce, not through the Requester. This is phase 3 of ADR-049 (friggframework/frigg#655).Draft until the core release. This needs
RateLimitError,classifyRateLimitandstatic rateLimitfrom friggframework/frigg#656, and the user warning from friggframework/frigg#658. When that core is published: raise the@friggframework/corerange of each module, remove the older-core guards in SalesforcewithLimits, and mark this PR ready. Until then it is safe to release: an older core ignores the static, andwithLimitsrethrows the original jsforce error there.What each module declares
retryAfterandresetHeadersparsers.classifyreadspolicyNamefrom the 429 body:DAILYis a daily limit with a one hour probe. HubSpot resets it at midnight in the account's time zone, which the module cannot know.Retry-Afterwins and the 10 s window is the fallback.REQUEST_LIMIT_EXCEEDED(a 403 with nostatusCodein jsforce) is a daily limit with a one hour probe: the window is a rolling 24 hours and has no reset time. The same code means too many concurrent long-running requests; a message that contains "concurrent" is a concurrency limit with a 30 s wait. The data methods go through the publicwithLimits(call), and a caller that usesapi.connwraps its own calls.classify.Each README has a "Rate limits" section.
Behaviour change
Every HubSpot 429 now has a hint, so the first retry waits 10 s, or the
Retry-Afterwhen there is one, instead of the 1 s of the fixed ladder.Caveats
withLimitsran against real jsforce 3.10.14 and a local server that answersREQUEST_LIMIT_EXCEEDED.maxConcurrency: 10is the ADR's figure. No HubSpot doc gives one, and nothing reads it yet.resetHeadersdoes nothing for HubSpot today: HubSpot documents no reset header.RateLimitErrormessage readsGET <instance url> REQUEST_LIMIT_EXCEEDED, because jsforce does not say which verb failed. The jsforce error is thecause.packages/v1-ready/salesforce/streamHandler.jshas a Salesforce consumer key and secret in the source, and a deadnforceimport. Rotate that secret and delete the file in a separate PR.Test plan
npx jest tests/rateLimit.test.js: 9 pass.npm run test:unit: 79 pass (45 onorigin/next).npx vitest run test/rateLimit.test.ts: 4 pass.🤖 Generated with Claude Code
📦 Published PR as canary version:
Canary Versions✨ Test out this PR locally via:
npm install @friggframework/api-module-hubspot@1.2.0-canary.102.1b8f325.0 npm install @friggframework/api-module-pipedrive@2.1.0-canary.102.1b8f325.0 npm install @friggframework/api-module-salesforce@1.1.0-canary.102.1b8f325.0 # or yarn add @friggframework/api-module-hubspot@1.2.0-canary.102.1b8f325.0 yarn add @friggframework/api-module-pipedrive@2.1.0-canary.102.1b8f325.0 yarn add @friggframework/api-module-salesforce@1.1.0-canary.102.1b8f325.0