refactor: Report the next delay through the property, not a return value

Author: jsonbailey

ldclient/impl/datasource/async_polling.py

@@ -104,7 +104,8 @@ async def _fetch_and_store(self) -> None:
             # logged, the handler has exited and exc_info() is empty.
             stacktrace = e
 
-        delay = self._retry.record_failure(kind)
+        self._retry.record_failure(kind)
+        delay = self._retry.next_delay
         level("%s - will retry in %.1fs" % (description, delay), exc_info=stacktrace)
 
         if self._data_source_update_sink is not None:

ldclient/impl/datasource/async_streaming.py

@@ -266,7 +266,8 @@ async def _handle_error(self, error: Exception) -> bool:
             description = "Error on stream connection: %s" % error
             level = log.warning
 
-        delay = self._retry.record_failure(kind)
+        self._retry.record_failure(kind)
+        delay = self._retry.next_delay
         level("%s - will retry in %.1fs" % (description, delay))
 
         if self._data_source_update_sink is not None:

ldclient/impl/datasource/polling.py

@@ -109,7 +109,8 @@ def _poll(self) -> None:
             # logged, the handler has exited and exc_info() is empty.
             stacktrace = e
 
-        delay = self._retry.record_failure(kind)
+        self._retry.record_failure(kind)
+        delay = self._retry.next_delay
         level("%s - will retry in %.1fs" % (description, delay), exc_info=stacktrace)
 
         if self._data_source_update_sink is not None:

ldclient/impl/datasource/streaming.py

@@ -245,7 +245,8 @@ def _handle_error(self, error: Exception) -> bool:
             description = "Error on stream connection: %s" % error
             level = log.warning
 
-        delay = self._retry.record_failure(kind)
+        self._retry.record_failure(kind)
+        delay = self._retry.next_delay
         level("%s - will retry in %.1fs" % (description, delay))
 
         if self._data_source_update_sink is not None:

ldclient/impl/retry.py

@@ -152,9 +152,10 @@ class RetryState:
     """
     Tracks how long a data source should wait before its next attempt.
 
-    A failure moves the state on and returns the wait. The delay for attempt
-    ``n`` is ``min(min_delay * 2 ** (n - 1), max_delay)``, less a random
-    jitter of up to half of it, and never less than the operating cadence.
+    A failure moves the state on and decides the next wait, which
+    :attr:`next_delay` reports. The delay for attempt ``n`` is
+    ``min(min_delay * 2 ** (n - 1), max_delay)``, less a random jitter of up to
+    half of it, and never less than the operating cadence.
 
     An unexpected failure moves the state to the extended regime, which raises
     both delay bounds. The bounds stay raised until the reset condition is met,
@@ -228,13 +229,12 @@ def in_extended_regime(self) -> bool:
         delay bounds."""
         return self._extended
 
-    def record_failure(self, kind: FailureKind) -> float:
+    def record_failure(self, kind: FailureKind) -> None:
         """
-        Records a failed attempt and returns how long to wait before the next
-        one, in seconds.
+        Records a failed attempt, and decides the wait before the next one.
 
-        The state moves on before the wait is computed, so the wait always
-        reflects the failure just recorded.
+        The state moves on before the wait is computed, so :attr:`next_delay`
+        always reflects the failure just recorded.
 
         :param kind: how the failure was classified
         """
@@ -256,7 +256,6 @@ def record_failure(self, kind: FailureKind) -> float:
             self._n += 1
 
         self._next_delay = self._compute_wait()
-        return self._next_delay
 
     def record_success(self) -> None:
         """

ldclient/testing/impl/test_retry.py

@@ -82,6 +82,13 @@ def real_jitter():
         yield
 
 
+def failure_delay(state, kind=NORMAL) -> float:
+    """Records a failure and reads back the wait it decided, which is the value
+    a data source reads."""
+    state.record_failure(kind)
+    return state.next_delay
+
+
 def streaming_state(initial_delay=1):
     return for_streaming(initial_delay)
 
@@ -119,7 +126,7 @@ def test_a_non_positive_delay_falls_back_to_the_default(self, configured, caplog
         state = for_streaming(configured)
 
         assert state.min_delay == DEFAULT_INITIAL_RECONNECT_DELAY
-        assert state.record_failure(NORMAL) == DEFAULT_INITIAL_RECONNECT_DELAY
+        assert failure_delay(state, NORMAL) == DEFAULT_INITIAL_RECONNECT_DELAY
         assert caplog.records[0].getMessage() == (
             "initial_reconnect_delay must be greater than zero; using the default of 1s"
         )
@@ -131,7 +138,7 @@ def test_a_positive_delay_is_left_alone(self, configured, caplog):
         state = for_streaming(configured)
 
         assert state.min_delay == configured
-        assert state.record_failure(NORMAL) == configured
+        assert failure_delay(state, NORMAL) == configured
         assert caplog.records == []
 
 
@@ -144,12 +151,12 @@ class TestStreamingExtendedDelayFloor:
         [(1, 300), (30, 300), (300, 300), (600, 600), (3600, 3600), (0, 300), (-5, 300)],
     )
     def test_the_extended_delay_never_starts_below_the_configured_delay(self, configured, expected):
-        assert for_streaming(configured).record_failure(UNEXPECTED) == expected
+        assert failure_delay(for_streaming(configured), UNEXPECTED) == expected
 
     @pytest.mark.parametrize("configured", [1, 30, 300, 600, 3600])
     def test_an_unexpected_failure_never_waits_less_than_a_normal_one(self, configured):
-        normal = for_streaming(configured).record_failure(NORMAL)
-        unexpected = for_streaming(configured).record_failure(UNEXPECTED)
+        normal = failure_delay(for_streaming(configured), NORMAL)
+        unexpected = failure_delay(for_streaming(configured), UNEXPECTED)
         assert unexpected >= normal
 
     @pytest.mark.parametrize(
@@ -162,28 +169,28 @@ def test_an_unexpected_failure_never_waits_less_than_a_normal_one(self, configur
     )
     def test_the_extended_ladder_still_doubles_to_the_ceiling(self, configured, ladder):
         state = for_streaming(configured)
-        delays = [state.record_failure(UNEXPECTED)]
-        delays += [state.record_failure(NORMAL) for _ in range(4)]
+        delays = [failure_delay(state, UNEXPECTED)]
+        delays += [failure_delay(state, NORMAL) for _ in range(4)]
         assert delays == ladder
 
 
 class TestStreamingDelayTable:
     def test_normal_regime_doubles_up_to_the_ceiling(self):
         state = streaming_state(initial_delay=1)
-        delays = [state.record_failure(NORMAL) for _ in range(8)]
+        delays = [failure_delay(state, NORMAL) for _ in range(8)]
         assert delays == [1, 2, 4, 8, 16, 30, 30, 30]
 
     def test_extended_regime_doubles_up_to_the_ceiling(self):
         state = streaming_state(initial_delay=1)
-        delays = [state.record_failure(UNEXPECTED)]
-        delays += [state.record_failure(NORMAL) for _ in range(5)]
+        delays = [failure_delay(state, UNEXPECTED)]
+        delays += [failure_delay(state, NORMAL) for _ in range(5)]
         assert delays == [5 * 60, 10 * 60, 20 * 60, 40 * 60, 60 * 60, 60 * 60]
 
     def test_a_configured_initial_delay_raises_the_ceiling_with_it(self):
         # RETRY 1.5.4 as amended: maxDelay must not fall below initialDelay.
         state = streaming_state(initial_delay=45)
         assert state.max_delay == 45
-        assert state.record_failure(NORMAL) == 45
+        assert failure_delay(state, NORMAL) == 45
 
     def test_the_ceiling_is_sticky_once_the_extended_regime_starts(self):
         # RETRY 1.5.5: a normal failure after an unexpected one must not lower
@@ -202,9 +209,9 @@ def test_a_second_unexpected_failure_keeps_counting_up(self):
         # Restarting the count on every unexpected failure would pin the delay
         # at the extended initial delay for ever.
         state = streaming_state(initial_delay=1)
-        assert state.record_failure(UNEXPECTED) == 5 * 60
-        assert state.record_failure(UNEXPECTED) == 10 * 60
-        assert state.record_failure(UNEXPECTED) == 20 * 60
+        assert failure_delay(state, UNEXPECTED) == 5 * 60
+        assert failure_delay(state, UNEXPECTED) == 10 * 60
+        assert failure_delay(state, UNEXPECTED) == 20 * 60
 
     def test_the_streaming_defaults_match_the_spec(self):
         state = streaming_state(initial_delay=1)
@@ -217,20 +224,20 @@ class TestJitter:
     def test_jitter_never_removes_more_than_half_the_delay(self):
         with fixed_retry_jitter(FULL_JITTER):
             state = streaming_state(initial_delay=8)
-            delay = state.record_failure(NORMAL)
+            delay = failure_delay(state, NORMAL)
             assert 4 <= delay < 8
 
     def test_no_jitter_leaves_the_delay_alone(self):
         state = streaming_state(initial_delay=8)
-        assert state.record_failure(NORMAL) == 8
+        assert failure_delay(state, NORMAL) == 8
 
     def test_every_delay_stays_within_the_jitter_bounds(self):
         # The real random source, so the bound has to hold for any draw rather
         # than for one seeded sequence.
         with real_jitter():
             state = streaming_state(initial_delay=1)
             for base in [1, 2, 4, 8, 16, 30, 30, 30]:
-                delay = state.record_failure(NORMAL)
+                delay = failure_delay(state, NORMAL)
                 assert base / 2 <= delay <= base
 
 
@@ -249,7 +256,7 @@ def test_a_minute_of_healthy_operation_resets_the_state(self):
             state.record_success()
             assert not state.in_extended_regime
             assert state.max_delay == STREAMING_MAX_DELAY
-            assert state.record_failure(NORMAL) == 1
+            assert failure_delay(state, NORMAL) == 1
 
     def test_a_reset_also_happens_on_the_failure_that_ends_a_healthy_stretch(self):
         with frozen_clock() as clock:
@@ -262,7 +269,7 @@ def test_a_reset_also_happens_on_the_failure_that_ends_a_healthy_stretch(self):
 
             # The state resets before this failure is counted, so the delay is
             # the first-retry delay again rather than the fourth.
-            assert state.record_failure(NORMAL) == 1
+            assert failure_delay(state, NORMAL) == 1
 
     def test_a_short_healthy_stretch_does_not_reset(self):
         with frozen_clock() as clock:
@@ -271,7 +278,7 @@ def test_a_short_healthy_stretch_does_not_reset(self):
 
             state.record_success()
             clock.advance(STREAMING_RESET_INTERVAL - 1)
-            assert state.record_failure(NORMAL) == 2
+            assert failure_delay(state, NORMAL) == 2
 
     def test_a_fast_flapping_connection_does_not_ratchet_into_the_extended_regime(self):
         # Every transport failure is normal, so no amount of flapping reaches
@@ -283,7 +290,7 @@ def test_a_fast_flapping_connection_does_not_ratchet_into_the_extended_regime(se
             for _ in range(20):
                 state.record_success()
                 clock.advance(5)
-                delays.append(state.record_failure(NORMAL))
+                delays.append(failure_delay(state, NORMAL))
                 clock.advance(1)
 
             assert not state.in_extended_regime
@@ -294,26 +301,26 @@ def test_a_fast_flapping_connection_does_not_ratchet_into_the_extended_regime(se
 class TestPollingCadence:
     def test_a_normal_failure_polls_again_on_schedule(self):
         state = polling_state(poll_interval=30)
-        assert [state.record_failure(NORMAL) for _ in range(4)] == [30, 30, 30, 30]
+        assert [failure_delay(state, NORMAL) for _ in range(4)] == [30, 30, 30, 30]
 
     def test_the_extended_regime_doubles_up_to_an_hour(self):
         state = polling_state(poll_interval=30)
-        delays = [state.record_failure(UNEXPECTED)]
-        delays += [state.record_failure(NORMAL) for _ in range(5)]
+        delays = [failure_delay(state, UNEXPECTED)]
+        delays += [failure_delay(state, NORMAL) for _ in range(5)]
         assert delays == [5 * 60, 10 * 60, 20 * 60, 40 * 60, 60 * 60, 60 * 60]
 
     def test_the_wait_never_falls_below_the_poll_interval(self):
         # RETRY 1.4.9. Full jitter would otherwise halve the delay.
         with fixed_retry_jitter(FULL_JITTER):
             state = polling_state(poll_interval=30)
-            assert state.record_failure(NORMAL) == 30
-            assert state.record_failure(UNEXPECTED) >= 30
+            assert failure_delay(state, NORMAL) == 30
+            assert failure_delay(state, UNEXPECTED) >= 30
 
     def test_a_poll_interval_longer_than_the_extended_bounds_wins(self):
         # The ceiling is lifted by record_failure clamping it against the
         # initial delay, not by for_polling clamping the ceiling itself.
         state = polling_state(poll_interval=2 * 60 * 60)
-        assert state.record_failure(UNEXPECTED) == 2 * 60 * 60
+        assert failure_delay(state, UNEXPECTED) == 2 * 60 * 60
         assert state.max_delay == 2 * 60 * 60
         assert state.min_delay == 2 * 60 * 60
 
@@ -324,7 +331,7 @@ def test_one_success_restores_the_cadence_while_the_state_is_still_raised(self):
         state = polling_state(poll_interval=30)
         state.record_failure(UNEXPECTED)
         state.record_failure(NORMAL)
-        assert state.record_failure(NORMAL) == 20 * 60
+        assert failure_delay(state, NORMAL) == 20 * 60
 
         state.record_success()
         assert state.next_delay == 30
@@ -341,7 +348,7 @@ def test_two_successes_in_a_row_reset_the_state(self):
         state.record_success()
         assert not state.in_extended_regime
         assert state.next_delay == 30
-        assert state.record_failure(NORMAL) == 30
+        assert failure_delay(state, NORMAL) == 30
 
     def test_a_failure_between_two_successes_clears_the_first(self):
         state = polling_state(poll_interval=30)
@@ -384,7 +391,7 @@ def test_a_reset_starts_the_attempt_count_over(self):
 
             # The delay drops back to the first-retry value, and the count
             # starts over with it.
-            assert state.record_failure(NORMAL) == 1
+            assert failure_delay(state, NORMAL) == 1
             assert state.attempts == 1
 
 
@@ -460,5 +467,5 @@ def test_a_long_outage_cannot_overflow_the_delay(self):
             reset_policy=AfterHealthyFor(60),
         )
         for _ in range(5000):
-            delay = state.record_failure(NORMAL)
+            delay = failure_delay(state, NORMAL)
         assert delay == 30