Automatically retrying on 503 errors #556
Conversation
hiranya911
commented
Jun 6, 2019
hiranya911
commented
- Increased retry count to 4
- Backoff factor set to 0.5 (this results in the delay pattern of 0s, 1s, 2s and 4s)
| const scope = nock('https://' + mockHost) | ||
| .get(mockPath) | ||
| .twice() | ||
| .times(5) |
There was a problem hiding this comment.
I think it is better to test passing explicit RetryConfig instead so you don't have to do things like:
(client as any).retry.backOffFactor = 0;
This also makes it easier to add changes to default config in the future.
And then add a test for default config.
test/unit/utils/api-request.spec.ts
Outdated
| maxDelayInMillis: 1000, | ||
| statusCodes: [503], | ||
| }); | ||
| const client = new HttpClient(); |
There was a problem hiding this comment.
Shouldn't this use testRetryConfig() ?
test/unit/utils/api-request.spec.ts
Outdated
| maxDelayInMillis: 60 * 1000, | ||
| statusCodes: [503], | ||
| }); | ||
| const client = new HttpClient(); |
There was a problem hiding this comment.
Should you pass explicit test retry config with backoff factor on, here and elsewhere?
There was a problem hiding this comment.
Used defaultRetryConfig() directly.
| * | ||
| * @return {RetryConfig} A new RetryConfig instance. | ||
| */ | ||
| function testRetryConfig(): RetryConfig { |
There was a problem hiding this comment.
should this take a parameter whether to reset backOffFactor and then you can use it in all tests not testing default behavior but require backOffFactor?
There was a problem hiding this comment.
That's same as default config. So using defaultRetryConfig() directly.
|
@bojeil-google I used |
2 participants
