fix: client no longer delays when it wont retry #51
Conversation
| @@ -360,7 +360,7 @@ function retry(fn, ctx) { | |||
| function attempt(i) { | |||
| return fn(ctx) | |||
| .catch((err) => { | |||
| if (maxAttempts > 0) { | |||
| if (i < maxAttempts && isCriticalError(err) && ctx.retryDelay && ctx.retryDelay > 0) { | |||
There was a problem hiding this comment.
I wonder whether it's worth leaving a comment explaining the rationale here? Perhaps even something as simple as (just so that you can take a quick glance at the code and understand what it does straight away):
// Only critical errors should retry
There was a problem hiding this comment.
Re: critical errors. In this instance isn't it clear from the code that that's the case?
There was a problem hiding this comment.
@cjewell47 there are a lot of other conditions chained in the if statement, but none of them matter unless the error is critical (which I think is the main fix/change in this PR). The comment would just highlight that importance, so that someone new to the code takes note 😃
There was a problem hiding this comment.
Thanks guys, we have added some jsdoc to make this a bit clearer
|
Can we please have some tests added for this |
Description
In the retry function, delay was firing without hitting retry, resulting in increased response times. We modified the function to only delay when the retry occurs.
Motivation and Context
Jira ticket
Types of changes
Checklist: