fix: correct retry backoff units - #960
roshnicdave wants to merge 1 commit into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #960 +/- ##
==========================================
- Coverage 45.52% 41.62% -3.90%
==========================================
Files 19 19
Lines 5520 7238 +1718
Branches 360 461 +101
==========================================
+ Hits 2513 3013 +500
- Misses 3003 4142 +1139
- Partials 4 83 +79 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Ran into this same bug independently and opened #961 (issue, with a reproduction) and #962 (PR) before finding this one — my mistake, I searched issues but not pull requests. I have closed #962; this PR was first and is the one that should land. Two small things from mine that might be worth folding in here, entirely up to you and @KnorpelSenf: 1. The docstring ceiling no longer matches the constant.
Before the fix that was wrong in the other direction (the ceiling was effectively ~13.9 days). After the fix the real ceiling is 20 minutes, matching the named constant 2. Why the suite never caught it, which might be worth a line in the PR body: the two existing retry tests assert #961 has a self-contained reproduction that needs no Telegram token and no network access to Happy to help however is useful — reviewing, testing, or nothing at all. |
|
This is an ugly fix. Instead of converting the units back and forth several times, we should instead refactor the code to use time units consistently everywhere. Also, the tests are just slop. I will do this myself. |
Fixes generic retry backoff delays being treated as seconds. Telegram
retry_aftervalues remain in seconds.Tests:
deno task test,deno task check,deno fmt --check,deno lint.