Merge pull request #528 from greenart7c3/ccr-313a666a-euay3o

Fix relays staying disconnected and reset them on VPN changes
This commit is contained in:
greenart7c3
2026-10-05 08:14:03 -03:00
committed by GitHub
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))
}
}