Merge pull request #907 from Chessing234/fix/relay-reconnect-recovery

fix(nostr): keep retrying relays instead of giving up on them forever

app/src/main/java/com/bitchat/android/nostr/NostrRelayManager.kt

@@ -14,8 +14,6 @@ import okhttp3.*
 import java.util.UUID
 import java.util.concurrent.ConcurrentHashMap
 import java.util.concurrent.atomic.AtomicBoolean
-import kotlin.math.min
-import kotlin.math.pow
 
 /**
  * Manages WebSocket connections to Nostr relays
@@ -48,12 +46,8 @@ class NostrRelayManager private constructor() {
             "wss://nostr21.com"
         )
         
-        // Exponential backoff configuration (same as iOS)
-        private const val INITIAL_BACKOFF_INTERVAL = com.bitchat.android.util.AppConstants.Nostr.INITIAL_BACKOFF_INTERVAL_MS  // 1 second
-        private const val MAX_BACKOFF_INTERVAL = com.bitchat.android.util.AppConstants.Nostr.MAX_BACKOFF_INTERVAL_MS    // 5 minutes
-        private const val BACKOFF_MULTIPLIER = com.bitchat.android.util.AppConstants.Nostr.BACKOFF_MULTIPLIER
-        private const val MAX_RECONNECT_ATTEMPTS = com.bitchat.android.util.AppConstants.Nostr.MAX_RECONNECT_ATTEMPTS
-        
+        // Reconnect backoff lives in RelayReconnectPolicy.
+
         // Track gift-wraps we initiated for logging
         private val pendingGiftWrapIDs = ConcurrentHashMap.newKeySet<String>()
         
@@ -1018,36 +1012,15 @@ class NostrRelayManager private constructor() {
         if (!desiredConnected.get() ||
             !isNetworkActionAllowed(connectionToken)
         ) return
-        
-        // Check if this is a DNS error
-        val errorMessage = error.message?.lowercase() ?: ""
-        if (errorMessage.contains("hostname could not be found") || 
-            errorMessage.contains("dns") ||
-            errorMessage.contains("unable to resolve host")) {
-            
-            val relay = relaysList.find { it.url == relayUrl }
-            if (relay?.lastError == null) {
-                Log.w(TAG, "Nostr relay DNS failure; not retrying")
-            }
-            return
-        }
-        
-        // Implement exponential backoff for non-DNS errors
+
+        // Every failure backs off and retries, including a name-resolution
+        // failure: "unable to resolve host" is what this device reports when it
+        // simply has no network, so treating it as permanent turns a walk
+        // through a tunnel into a dead relay layer for the rest of the process.
         val relay = relaysList.find { it.url == relayUrl } ?: return
-        relay.reconnectAttempts++
-        
-        // Stop attempting after max attempts
-        if (relay.reconnectAttempts >= MAX_RECONNECT_ATTEMPTS) {
-            Log.w(TAG, "Max Nostr relay reconnection attempts reached")
-            return
-        }
-        
-        // Calculate backoff interval
-        val backoffInterval = min(
-            INITIAL_BACKOFF_INTERVAL * BACKOFF_MULTIPLIER.pow(relay.reconnectAttempts - 1.0),
-            MAX_BACKOFF_INTERVAL.toDouble()
-        ).toLong()
-        
+        relay.reconnectAttempts = RelayReconnectPolicy.nextAttempt(relay.reconnectAttempts)
+        val backoffInterval = RelayReconnectPolicy.backoffMs(relay.reconnectAttempts)
+
         relay.nextReconnectTime = System.currentTimeMillis() + backoffInterval
         
         Log.d(TAG, "Scheduling Nostr relay reconnection")

app/src/main/java/com/bitchat/android/nostr/RelayReconnectPolicy.kt

@@ -0,0 +1,45 @@
+package com.bitchat.android.nostr
+
+import com.bitchat.android.util.AppConstants
+import kotlin.math.min
+import kotlin.math.pow
+
+/**
+ * Reconnect schedule for a Nostr relay socket.
+ *
+ * A phone loses its data connection constantly — airplane mode, a tunnel, a
+ * Wi-Fi to cellular handover, a dead zone — and every relay fails at once when
+ * it does. The schedule therefore has to survive an outage of arbitrary length
+ * and heal on its own, because nothing else will: the relay layer registers no
+ * connectivity callback, the periodic subscription validator only repairs
+ * subscriptions on sockets that are already open, and `connect()` runs once at
+ * startup.
+ *
+ * So the backoff grows exponentially and then *saturates* rather than
+ * terminating. Retrying forever at the ceiling costs one connection attempt per
+ * relay per [AppConstants.Nostr.MAX_BACKOFF_INTERVAL_MS]; giving up costs the
+ * user every internet DM, delivery receipt and geohash channel until they
+ * notice and restart the app.
+ */
+internal object RelayReconnectPolicy {
+
+    /**
+     * Attempt count past which the interval stops growing. Beyond this the
+     * exponential term is already above the ceiling, so pinning it keeps the
+     * schedule at a steady [AppConstants.Nostr.MAX_BACKOFF_INTERVAL_MS] and
+     * keeps the exponent from running away over a long outage.
+     */
+    const val SATURATION_ATTEMPTS: Int = AppConstants.Nostr.MAX_RECONNECT_ATTEMPTS
+
+    /** Attempt number to record after a failure at [previousAttempts]. */
+    fun nextAttempt(previousAttempts: Int): Int =
+        (previousAttempts.coerceAtLeast(0) + 1).coerceAtMost(SATURATION_ATTEMPTS)
+
+    /** Delay before the reconnect for a given attempt number (1-based). */
+    fun backoffMs(attempt: Int): Long {
+        val step = attempt.coerceAtLeast(1)
+        val exponential = AppConstants.Nostr.INITIAL_BACKOFF_INTERVAL_MS *
+            AppConstants.Nostr.BACKOFF_MULTIPLIER.pow(step - 1.0)
+        return min(exponential, AppConstants.Nostr.MAX_BACKOFF_INTERVAL_MS.toDouble()).toLong()
+    }
+}

app/src/test/kotlin/com/bitchat/android/nostr/RelayReconnectPolicyTest.kt

@@ -0,0 +1,96 @@
+package com.bitchat.android.nostr
+
+import com.bitchat.android.util.AppConstants
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertTrue
+import org.junit.Test
+
+/**
+ * The relay layer has no connectivity callback, its periodic validator only
+ * repairs subscriptions on sockets that are already open, and `connect()` runs
+ * once at startup. The backoff schedule is therefore the only thing that can
+ * bring relays back after the phone loses its data connection, so it has to
+ * saturate rather than terminate.
+ */
+class RelayReconnectPolicyTest {
+
+    private val initial = AppConstants.Nostr.INITIAL_BACKOFF_INTERVAL_MS
+    private val ceiling = AppConstants.Nostr.MAX_BACKOFF_INTERVAL_MS
+
+    @Test
+    fun `the first retry waits the initial interval`() {
+        assertEquals(initial, RelayReconnectPolicy.backoffMs(RelayReconnectPolicy.nextAttempt(0)))
+    }
+
+    @Test
+    fun `the interval doubles per attempt until it reaches the ceiling`() {
+        var attempt = 0
+        var previous = 0L
+        var sawCeiling = false
+
+        repeat(RelayReconnectPolicy.SATURATION_ATTEMPTS) {
+            attempt = RelayReconnectPolicy.nextAttempt(attempt)
+            val delay = RelayReconnectPolicy.backoffMs(attempt)
+
+            assertTrue("delay must never exceed the ceiling", delay <= ceiling)
+            if (delay == ceiling) {
+                sawCeiling = true
+            } else {
+                assertEquals("expected doubling below the ceiling", maxOf(initial, previous * 2), delay)
+            }
+            previous = delay
+        }
+
+        assertTrue("the schedule must actually reach the ceiling", sawCeiling)
+    }
+
+    @Test
+    fun `an outage longer than the schedule keeps retrying at the ceiling`() {
+        var attempt = 0
+        // Far past the old give-up point; a real outage can last hours.
+        repeat(500) { attempt = RelayReconnectPolicy.nextAttempt(attempt) }
+
+        assertEquals(RelayReconnectPolicy.SATURATION_ATTEMPTS, attempt)
+        assertEquals(ceiling, RelayReconnectPolicy.backoffMs(attempt))
+    }
+
+    @Test
+    fun `a long outage cannot run the attempt counter or the exponent away`() {
+        var attempt = 0
+        repeat(10_000) { attempt = RelayReconnectPolicy.nextAttempt(attempt) }
+
+        val delay = RelayReconnectPolicy.backoffMs(attempt)
+        assertTrue("delay must stay finite and bounded", delay in 1..ceiling)
+    }
+
+    @Test
+    fun `a successful connection resets the schedule to the initial interval`() {
+        var attempt = 0
+        repeat(6) { attempt = RelayReconnectPolicy.nextAttempt(attempt) }
+        assertTrue(RelayReconnectPolicy.backoffMs(attempt) > initial)
+
+        // updateRelayStatus zeroes reconnectAttempts on a successful open.
+        attempt = 0
+
+        assertEquals(initial, RelayReconnectPolicy.backoffMs(RelayReconnectPolicy.nextAttempt(attempt)))
+    }
+
+    @Test
+    fun `a nonsensical stored attempt count still yields a usable delay`() {
+        assertEquals(initial, RelayReconnectPolicy.backoffMs(RelayReconnectPolicy.nextAttempt(-5)))
+        assertTrue(RelayReconnectPolicy.backoffMs(0) in 1..ceiling)
+        assertTrue(RelayReconnectPolicy.backoffMs(Int.MAX_VALUE) in 1..ceiling)
+    }
+
+    @Test
+    fun `the whole schedule stays under an hour of total wait before the ceiling`() {
+        var attempt = 0
+        var total = 0L
+        repeat(RelayReconnectPolicy.SATURATION_ATTEMPTS) {
+            attempt = RelayReconnectPolicy.nextAttempt(attempt)
+            total += RelayReconnectPolicy.backoffMs(attempt)
+        }
+
+        assertTrue("reaching the steady state must not take an hour", total < 60 * 60 * 1000L)
+    }
+}