Skip to content

Commit c19ac3c

Browse files
authored
fix: use monotonic clock for elapsed time and handle nil max_elapsed_time (#122)
* fix: use monotonic clock for elapsed time and handle nil max_elapsed_time - Replace Time.now with Process.clock_gettime(CLOCK_MONOTONIC) so retry timing is immune to wall-clock adjustments (NTP, manual changes). - Guard can_retry? against max_elapsed_time: nil, which previously raised NoMethodError on the comparison path. - Remove dead '* 1.0' in ExponentialBackoff#randomize (rand_factor is already a float). - Add regression tests for both fixes. * address PR review: fix float coercion, strengthen tests, document nil max_elapsed_time * docs: add changelog entries for monotonic clock and nil max_elapsed_time fixes
1 parent b828349 commit c19ac3c

5 files changed

Lines changed: 45 additions & 6 deletions

File tree

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
# HEAD
22

3+
- Fix: Use `Process.clock_gettime(CLOCK_MONOTONIC)` for elapsed time tracking so retry timing is immune to wall-clock adjustments (NTP, manual changes).
4+
- Fix: Handle `max_elapsed_time: nil` gracefully instead of raising `NoMethodError`.
5+
- Remove dead `* 1.0` float coercion in `ExponentialBackoff#randomize`.
6+
37
## 3.4.0
48

59
- Add `retry_if` option to support custom retry predicates, including checks against wrapped `exception.cause` values.

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ Here are the available options, in some vague order of relevance to most common
8787
| **`on_retry`** | `nil` | `Proc` to call after each try is rescued. [Read more](#callbacks). |
8888
| **`sleep_disabled`** | `false` | When true, disable exponential backoff and attempt retries immediately. |
8989
| **`base_interval`** | `0.5` | The initial interval in seconds between tries. |
90-
| **`max_elapsed_time`** | `900` (15 min) | The maximum amount of total time in seconds that code is allowed to keep being retried. |
90+
| **`max_elapsed_time`** | `900` (15 min) | The maximum amount of total time in seconds that code is allowed to keep being retried. Set to `nil` to disable the time limit and retry based solely on `tries`. |
9191
| **`max_interval`** | `60` | The maximum interval in seconds that any individual retry can reach. |
9292
| **`multiplier`** | `1.5` | Each successive interval grows by this factor. A multipler of 1.5 means the next interval will be 1.5x the current interval. |
9393
| **`rand_factor`** | `0.5` | The percentage to randomize the next retry interval time. The next interval calculation is `randomized_interval = retry_interval * (random value in range [1 - randomization_factor, 1 + randomization_factor])` |

lib/retriable.rb

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,8 @@ def retriable(opts = {}, &block)
4141

4242
exception_list = on.is_a?(Hash) ? on.keys : on
4343
exception_list = [*exception_list]
44-
start_time = Time.now
45-
elapsed_time = -> { Time.now - start_time }
44+
start_time = Process.clock_gettime(Process::CLOCK_MONOTONIC)
45+
elapsed_time = -> { Process.clock_gettime(Process::CLOCK_MONOTONIC) - start_time }
4646

4747
tries = intervals.size + 1
4848

@@ -101,7 +101,10 @@ def call_on_retry(on_retry, exception, try, elapsed_time, interval)
101101
end
102102

103103
def can_retry?(try, tries, elapsed_time, interval, max_elapsed_time)
104-
try < tries && (elapsed_time + interval) <= max_elapsed_time
104+
return false unless try < tries
105+
return true if max_elapsed_time.nil?
106+
107+
(elapsed_time + interval) <= max_elapsed_time
105108
end
106109

107110
# When `on` is a Hash, we need to verify the exception matches a pattern.

lib/retriable/exponential_backoff.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ def initialize(opts = {})
2828

2929
def intervals
3030
intervals = Array.new(tries) do |iteration|
31-
[base_interval * multiplier**iteration, max_interval].min
31+
[base_interval * (multiplier**iteration), max_interval].min
3232
end
3333

3434
return intervals if rand_factor.zero?
@@ -39,7 +39,7 @@ def intervals
3939
private
4040

4141
def randomize(interval)
42-
delta = rand_factor * interval * 1.0
42+
delta = rand_factor * interval.to_f
4343
min = interval - delta
4444
max = interval + delta
4545
rand(min..max)

spec/retriable_spec.rb

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -315,6 +315,38 @@ def increment_tries_with_exception(exception_class = nil)
315315
expect(@tries).to eq(2)
316316
end
317317

318+
it "retries up to tries limit when max_elapsed_time is nil" do
319+
expect do
320+
described_class.retriable(tries: 4, max_elapsed_time: nil) { increment_tries_with_exception }
321+
end.to raise_error(StandardError)
322+
323+
expect(@tries).to eq(4)
324+
end
325+
326+
it "uses monotonic clock for elapsed time tracking" do
327+
# Stub Process.clock_gettime to return controlled values so we can
328+
# verify elapsed_time passed to on_retry is derived from the monotonic clock.
329+
clock_calls = 0
330+
allow(Process).to receive(:clock_gettime).with(Process::CLOCK_MONOTONIC) do
331+
value = clock_calls.to_f
332+
clock_calls += 1
333+
value
334+
end
335+
336+
elapsed_times = []
337+
on_retry = ->(_exception, _try, elapsed_time, _next_interval) { elapsed_times << elapsed_time }
338+
339+
expect do
340+
described_class.retriable(tries: 3, on_retry: on_retry) { increment_tries_with_exception }
341+
end.to raise_error(StandardError)
342+
343+
# start_time (call 0) + at least one elapsed_time computation per retry
344+
expect(clock_calls).to be >= 3
345+
# elapsed_time values should be positive and non-decreasing
346+
expect(elapsed_times).to all(be > 0)
347+
expect(elapsed_times).to eq(elapsed_times.sort)
348+
end
349+
318350
it "raises ArgumentError on invalid options" do
319351
expect { described_class.retriable(does_not_exist: 123) { increment_tries } }.to raise_error(ArgumentError)
320352
end

0 commit comments

Comments
 (0)