fix(transport): clamp FibonacciBackoff state at max to avoid u64 overflow (#770)

* fix(transport): clamp FibonacciBackoff state at max to avoid u64 overflow

`next_delay()` capped only the returned delay; the internal state kept
growing as a raw Fibonacci sequence, which overflows u64 at call 93
(fib(94)). A reconnect loop with a large `max_reconnect_attempts` or
`reconnect_forever` reaches that during a long outage. With the
30 s `max` both call sites use, about 45 minutes of continuous outage.
In debug builds `next_delay()` would panic with `attempt to add with
overflow`, and in release the addition would wrap and the delays would
become nonsense.

* fix(transport): saturate backoff sum and honor max=0

- saturating_add covers max values above fib(93), where the raw sum
  overflows before the clamp engages
- clamp current at construction so max=0 keeps meaning no delay instead
  of a fixed 1s

---------

Co-authored-by: Wil Boayue <wil.boayue@gmail.com>

Author: tradatious

CHANGELOG.md

@@ -46,6 +46,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- `FibonacciBackoff` no longer overflows `u64` on the 93rd consecutive `next_delay()` call. The backoff state kept growing past `max` (only the returned delay was capped), so a reconnect loop with a large `max_reconnect_attempts` or `reconnect_forever` would panic with `attempt to add with overflow` in debug builds, or wrap to nonsense delays in release, once an outage outlasted ~92 attempts. The state is now clamped at `max`. Returned delays are unchanged.
+
 - Notice 2188 ("Up-to-the-second historical data requires additional subscription for the API.") is classified as a data advisory instead of a hard error. TWS sends it and then delivers the historical bars anyway — the account merely lacks the up-to-the-second tail — but `historical_data()` returned `Err` on the notice and discarded the bars that followed, so accounts without the real-time entitlement for a listing got no history at all for those symbols. `DATA_ADVISORY_CODES` widens from `[i32; 2]` to `[i32; 3]`, which is breaking only for code binding the const with an explicit array type (#765).
 
 - Error 10090 ("Part of requested market data is not subscribed. Subscription-independent ticks are still active") is classified as a data advisory like 10089 and 10167: it is published as a non-terminal notice instead of ending the market-data subscription. TWS sends it on partial entitlement — commonly an options subscription without the underlying — and keeps delivering the ticks the account is entitled to, but the subscription was torn down before they could arrive. `DATA_ADVISORY_CODES` widens again to `[i32; 4]` (#768).

src/transport/common.rs

@@ -133,21 +133,23 @@ impl FibonacciBackoff {
     pub(crate) fn new(max: u64) -> Self {
         FibonacciBackoff {
             previous: 0,
-            current: 1,
+            // Clamped so `current <= max` holds from construction on; keeps
+            // `max: 0` meaning "no delay" rather than a fixed 1s.
+            current: 1.min(max),
             max,
         }
     }
 
     pub(crate) fn next_delay(&mut self) -> Duration {
-        let next = self.previous + self.current;
-        self.previous = self.current;
-        self.current = next;
-
-        if next > self.max {
-            Duration::from_secs(self.max)
-        } else {
-            Duration::from_secs(next)
+        // Note: `max` must clamp `previous` and `current` (not just the return value)
+        // because u64 overflows at fib(94). The saturating_add covers `max` values
+        // large enough that the sum overflows before the clamp can engage.
+        if self.current < self.max {
+            let next = self.previous.saturating_add(self.current).min(self.max);
+            self.previous = self.current;
+            self.current = next;
         }
+        Duration::from_secs(self.current)
     }
 }
 

src/transport/common_tests.rs

@@ -137,3 +137,38 @@ fn test_fibonacci_backoff() {
     assert_eq!(backoff.next_delay(), Duration::from_secs(10)); // capped at max
     assert_eq!(backoff.next_delay(), Duration::from_secs(10)); // stays at max
 }
+
+/// The raw Fibonacci sequence overflows u64 after ~93 steps, which can be
+/// reached during a long outage with a large `max_reconnect_attempts`. The
+/// internal steps must be clamped at `max` to avoid u64 overflow regardless of
+/// the number of calls. Without that, this test will fail in debug builds with
+/// `attempt to add with overflow` at call 93.
+#[test]
+fn test_fibonacci_backoff_never_overflows() {
+    let mut backoff = FibonacciBackoff::new(30);
+    for _ in 0..100 {
+        assert!(backoff.next_delay() <= Duration::from_secs(30));
+    }
+    assert_eq!(backoff.next_delay(), Duration::from_secs(30));
+}
+
+/// A `max` above fib(93) lets the raw sum overflow before the clamp can
+/// engage; `saturating_add` must cover that. Without it, call ~93 panics with
+/// `attempt to add with overflow` in debug builds.
+#[test]
+fn test_fibonacci_backoff_never_overflows_with_huge_max() {
+    let mut backoff = FibonacciBackoff::new(u64::MAX);
+    for _ in 0..100 {
+        backoff.next_delay();
+    }
+    assert_eq!(backoff.next_delay(), Duration::from_secs(u64::MAX));
+}
+
+/// `max: 0` means no delay, not a fixed 1s: `current` starts clamped at
+/// `max`, so the delay must respect `max` from the first call.
+#[test]
+fn test_fibonacci_backoff_zero_max() {
+    let mut backoff = FibonacciBackoff::new(0);
+    assert_eq!(backoff.next_delay(), Duration::ZERO);
+    assert_eq!(backoff.next_delay(), Duration::ZERO);
+}