Adjust rate limiting dynamically based on API response #48
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
newhouse
force-pushed
the
newhouse/2220/rate-limiting-improvements
branch
from
f689c65 to
f9c775b
Compare
benogle
approved these changes
May 26, 2021
| @@ -53,7 +53,7 @@ | |||
| "abort-controller": "^3.0.0", | |||
| "extract-files": "^6", | |||
| "form-data": "^3.0.0", | |||
| "limiter": "^1.1.5", | |||
| "limiter": "^2.1.0", | |||
Comment on lines
+509
to
+518
| const remainingRequests = await this.limiter.removeTokens(1) | ||
| if (remainingRequests < 1) { | ||
| await sleep(this.requestLimitMS + failBufferMS) | ||
| } | ||
| const retry = async (ms) => { | ||
| await sleep(ms) | ||
| return this._throttle(fn) | ||
| } | ||
|
|
||
| return fn(retry) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of the change
So that the rate limiting in the Node Client tries to do a good job, I've done 2 things here:
1. Allow the User to pass akeyTypeoption to the constructor that will determine what rate limiting options it will try to do2. Leverage the response headers from the server to adjust the rate limiter if necessary, so in the end, the User doesn't really need to know/do anything.
This 2nd one above deserves some discussion/thought:
An additional benefit of this is that if we ever change our rate limits, or want to change them per-client/api key, the code will "just work"™️ for them in the future.
If we like this, perhaps I should actually rip all the stuff where the User can even specify rate limit stuff (requestLimit,requestLimitMS,keyType), and let it all come from the server?☝️ we did this
Type of change
Related issues
Relates to https://github.com/anvilco/anvil/issues/2220
Checklists
Development
Code review