fix: non-zero delay between first attempt and first retry for linear and exp strategy #163
Conversation
See issue JustinBeckwith#122. This is to confirm that the delay between the first (actual) attempt and the first retry is not zero, but the configured delay. tests: prettier format (noop) tests: prettier format (noop)
The mutation did not affect the code below. This is for issue JustinBeckwith#122. calc delay: prettier (format)
jgehrcke
changed the title
Hm. Should not even be needed. https://nodejs.org/docs/latest-v10.x/api/process.html#process_process_hrtime_bigint added in: v10.7.0. Dropping that commit. |
Found that state is not retained across retries when mutating `config.currentRetryAttempt` here.
jgehrcke
force-pushed
the
jp/delay-between-first-attempt-and-first-retry
branch
from
9efeff2
to
c059361
Compare
|
Only problem - looks like you need to run an |
This is to address the `gts fix` error
The 'process.hrtime.bigint' is not supported
until Node.js 10.7.0. The configured version
range is '>=10.0.0'
This function is only used in the test suite
so maybe it's not the best solution to change
the NodeJS version requirement for the
entire package. However, whoever runs this on
NodeJS older than 10.7.0 (released on
2018-07-18) might want to be encouraged to
use a more recent version.
Thank you. Addressed the "lint issue" pragmatically now. See commit message. Let me know if you disagree, can probably be solved differently w/o touching the |
|
Heh, curiosity killed the cat - what code did you write that ran afoul of the node version linter? |
Not sure if I really understand the question but I hope that the commit message of my last commit answers it. If it doesn't then I would appreciate clarification! :) (maybe a tiny bit of clarification on top of the commit message: the new tests rely on |
|
🎉 This PR is included in version 2.5.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
It introduced a delay between first attempt and first retry: JustinBeckwith/retry-axios#163 Therefore tests with retry takes more time then before. The longest run took 2s 33ms.
It introduced a delay between first attempt and first retry: JustinBeckwith/retry-axios#163 Therefore tests with retry takes more time then before. The longest run took 2s 33ms.
One of the issues discussed in #122 is that for the
linearandexponentialstrategy the delay between the first and second HTTP request is 0.This PR:
(err.config as RaxConfig).raxConfig!.currentRetryAttempt! += 1;to before doing the math that uses the result.retrycountand adds a comment explaining what it really means to make future work on this code section a little more straight-forward.Note that for measuring time difference I wanted to use a monotonic time source and the "modern"
process.hrtime.bigint()and therefore I was bumping theengineinpackage.json.The duration of
npm run testchanged from ~7 s to ~27 s because of the delay now hitting in.@JustinBeckwith I would appreciate review!
Note that next up we can maybe change the time constants for the exponential backoff as was also proposed in #122; to make the first delay a little smaller than 0.5 seconds.