-
Notifications
You must be signed in to change notification settings - Fork 97
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: add exponential backoff and retries for push/send #52
Conversation
ac19898
to
45c023a
Compare
45c023a
to
70047b2
Compare
Codecov Report
@@ Coverage Diff @@
## master #52 +/- ##
==========================================
- Coverage 99.35% 98.77% -0.58%
==========================================
Files 1 1
Lines 154 163 +9
Branches 28 29 +1
==========================================
+ Hits 153 161 +8
Misses 1 1
- Partials 0 1 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
@@ -6,21 +6,6 @@ afterEach(() => { | |||
(fetch as any).reset(); | |||
}); | |||
|
|||
test('limits the number of concurrent requests', async () => { |
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.
The addition of promiseRetry means that the promiseLimit is no longer the first awaited call, so this test is no longer reliable.
Noted inline about why
|
I was unable to get the mock timers to work unfortunately. That being said, these seem to finish in under the default timeout, so I'm not too worried. |
My primary concern is human time, not CI time. The unit tests are actually "fast, reliable tests" that should give feedback as fast as possible, so we never want to wait for timers with them. Another reasonable solution would be to make it so that the backoff in tests is always 0ms. |
Why
We'll be adding rate limiting to the push/send API soon, so before that it'd be good to have this library handle 429s gracefully.
How
Add
promise-retry
with 2 retries and exponential backoff.Test Plan
Run unit test.