-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
feat(utils): Introduce rate limit helpers #4685
Conversation
size-limit report
|
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.
A few naming nits, but otherwise looks good!
}; | ||
const headers = {}; | ||
const updatedRateLimits = updateRateLimits(rateLimits, headers); | ||
expect(rateLimits).toStrictEqual(updatedRateLimits); |
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.
expect(rateLimits).toStrictEqual(updatedRateLimits); | |
expect(updatedRateLimits).toStrictEqual(rateLimits); |
Our expectation is, effectively, "the result will be correct," so I think it makes more sense to swap these. Also, I'm curious why you're using strict equal here over equal. Are you testing that they didn't get converted to strings?
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.
updatedRateLimits
and rateLimits
should be rate limits objects. I guess we don't need the undefined
checks from toStrictEqual
, I'll just update it
packages/utils/src/ratelimit.ts
Outdated
const rlHeader = headers['x-sentry-rate-limits']; | ||
const raHeader = headers['retry-after']; |
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.
const rlHeader = headers['x-sentry-rate-limits']; | |
const raHeader = headers['retry-after']; | |
const rateLimitHeader = headers['x-sentry-rate-limits']; | |
const retryAfterHeader = headers['retry-after']; |
You know me and spelling things out... 😛
Co-authored-by: Katie Byers <lobsterkatie@gmail.com>
Co-authored-by: Katie Byers <lobsterkatie@gmail.com>
Co-authored-by: Katie Byers <lobsterkatie@gmail.com>
This patch converts the browser based transport to use the ratelimit helpers introduced in #4685
Ref: #4660
As we move toward introducing a new transport API, the first step is to validate rate-limiting logic. We do this by creating a new set of functional helpers that mutate a rate limits object.
In 5 seconds I will link a PR that does this conversion in browser.
All credits to @kamilogorek as 95% of this is from
v7-dev
!Resolves https://getsentry.atlassian.net/browse/WEB-660