From e85d6c5fbc5b5df528c857ba5e23fab7d044296d Mon Sep 17 00:00:00 2001
From: Greg Price <greg@zulip.com>
Date: Wed, 5 Jun 2024 15:09:40 -0700
Subject: [PATCH] backoff: Always wait a positive duration

Fixes: #602
---
 lib/api/backoff.dart       |  9 ++++++++-
 test/api/backoff_test.dart | 21 +++++++++++++++++++++
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/lib/api/backoff.dart b/lib/api/backoff.dart
index 404b63ee60..640509d24d 100644
--- a/lib/api/backoff.dart
+++ b/lib/api/backoff.dart
@@ -37,6 +37,12 @@ class BackoffMachine {
   /// maximizes the range while preserving a capped exponential shape on
   /// the expected value.  Greg discusses this in more detail at:
   ///   https://github.com/zulip/zulip-mobile/pull/3841
+  ///
+  /// The duration is always positive; [Duration] works in microseconds, so
+  /// we deviate from the idealized uniform distribution just by rounding
+  /// the smallest durations up to one microsecond instead of down to zero.
+  /// Because in the real world any delay takes nonzero time, this mainly
+  /// affects tests that use fake time, and keeps their behavior more realistic.
   Future<void> wait() async {
     _startTime ??= DateTime.now();
 
@@ -45,7 +51,8 @@ class BackoffMachine {
       * min(_durationCeilingMs,
             _firstDurationMs * pow(_base, _waitsCompleted));
 
-    await Future<void>.delayed(Duration(milliseconds: durationMs.round()));
+    await Future<void>.delayed(Duration(
+      microseconds: max(1, (1000 * durationMs).round())));
 
     _waitsCompleted++;
   }
diff --git a/test/api/backoff_test.dart b/test/api/backoff_test.dart
index 132dcf0eee..0b66d5028f 100644
--- a/test/api/backoff_test.dart
+++ b/test/api/backoff_test.dart
@@ -1,4 +1,5 @@
 import 'dart:async';
+import 'dart:math';
 
 import 'package:checks/checks.dart';
 import 'package:clock/clock.dart';
@@ -51,4 +52,24 @@ void main() {
       check(maxFromAllTrials).isGreaterThan(expectedMax * 0.75);
     }
   });
+
+  test('BackoffMachine timeouts are always positive', () {
+    // Regression test for: https://github.com/zulip/zulip-flutter/issues/602
+    // This is a randomized test with a false-failure rate of zero.
+
+    // In the pre-#602 implementation, the first timeout was zero
+    // when a random number from [0, 100] was below 0.5.
+    // [numTrials] is chosen so that an implementation with that behavior
+    // will fail the test with probability 99%.
+    const hypotheticalFailureRate = 0.5 / 100;
+    const numTrials = 2 * ln10 / hypotheticalFailureRate;
+
+    awaitFakeAsync((async) async {
+      for (int i = 0; i < numTrials; i++) {
+        final duration = await measureWait(BackoffMachine().wait());
+        check(duration).isGreaterThan(Duration.zero);
+      }
+      check(async.pendingTimers).isEmpty();
+    });
+  });
 }