diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/OkHttpWebSocket.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/OkHttpWebSocket.kt index 9f2ad864a6..d518f1e096 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/OkHttpWebSocket.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/OkHttpWebSocket.kt @@ -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() { diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/single/basic/BasicRelayClient.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/single/basic/BasicRelayClient.kt index 33877f0f88..4f00ead43f 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/single/basic/BasicRelayClient.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/single/basic/BasicRelayClient.kt @@ -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 diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/sockets/WebsocketBuilder.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/sockets/WebsocketBuilder.kt index aef749f774..ea8c197faa 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/sockets/WebsocketBuilder.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/sockets/WebsocketBuilder.kt @@ -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 } diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/single/basic/BasicRelayClientBackoffTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/single/basic/BasicRelayClientBackoffTest.kt new file mode 100644 index 0000000000..ad7c04c757 --- /dev/null +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/single/basic/BasicRelayClientBackoffTest.kt @@ -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) + } +}