fix: honor relay backoff during Tor bootstrap via per-relay config check

Before Tor is ready, relays were disconnecting and immediately reconnecting, ignoring BasicRelayClient's exponential backoff. RelayProxyClientConnector calls reconnect(ignoreRetryDelays = true) on every infrastructure change. That flag flows pool-wide into connectAndSyncFiltersIfDisconnected and unconditionally bypassed the backoff gate. While Tor is still bootstrapping its SOCKS port isn't listening, so each unrelated infra event (connectivity transitions, self-heal restarts, Tor status churn) forced an immediate reconnect that failed again — the backoff was computed (delay kept doubling) but never consulted. Make the bypass per-relay and conditional on the relay's transport config actually changing since its last attempt: - WebsocketBuilder gains connectionConfig(url): an opaque, value-comparable token of the transport config (proxy + timeouts). Default null = untracked, preserving legacy always-bypass behavior for in-process/standalone/test/desktop builders. - BasicRelayClient records the token at each attempt and only lets a forced reconnect skip the backoff when the token changed. A Tor relay whose SOCKS config is unchanged keeps honoring its backoff; the moment Tor flips to active (proxy port appears) the token changes and it reconnects immediately. Clearnet relays still retry immediately whenever their own client changes. - OkHttpWebSocket.Builder reports the proxy+timeout fingerprint, reused by needsReconnect() to avoid drift. Adds BasicRelayClientBackoffTest covering the honored/bypassed/untracked cases. https://claude.ai/code/session_01SCz8kdYs2FwesEyzbhmRPY
2026-08-11 08:47:33 +00:00 · 2026-05-29 22:30:53 +00:00
parent 1ec5f559e6
commit e1a651e72e
4 changed files with 190 additions and 17 deletions
@@ -34,6 +34,30 @@ import kotlinx.coroutines.launch
 import okhttp3.OkHttpClient
 import okhttp3.Request
 import okhttp3.Response
+import java.net.Proxy
+
+/**
+ * Value-comparable fingerprint of the relevant parts of an [OkHttpClient] for a relay
+ * connection: the proxy (Tor SOCKS or none) and the timeouts. Two clients with the same
+ * fingerprint connect the same way, so a relay does not need to reconnect — and a forced
+ * reconnect should not skip the backoff — when the fingerprint is unchanged.
+ */
+private data class RelayTransportConfig(
+    val proxy: Proxy?,
+    val connectTimeoutMillis: Int,
+    val readTimeoutMillis: Int,
+    val writeTimeoutMillis: Int,
+    val callTimeoutMillis: Int,
+)
+
+private fun OkHttpClient.relayTransportConfig() =
+    RelayTransportConfig(
+        proxy = proxy,
+        connectTimeoutMillis = connectTimeoutMillis,
+        readTimeoutMillis = readTimeoutMillis,
+        writeTimeoutMillis = writeTimeoutMillis,
+        callTimeoutMillis = callTimeoutMillis,
+    )

 class OkHttpWebSocket(
    val url: NormalizedRelayUrl,
@@ -50,21 +74,7 @@ class OkHttpWebSocket(

        val myUsingOkHttp = usingOkHttp ?: return true

-        val currentOkHttp = httpClient(url)
-
-        val usingProxy = myUsingOkHttp.proxy
-        val currentProxy = currentOkHttp.proxy
-
-        if (usingProxy != null && currentProxy != null && usingProxy != currentProxy) return true
-        if (usingProxy == null && currentProxy != null) return true
-        if (usingProxy != null && currentProxy == null) return true
-
-        if (currentOkHttp.readTimeoutMillis != myUsingOkHttp.readTimeoutMillis) return true
-        if (currentOkHttp.writeTimeoutMillis != myUsingOkHttp.writeTimeoutMillis) return true
-        if (currentOkHttp.connectTimeoutMillis != myUsingOkHttp.connectTimeoutMillis) return true
-        if (currentOkHttp.callTimeoutMillis != myUsingOkHttp.callTimeoutMillis) return true
-
-        return false
+        return myUsingOkHttp.relayTransportConfig() != httpClient(url).relayTransportConfig()
    }

    override fun connect() {
@@ -139,6 +149,11 @@ class OkHttpWebSocket(
            url: NormalizedRelayUrl,
            out: WebSocketListener,
        ) = OkHttpWebSocket(url, httpClient, out)
+
+        // Proxy + timeout fingerprint of the client this url would use right now.
+        // BasicRelayClient compares it against the last attempt to decide whether a
+        // forced reconnect may skip the exponential backoff.
+        override fun connectionConfig(url: NormalizedRelayUrl): Any = httpClient(url).relayTransportConfig()
    }

    override fun disconnect() {
@@ -74,6 +74,11 @@ open class BasicRelayClient(
    private var lastConnectTentativeInSeconds: Long = 0L // the beginning of time.
    private var delayToConnectInSeconds = DELAY_TO_RECONNECT_IN_SECS

+    // The transport config (proxy, timeouts) used on the last connection attempt.
+    // Lets a forced reconnect tell whether the situation that caused the failures
+    // actually changed (e.g. Tor finally came up) before skipping the backoff.
+    private var lastAttemptConfig: Any? = null
+
    // Makes sure only one socket is open for each url
    private var connectingMutex = AtomicBoolean(false)

@@ -98,6 +103,7 @@ open class BasicRelayClient(
            listener.onConnecting(this)

            lastConnectTentativeInSeconds = TimeUtils.now()
+            lastAttemptConfig = socketBuilder.connectionConfig(url)

            socket = socketBuilder.build(url, MyWebsocketListener())
            socket?.connect()
@@ -213,14 +219,31 @@ open class BasicRelayClient(

    override fun connectAndSyncFiltersIfDisconnected(ignoreRetryDelays: Boolean) {
        if (!isConnectionStarted() && !connectingMutex.load()) {
-            // waits 60 seconds to reconnect after disconnected.
-            if (ignoreRetryDelays || TimeUtils.now() > lastConnectTentativeInSeconds + delayToConnectInSeconds) {
+            // A forced reconnect (ignoreRetryDelays) only skips the backoff when this
+            // relay's transport config actually changed since the last attempt.
+            // Otherwise we honor the exponential backoff, so a relay that keeps failing
+            // under the same config (e.g. a Tor relay while Tor is still booting and the
+            // SOCKS port is not yet listening) is not reconnected-failed-reconnected on
+            // every unrelated infrastructure event.
+            if ((ignoreRetryDelays && transportConfigChanged()) ||
+                TimeUtils.now() > lastConnectTentativeInSeconds + delayToConnectInSeconds
+            ) {
                upRelayDelayToConnect()
                connect()
            }
        }
    }

+    /**
+     * True when the transport config this relay would use now differs from the one used
+     * on the last attempt — or when the socket builder doesn't track configs (returns
+     * null), in which case a requested bypass is always honored (legacy behavior).
+     */
+    private fun transportConfigChanged(): Boolean {
+        val current = socketBuilder.connectionConfig(url)
+        return current == null || current != lastAttemptConfig
+    }
+
    fun upRelayDelayToConnect() {
        if (delayToConnectInSeconds < TimeUtils.FIVE_MINUTES) {
            delayToConnectInSeconds = delayToConnectInSeconds * 2
@@ -27,4 +27,20 @@ interface WebsocketBuilder {
        url: NormalizedRelayUrl,
        out: WebSocketListener,
    ): WebSocket
+
+    /**
+     * Returns an opaque, value-comparable token describing the transport
+     * configuration (proxy, timeouts, ...) this builder would currently use for
+     * [url]. [com.vitorpamplona.quartz.nip01Core.relay.client.single.basic.BasicRelayClient]
+     * stores the token of its last connection attempt and, when a reconnect asks
+     * to ignore the backoff, only grants the bypass if this token changed since
+     * that attempt. That way a relay that keeps failing under the *same* config
+     * (e.g. a Tor relay while Tor is still bootstrapping and the SOCKS port is not
+     * yet listening) keeps honoring its exponential backoff instead of being
+     * hammered on every unrelated infrastructure event.
+     *
+     * The default returns `null`, which the client treats as "untracked" and
+     * therefore always honors the requested bypass (legacy behavior).
+     */
+    fun connectionConfig(url: NormalizedRelayUrl): Any? = null
 }
@@ -0,0 +1,119 @@
+/*
+ * Copyright (c) 2025 Vitor Pamplona
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy of
+ * this software and associated documentation files (the "Software"), to deal in
+ * the Software without restriction, including without limitation the rights to use,
+ * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the
+ * Software, and to permit persons to whom the Software is furnished to do so,
+ * subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in all
+ * copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
+ * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
+ * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN
+ * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
+ * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+ */
+package com.vitorpamplona.quartz.nip01Core.relay.client.single.basic
+
+import com.vitorpamplona.quartz.nip01Core.relay.client.listeners.EmptyConnectionListener
+import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl
+import com.vitorpamplona.quartz.nip01Core.relay.sockets.WebSocket
+import com.vitorpamplona.quartz.nip01Core.relay.sockets.WebSocketListener
+import com.vitorpamplona.quartz.nip01Core.relay.sockets.WebsocketBuilder
+import kotlin.test.Test
+import kotlin.test.assertEquals
+
+/**
+ * Regression tests for the per-relay backoff bypass in [BasicRelayClient].
+ *
+ * A forced reconnect (`ignoreRetryDelays = true`) must only skip the exponential
+ * backoff when the relay's transport config actually changed since the last failed
+ * attempt. This is what stops Tor relays from reconnect-fail-reconnecting on every
+ * infrastructure event while Tor is still bootstrapping (the SOCKS port is unchanged,
+ * so the config token is unchanged, so the backoff is honored).
+ */
+class BasicRelayClientBackoffTest {
+    private val url = NormalizedRelayUrl("wss://relay.test")
+
+    /**
+     * Fake transport: every [WebSocket.connect] immediately reports a Tor-style
+     * connection failure, returning the relay to the disconnected state. [config]
+     * is the value-comparable transport token the builder reports; tests mutate it
+     * to simulate Tor coming up / the proxy changing.
+     */
+    private class FakeBuilder(
+        var config: Any?,
+    ) : WebsocketBuilder {
+        var connectAttempts = 0
+
+        override fun build(
+            url: NormalizedRelayUrl,
+            out: WebSocketListener,
+        ): WebSocket =
+            object : WebSocket {
+                override fun needsReconnect() = true
+
+                override fun connect() {
+                    connectAttempts++
+                    // Simulate a Tor SOCKS port that isn't listening yet. The "failed to
+                    // connect to /127.0.0.1" message is the ignored-error path, so no long
+                    // backoff is forced — only the normal doubling applies.
+                    out.onFailure(RuntimeException("failed to connect to /127.0.0.1:9050"), null, null)
+                }
+
+                override fun disconnect() {}
+
+                override fun send(msg: String) = true
+            }
+
+        override fun connectionConfig(url: NormalizedRelayUrl): Any? = config
+    }
+
+    @Test
+    fun forcedReconnectHonorsBackoffWhenConfigUnchanged() {
+        val builder = FakeBuilder(config = "tor:9050:booting")
+        val relay = BasicRelayClient(url, builder, EmptyConnectionListener)
+
+        // First forced attempt: nothing has connected yet, so it fires and fails.
+        relay.connectAndSyncFiltersIfDisconnected(ignoreRetryDelays = true)
+        assertEquals(1, builder.connectAttempts)
+
+        // Tor is still booting: same config token. A second forced reconnect must NOT
+        // bypass the backoff (the failing situation hasn't changed), so no new attempt.
+        relay.connectAndSyncFiltersIfDisconnected(ignoreRetryDelays = true)
+        relay.connectAndSyncFiltersIfDisconnected(ignoreRetryDelays = true)
+        assertEquals(1, builder.connectAttempts)
+    }
+
+    @Test
+    fun forcedReconnectBypassesBackoffWhenConfigChanged() {
+        val builder = FakeBuilder(config = "tor:9050:booting")
+        val relay = BasicRelayClient(url, builder, EmptyConnectionListener)
+
+        relay.connectAndSyncFiltersIfDisconnected(ignoreRetryDelays = true)
+        assertEquals(1, builder.connectAttempts)
+
+        // Tor finished bootstrapping: the proxy this relay would use changed. A forced
+        // reconnect must now bypass the backoff and try immediately.
+        builder.config = "tor:17392:active"
+        relay.connectAndSyncFiltersIfDisconnected(ignoreRetryDelays = true)
+        assertEquals(2, builder.connectAttempts)
+    }
+
+    @Test
+    fun untrackedBuilderAlwaysHonorsForcedReconnect() {
+        // A builder that doesn't track configs (returns null) keeps the legacy behavior:
+        // a forced reconnect always tries immediately.
+        val builder = FakeBuilder(config = null)
+        val relay = BasicRelayClient(url, builder, EmptyConnectionListener)
+
+        relay.connectAndSyncFiltersIfDisconnected(ignoreRetryDelays = true)
+        relay.connectAndSyncFiltersIfDisconnected(ignoreRetryDelays = true)
+        assertEquals(2, builder.connectAttempts)
+    }
+}