chore(PSE): PSE-2 Add rate limiting support to BigCommerce API client#9
chore(PSE): PSE-2 Add rate limiting support to BigCommerce API client#9jfugalde wants to merge 1 commit intobigcommerce:masterfrom
Conversation
- Introduced a semaphore and token bucket for managing concurrent requests and rate limiting. - Enhanced the Request class to handle maxRetries for rate limit errors. - Updated tests to verify the new rate limiting functionality and ensure proper semaphore management. - Added configuration options for maxConcurrent and requestsPerSecond in the BigCommerce constructor.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f4555584c5
ℹ️ 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".
| if (this._tokenBucket) await this._tokenBucket.consume(); | ||
| if (this._semaphore) await this._semaphore.acquire(); |
There was a problem hiding this comment.
Consume rate tokens after semaphore acquisition
request() currently calls this._tokenBucket.consume() before waiting on this._semaphore.acquire(), so when both limiter options are enabled, requests can burn tokens while queued on concurrency and then burst once a slot opens. In practice, with maxConcurrent=1 and requestsPerSecond=1, one long request followed by several short ones can start multiple queued calls within the same second, violating the configured outbound rate and causing avoidable 429s. Acquire the semaphore first (or re-check token availability immediately before dispatch) so pacing reflects actual send time.
Useful? React with 👍 / 👎.
Jira: PSE-2
What?
A description about what this pull request implements and its purpose. Try to be detailed and describe any technical details to simplify the job of the reviewer and the individual on production support.
Why?
Adding rate limit capability so that apps can manage this in a graceful way
...
Testing / Proof
` BigCommerce
#constructor
✓ should return an error if config is missing
✓ should save config to the object
✓ should set api version to a default
given limiter config
✓ should create a semaphore when maxConcurrent is provided
✓ should create a token bucket when requestsPerSecond is provided
✓ should create both gates when both options are provided
✓ should not create any gates when limiter is omitted
#verify
✓ should return the JSON data
given a null signed request
✓ should return null
given a signed request without a full stop
✓ should return null
given an invalid signature
✓ should return null
given an invalid signature (different length)
✓ should return null
#verifyJWT
✓ should return the JSON data
given a null JWT
✓ should return error
given an invalid signature
✓ should return an error
#constructJWTFromAuthData
given auth callback data
✓ should return a valid jwt
#createCustomerLoginJWT
given a customer ID and channel ID
✓ should return a valid jwt
#authorize
✓ should return an object
when the query params are missing
✓ should return an error
when the authorization fails
✓ should return and error
#createAPIRequest
✓ should create a request object with the correct headers
✓ should have the correct API hostname
#request
✓ should make a call to the request object
✓ should use v3 if specified in config
when the header requirements are not met
✓ should return an error
when the response type is xml
✓ should call the request object with extension .xml
when the response type is json
✓ should make a call to the request object with an empty extension
#get
✓ should make a request with the correct arguments
#post
✓ should make a request with the correct arguments
#put
✓ should make a request with the correct arguments
#delete
✓ should make a request with the correct arguments
#request with limiter
✓ should pass maxRetries config through to the Request via createAPIRequest
✓ should acquire and release the semaphore on each request
✓ should release the semaphore even when the request fails
✓ should make requests normally without pacing config
Semaphore
given a limit of 2
✓ should resolve immediately when under the limit
✓ should block the third acquire until a release
✓ should decrement running on release
✓ should process queued acquires in order
TokenBucket
given a rate of 10 tokens per second
✓ should consume up to capacity immediately
✓ should wait when tokens are exhausted (102ms)
✓ should refill tokens over time (201ms)
✓ should not exceed capacity on refill (302ms)`
@bigcommerce/api-client-developers