Skip to content
This repository has been archived by the owner on Feb 13, 2024. It is now read-only.

#284 Added options for axiosRetryConfig, disable axiosRetry if retryCount is 0 #285

Merged
merged 3 commits into from Jul 28, 2021

Conversation

Dahaden
Copy link
Contributor

@Dahaden Dahaden commented Jun 30, 2021

Tried to write tests but found sinon (or at least sinon at the current version) was not able to spy or stub axiosRetry to allow me to check the options passed in (possibly I was testing the wrong way?)

@nd4p90x nd4p90x linked an issue Jun 30, 2021 that may be closed by this pull request
retryCondition: this._isErrorRetryable,
retryDelay: axiosRetry.exponentialDelay
})
if (options.retryCount !== 0) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here it should be === 0, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No? If the user sets a retryCount specifically of 0 then we do not need axiosRetry :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right. Sorry

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No worries :D Its always good to double check :)
Appreciate the review!

index.js Outdated Show resolved Hide resolved
Dahaden and others added 2 commits July 15, 2021 09:49
@nd4p90x nd4p90x added this to Needs Review in analytics-node Jul 26, 2021
@nd4p90x nd4p90x requested a review from pooyaj July 26, 2021 15:14
@pooyaj pooyaj merged commit b13490a into segmentio:master Jul 28, 2021
analytics-node automation moved this from Needs Review to Done Jul 28, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

Add jitter to retry timeout
4 participants