Fix relays staying disconnected and reset them on VPN changes

Relays could stay down until the app was force closed:

- RelayHealthTracker marked a relay dead permanently, and every failed
  dial was counted twice (onCannotConnect + onDisconnected), so ~5 failed
  attempts during a bad-connectivity stretch dropped every relay from the
  subscriptions. Only an OS network change or a manual reconnect cleared
  it. Dead relays now get another chance after a 15 minute cooldown, and
  only failed dials count.
- After onLost, the next network never cleared the dead list (lastNetwork
  was null), and resets that did happen never re-added dead relays to the
  subscriptions until the 5 minute safety tick.
- Network changes reconnected with Quartz's retry backoff still in place
  (up to 5 minutes, set at once on an unresolved host while offline), and
  kept sockets from the old network that only fail on ping timeout.

Network changes are now detected by NetworkChangeDetector (default
network, transports including VPN, regaining internet access) and handled
by Amber.resetRelayConnections, which drops all sockets, clears the dead
list and backoff, redials and refreshes the subscriptions. A VPN coming up
or going down, or moving between Wi-Fi and mobile, now resets relays the
same way switching Wi-Fi <-> mobile does. Manual reconnect uses the same
path.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Sr7iw4HfVgyBCrdsCxSVAX
This commit is contained in:
Claude
2026-10-03 17:32:42 +00:00
parent fc2e0de5dd
commit e93202958d
8 changed files with 389 additions and 62 deletions
@@ -47,6 +47,7 @@ import com.greenart7c3.nostrsigner.okhttp.HttpClientManager
import com.greenart7c3.nostrsigner.okhttp.OkHttpWebSocket
import com.greenart7c3.nostrsigner.relays.AmberRelayStats
import com.greenart7c3.nostrsigner.relays.NostrClientLoggerListener
import com.greenart7c3.nostrsigner.relays.RelayHealthTracker
import com.greenart7c3.nostrsigner.service.ApplicationNameCache
import com.greenart7c3.nostrsigner.service.BackupApplicationsWorker
import com.greenart7c3.nostrsigner.service.ClearLogsWorker
@@ -206,6 +207,7 @@ class Amber :
val isOnMobileDataState = mutableStateOf(false)
val isOnWifiDataState = mutableStateOf(false)
val isOnVpnState = mutableStateOf(false)
val isOnOfflineState = mutableStateOf(false)
/** npubs whose AndroidKeyStore key failed to decrypt (device KeyMint bug). */
@@ -258,7 +260,8 @@ class Amber :
fun updateNetworkCapabilities(networkCapabilities: NetworkCapabilities?): Boolean {
val isOnMobileData = networkCapabilities?.hasTransport(NetworkCapabilities.TRANSPORT_CELLULAR) == true
val isOnWifi = networkCapabilities?.hasTransport(NetworkCapabilities.TRANSPORT_WIFI) == true
val isOffline = !isOnMobileData && !isOnWifi
val isOnVpn = networkCapabilities?.hasTransport(NetworkCapabilities.TRANSPORT_VPN) == true
val isOffline = networkCapabilities?.hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) != true
var changedNetwork = false
@@ -274,6 +277,11 @@ class Amber :
changedNetwork = true
}
if (isOnVpnState.value != isOnVpn) {
isOnVpnState.value = isOnVpn
changedNetwork = true
}
if (isOnOfflineState.value != isOffline) {
isOnOfflineState.value = isOffline
changedNetwork = true
@@ -571,6 +579,39 @@ class Amber :
}
}
/**
* Drops every relay socket and dials again from scratch. Called when the device's
* network identity changes (Wi-Fi <-> mobile, a VPN coming up or going down, the
* network regaining internet access).
*
* Sockets opened on the previous network are bound to routes that may no longer
* exist: they keep reporting "connected" and only fail once OkHttp's ping times out,
* minutes later. Relays that failed while the device was offline also carry state
* that would keep them down on the new network: Quartz's per-relay backoff (up to
* five minutes, set at once on an unresolved host) and RelayHealthTracker's dead
* list, which removes them from the subscriptions entirely. All of it is about the
* old network, so it is cleared here.
*/
suspend fun resetRelayConnections(reason: String) {
if (BuildFlavorChecker.isOfflineFlavor()) return
if (settings.killSwitch.value) return
AmberLog.d(TAG, "Resetting relay connections: $reason")
RelayHealthTracker.reset()
// The teardown reports onDisconnected for every relay; mark it intentional so
// NostrClientLoggerListener does not count it as failures or schedule retries.
intentionalDisconnectTime = System.currentTimeMillis()
client.disconnect()
// disconnect() also clears each relay's backoff; this covers relays without a socket.
client.resetBackoff()
client.connect()
// Re-adds relays the dead list had dropped from the subscriptions.
checkForNewRelaysAndUpdateAllFilters()
stats.updateNotification()
}
// computeIfAbsent (not check-then-put): AppDatabase.getDatabase builds a
// fresh RoomDatabase every call, so a racing second caller would build a
// duplicate that loses the put() and leaks its SQLiteConnection (caught by
@@ -60,8 +60,13 @@ class NostrClientLoggerListener(
// is still worth retrying. Once a relay is dead, RelayHealthTracker also makes
// NotificationSubscription.updateFilter drop it from the subscription relay
// set, so Quartz stops opening sockets to it on every refresh. The streak resets on a
// successful connection (onConnected) or a network change / manual reconnect.
private fun scheduleReconnect(relay: NormalizedRelayUrl) {
// successful connection (onConnected), a network change / manual reconnect, or
// after RelayHealthTracker's cooldown.
//
// Only failed dials count (onCannotConnect). Quartz reports every failed dial as
// onCannotConnect followed by onDisconnected, so counting both halved the real
// threshold, and a plain server-side close is not a connection failure.
private fun recordFailureAndScheduleReconnect(relay: NormalizedRelayUrl) {
if (!RelayHealthTracker.recordFailure(relay)) {
AmberLog.d(Amber.TAG, "Relay ${relay.url} marked dead; skipping reconnect")
return
@@ -69,6 +74,14 @@ class NostrClientLoggerListener(
reconnectWithBackoff()
}
private fun scheduleReconnect(relay: NormalizedRelayUrl) {
if (RelayHealthTracker.isDead(relay)) {
AmberLog.d(Amber.TAG, "Relay ${relay.url} is dead; skipping reconnect")
return
}
reconnectWithBackoff()
}
private fun reconnectWithBackoff() {
val now = System.currentTimeMillis()
if (now - lastDisconnectTime > 60_000) {
@@ -129,7 +142,7 @@ class NostrClientLoggerListener(
return
}
scheduleReconnect(relay.url)
recordFailureAndScheduleReconnect(relay.url)
super.onCannotConnect(relay, errorMessage)
}
@@ -5,7 +5,7 @@ import java.util.concurrent.ConcurrentHashMap
/**
* Tracks consecutive connection failures per relay so the app can stop retrying
* relays that are permanently unreachable.
* relays that are unreachable.
*
* Quartz's relay pool is driven by the relays referenced in active subscriptions:
* [NotificationSubscription.updateFilter] re-subscribes on every refresh, and any relay
@@ -18,28 +18,47 @@ import java.util.concurrent.ConcurrentHashMap
* them entirely. The streak is cleared when a relay connects successfully
* ([recordSuccess]), and [reset] is called on every OS network change and on a
* manual reconnect so previously-dead relays get a fresh chance.
*
* Being dead is not permanent: [DEAD_COOLDOWN_MS] after its last failure a relay is
* eligible again, so the periodic subscription refresh re-adds it. Without the
* cooldown a stretch of bad connectivity that never surfaced as an OS network change
* (a Wi-Fi that lost internet and got it back, a weak cell signal, a relay outage)
* marked every relay dead and the app stayed disconnected until it was force closed.
*/
object RelayHealthTracker {
private const val MAX_RECONNECT_ATTEMPTS = 10
const val DEAD_COOLDOWN_MS = 15 * 60 * 1000L
private val failureCounts = ConcurrentHashMap<NormalizedRelayUrl, Int>()
private class Health(val failures: Int, val lastFailureAt: Long)
private val health = ConcurrentHashMap<NormalizedRelayUrl, Health>()
/** Injectable for tests. */
internal var clock: () -> Long = System::currentTimeMillis
/** Records a failed connection attempt. Returns true while the relay is still worth retrying. */
fun recordFailure(relay: NormalizedRelayUrl): Boolean {
val failures = failureCounts.merge(relay, 1, Int::plus) ?: 1
return failures <= MAX_RECONNECT_ATTEMPTS
val now = clock()
val updated = health.merge(relay, Health(1, now)) { old, _ -> Health(old.failures + 1, now) }
return updated == null || updated.failures <= MAX_RECONNECT_ATTEMPTS
}
/** Clears the failure streak after a successful connection. */
fun recordSuccess(relay: NormalizedRelayUrl) {
failureCounts.remove(relay)
health.remove(relay)
}
/** True once a relay has failed [MAX_RECONNECT_ATTEMPTS] times in a row without recovering. */
fun isDead(relay: NormalizedRelayUrl): Boolean = (failureCounts[relay] ?: 0) > MAX_RECONNECT_ATTEMPTS
/**
* True once a relay has failed more than [MAX_RECONNECT_ATTEMPTS] times in a row without
* recovering, until [DEAD_COOLDOWN_MS] has passed since its last failure.
*/
fun isDead(relay: NormalizedRelayUrl): Boolean {
val entry = health[relay] ?: return false
return entry.failures > MAX_RECONNECT_ATTEMPTS && clock() - entry.lastFailureAt < DEAD_COOLDOWN_MS
}
/** Forgets all failure history so every relay is eligible to be retried again. */
fun reset() {
failureCounts.clear()
health.clear()
}
}
@@ -16,85 +16,55 @@ import com.greenart7c3.nostrsigner.BuildFlavorChecker
import com.greenart7c3.nostrsigner.LocalPreferences
import com.greenart7c3.nostrsigner.models.TorMode
import com.greenart7c3.nostrsigner.okhttp.HttpClientManager
import com.greenart7c3.nostrsigner.relays.RelayHealthTracker
import java.util.Timer
import java.util.TimerTask
import kotlin.collections.set
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.Job
import kotlinx.coroutines.SupervisorJob
import kotlinx.coroutines.delay
import kotlinx.coroutines.launch
class ConnectivityService : Service() {
private val timer = Timer()
private val scope = CoroutineScope(Dispatchers.IO + SupervisorJob())
private val networkChangeDetector = NetworkChangeDetector()
private var pendingRelayReset: Job? = null
private val networkCallback =
object : ConnectivityManager.NetworkCallback() {
var lastNetwork: Network? = null
override fun onAvailable(network: Network) {
super.onAvailable(network)
if (BuildFlavorChecker.isOfflineFlavor()) return
if (Amber.instance.settings.killSwitch.value) return
if (lastNetwork != null && lastNetwork != network) {
// New network: give previously-dead relays a fresh chance.
// RelayHealthTracker.reset() makes updateFilter re-add them
// to the subscription set (explicit refreshes plus the
// periodic safety net below).
if (Amber.instance.settings.torMode == TorMode.BUILTIN && !TorManager.isRunning.value) {
// Built-in Tor gave up earlier (bounded startup retries
// in runMigrations). The network is back, so retry now
// instead of waiting for a manual restart.
TorManager.restart(this@ConnectivityService, Amber.instance.applicationIOScope)
}
RelayHealthTracker.reset()
scope.launch(Dispatchers.IO) {
if (!Amber.instance.client.isActive()) {
Amber.instance.client.connect()
}
Amber.instance.client.reconnect(true)
}
// onCapabilitiesChanged follows right away, but read them here too in
// case it does not, so a new default network is never missed.
val connectivityManager =
(getSystemService(ConnectivityManager::class.java) as ConnectivityManager)
connectivityManager.getNetworkCapabilities(network)?.let {
onNetworkState(network, it)
}
lastNetwork = network
}
// Network capabilities have changed for the network
override fun onCapabilitiesChanged(
network: Network,
networkCapabilities: NetworkCapabilities,
) {
super.onCapabilitiesChanged(network, networkCapabilities)
if (BuildFlavorChecker.isOfflineFlavor()) return
if (Amber.instance.settings.killSwitch.value) return
val changed = Amber.instance.updateNetworkCapabilities(networkCapabilities)
if (changed) {
// Transport changed (e.g. wifi <-> mobile): retry dead relays.
RelayHealthTracker.reset()
}
scope.launch(Dispatchers.IO) {
AmberLog.d(
"ServiceManager NetworkCallback",
"onCapabilitiesChanged: ${network.networkHandle} hasMobileData ${Amber.instance.isOnMobileDataState.value} hasWifi ${Amber.instance.isOnWifiDataState.value}",
)
if (!Amber.instance.client.isActive()) {
Amber.instance.client.connect()
}
if (changed) {
Amber.instance.client.reconnect(true)
}
}
onNetworkState(network, networkCapabilities)
}
override fun onLost(network: Network) {
super.onLost(network)
if (BuildFlavorChecker.isOfflineFlavor()) return
lastNetwork = null
AmberLog.d("ServiceManager NetworkCallback", "onLost: ${network.networkHandle}")
if (!networkChangeDetector.onLost(network.networkHandle)) return
cancelPendingRelayReset()
val connectivityManager =
(getSystemService(ConnectivityManager::class.java) as ConnectivityManager)
@@ -104,6 +74,54 @@ class ConnectivityService : Service() {
}
}
private fun onNetworkState(network: Network, capabilities: NetworkCapabilities) {
// Always tracked (also under the kill switch) so the detector's baseline is
// current when relays come back.
Amber.instance.updateNetworkCapabilities(capabilities)
val reason = networkChangeDetector.onNetwork(capabilities.toSnapshot(network))
if (Amber.instance.settings.killSwitch.value) return
AmberLog.d(
"ServiceManager NetworkCallback",
"network ${network.networkHandle} mobile ${Amber.instance.isOnMobileDataState.value} wifi ${Amber.instance.isOnWifiDataState.value} vpn ${Amber.instance.isOnVpnState.value} change: $reason",
)
if (reason != null) {
if (Amber.instance.settings.torMode == TorMode.BUILTIN && !TorManager.isRunning.value) {
// Built-in Tor gave up earlier (bounded startup retries
// in runMigrations). The network is back, so retry now
// instead of waiting for a manual restart.
TorManager.restart(this, Amber.instance.applicationIOScope)
}
scheduleRelayReset(reason)
} else if (!Amber.instance.client.isActive()) {
scope.launch {
Amber.instance.client.connect()
}
}
}
/**
* Network changes arrive as a burst of callbacks (the new network, then its
* capabilities as validation completes); wait for it to settle so the relays are
* rebuilt once, on the final network.
*/
@Synchronized
private fun scheduleRelayReset(reason: String) {
pendingRelayReset?.cancel()
pendingRelayReset = scope.launch {
delay(NETWORK_SETTLE_MS)
Amber.instance.resetRelayConnections(reason)
}
}
@Synchronized
private fun cancelPendingRelayReset() {
pendingRelayReset?.cancel()
pendingRelayReset = null
}
override fun onBind(intent: Intent): IBinder? = null
override fun onCreate() {
@@ -184,6 +202,7 @@ class ConnectivityService : Service() {
override fun onDestroy() {
timer.cancel()
cancelPendingRelayReset()
if (!BuildFlavorChecker.isOfflineFlavor()) {
try {
AmberLog.d(Amber.TAG, "unregisterNetworkCallback")
@@ -232,5 +251,21 @@ class ConnectivityService : Service() {
* nothing changed, and a 30s period prevented doze 2,880 times a day.
*/
const val UPDATE_FILTER_PERIOD_MS = 5 * 60 * 1000L
private const val NETWORK_SETTLE_MS = 1_000L
private val TRACKED_TRANSPORTS = intArrayOf(
NetworkCapabilities.TRANSPORT_CELLULAR,
NetworkCapabilities.TRANSPORT_WIFI,
NetworkCapabilities.TRANSPORT_ETHERNET,
NetworkCapabilities.TRANSPORT_BLUETOOTH,
NetworkCapabilities.TRANSPORT_VPN,
)
private fun NetworkCapabilities.toSnapshot(network: Network) = NetworkSnapshot(
networkId = network.networkHandle,
transports = TRACKED_TRANSPORTS.filter { hasTransport(it) }.toSet(),
validated = hasCapability(NetworkCapabilities.NET_CAPABILITY_VALIDATED),
)
}
}
@@ -0,0 +1,79 @@
package com.greenart7c3.nostrsigner.service
import android.net.NetworkCapabilities.TRANSPORT_VPN
/**
* What the relay connections care about in the device's default network.
*
* @param networkId the network's handle; a different value means a different network
* (switching Wi-Fi <-> mobile, or a VPN taking over as the default network).
* @param transports the transports the network reports (Wi-Fi, cellular, ethernet, VPN, ...).
* A VPN network also reports its underlying transport, so a VPN moving from Wi-Fi to
* mobile shows up here even though the default network itself stays the same.
* @param validated whether Android verified the network actually reaches the internet.
*/
data class NetworkSnapshot(
val networkId: Long,
val transports: Set<Int>,
val validated: Boolean,
)
/**
* Decides when a default-network callback means the relay sockets must be rebuilt.
*
* Sockets opened on one network do not survive a move to another: the old routes are
* gone (or, with a VPN, the traffic now takes a different path) but the sockets keep
* looking connected until a ping times out. Callbacks arrive in bursts and repeat the
* same state, so this keeps the last snapshot and only reports real changes.
*/
class NetworkChangeDetector {
private var last: NetworkSnapshot? = null
private var lost = false
/**
* Records the current default network. Returns why the relays should be reset,
* or null when nothing that matters changed.
*/
@Synchronized
fun onNetwork(current: NetworkSnapshot): String? {
val previous = last
last = current
if (previous == null) {
// The first report after registering the callback is the network the
// relays are already connecting on; only a return after a loss is a change.
val wasLost = lost
lost = false
return if (wasLost) "network available again" else null
}
return when {
previous.networkId != current.networkId -> describeSwitch(previous, current)
previous.transports != current.transports -> describeSwitch(previous, current)
!previous.validated && current.validated -> "network regained internet access"
else -> null
}
}
/**
* Records that [networkId] went away. Returns true when it was the current default
* network; the loss of a network that was already replaced changes nothing.
*/
@Synchronized
fun onLost(networkId: Long): Boolean {
if (last?.networkId != networkId) return false
last = null
lost = true
return true
}
private fun describeSwitch(previous: NetworkSnapshot, current: NetworkSnapshot): String {
val wasVpn = TRANSPORT_VPN in previous.transports
val isVpn = TRANSPORT_VPN in current.transports
return when {
!wasVpn && isVpn -> "VPN connected"
wasVpn && !isVpn -> "VPN disconnected"
else -> "network changed"
}
}
}
@@ -4,16 +4,14 @@ import android.content.BroadcastReceiver
import android.content.Context
import android.content.Intent
import com.greenart7c3.nostrsigner.Amber
import com.greenart7c3.nostrsigner.relays.RelayHealthTracker
import kotlinx.coroutines.launch
class ReconnectReceiver : BroadcastReceiver() {
override fun onReceive(context: Context, intent: Intent) {
Amber.instance.applicationIOScope.launch {
// Manual reconnect: clear dead-relay state and re-run the filters so any
// relay previously dropped from the pool is re-added before reconnecting.
RelayHealthTracker.reset()
Amber.instance.checkForNewRelaysAndUpdateAllFilters(shouldReconnect = true)
// Manual reconnect: redial every relay now, ignoring dead-relay state and
// reconnect backoff, and re-add any relay previously dropped from the pool.
Amber.instance.resetRelayConnections("manual reconnect")
}
}
}
@@ -0,0 +1,62 @@
package com.greenart7c3.nostrsigner.relays
import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl
import org.junit.After
import org.junit.Assert.assertFalse
import org.junit.Assert.assertTrue
import org.junit.Before
import org.junit.Test
class RelayHealthTrackerTest {
private val relay = NormalizedRelayUrl("wss://relay.example.com/")
private var now = 1_000_000L
@Before
fun setUp() {
RelayHealthTracker.reset()
RelayHealthTracker.clock = { now }
}
@After
fun tearDown() {
RelayHealthTracker.reset()
RelayHealthTracker.clock = System::currentTimeMillis
}
private fun failUntilDead() {
repeat(10) { assertTrue(RelayHealthTracker.recordFailure(relay)) }
assertFalse(RelayHealthTracker.recordFailure(relay))
}
@Test
fun relayIsDeadAfterTooManyFailures() {
failUntilDead()
assertTrue(RelayHealthTracker.isDead(relay))
}
@Test
fun deadRelayGetsAnotherChanceAfterTheCooldown() {
failUntilDead()
now += RelayHealthTracker.DEAD_COOLDOWN_MS
assertFalse(RelayHealthTracker.isDead(relay))
}
@Test
fun failingAgainAfterTheCooldownRestartsIt() {
failUntilDead()
now += RelayHealthTracker.DEAD_COOLDOWN_MS
assertFalse(RelayHealthTracker.recordFailure(relay))
assertTrue(RelayHealthTracker.isDead(relay))
}
@Test
fun successAndResetClearTheDeadState() {
failUntilDead()
RelayHealthTracker.recordSuccess(relay)
assertFalse(RelayHealthTracker.isDead(relay))
failUntilDead()
RelayHealthTracker.reset()
assertFalse(RelayHealthTracker.isDead(relay))
}
}
@@ -0,0 +1,80 @@
package com.greenart7c3.nostrsigner.service
import android.net.NetworkCapabilities.TRANSPORT_CELLULAR
import android.net.NetworkCapabilities.TRANSPORT_VPN
import android.net.NetworkCapabilities.TRANSPORT_WIFI
import org.junit.Assert.assertEquals
import org.junit.Assert.assertFalse
import org.junit.Assert.assertNull
import org.junit.Assert.assertTrue
import org.junit.Test
class NetworkChangeDetectorTest {
private val wifi = NetworkSnapshot(1, setOf(TRANSPORT_WIFI), validated = true)
private val mobile = NetworkSnapshot(2, setOf(TRANSPORT_CELLULAR), validated = true)
private val vpnOverWifi = NetworkSnapshot(3, setOf(TRANSPORT_VPN, TRANSPORT_WIFI), validated = true)
@Test
fun firstNetworkIsTheBaselineNotAChange() {
val detector = NetworkChangeDetector()
assertNull(detector.onNetwork(wifi))
}
@Test
fun repeatedCallbacksForTheSameStateAreIgnored() {
val detector = NetworkChangeDetector()
detector.onNetwork(wifi)
assertNull(detector.onNetwork(wifi))
assertNull(detector.onNetwork(wifi.copy(validated = false)))
}
@Test
fun switchingBetweenWifiAndMobileIsAChange() {
val detector = NetworkChangeDetector()
detector.onNetwork(wifi)
assertEquals("network changed", detector.onNetwork(mobile))
assertEquals("network changed", detector.onNetwork(wifi))
}
@Test
fun vpnComingUpAndGoingDownIsAChange() {
val detector = NetworkChangeDetector()
detector.onNetwork(wifi)
assertEquals("VPN connected", detector.onNetwork(vpnOverWifi))
assertEquals("VPN disconnected", detector.onNetwork(wifi))
}
@Test
fun vpnMovingToAnotherUnderlyingTransportIsAChange() {
val detector = NetworkChangeDetector()
detector.onNetwork(vpnOverWifi)
assertEquals(
"network changed",
detector.onNetwork(vpnOverWifi.copy(transports = setOf(TRANSPORT_VPN, TRANSPORT_CELLULAR))),
)
}
@Test
fun regainingInternetAccessIsAChange() {
val detector = NetworkChangeDetector()
detector.onNetwork(wifi.copy(validated = false))
assertEquals("network regained internet access", detector.onNetwork(wifi))
}
@Test
fun networkReturningAfterALossIsAChange() {
val detector = NetworkChangeDetector()
detector.onNetwork(wifi)
assertTrue(detector.onLost(wifi.networkId))
assertEquals("network available again", detector.onNetwork(wifi))
}
@Test
fun losingANetworkThatWasAlreadyReplacedIsIgnored() {
val detector = NetworkChangeDetector()
detector.onNetwork(wifi)
detector.onNetwork(mobile)
assertFalse(detector.onLost(wifi.networkId))
assertNull(detector.onNetwork(mobile))
}
}