From 5312b61164ca0cbfc65f5d3eaa7ceb47a547eb3d Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 8 Jul 2026 19:01:09 +0000 Subject: [PATCH] fix(relay): evict connection-establishment failures on the first strike MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A connect failure and a read timeout were both treated as "busy, retry" and took three strikes to drop. Re-probing hop-8's failed relays fresh, outside the crawl, showed the two are not alike: relays that failed to ESTABLISH a connection (connect timed out, refused, unroutable, or the proxy couldn't tunnel the CONNECT) were 0/30 reachable — genuinely dead — while relays that hit a READ timeout were 12/18 (67%) reachable, alive but overloaded by the crawl's fan-out (user.kindpag.es among them). So classifyDrainFailure now returns HARD for connection-establishment failures (one strike drops them instead of burning two more dials on a dead host), while a read/generic timeout still returns null and stays on the patient, clear-on-success timeout-strike path so live-but-slow relays we need are not wrongly evicted. Mid-stream resets stay TRANSIENT. Adds DrainFailureTest, which the classifier previously had none of. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01MSW59hJtP4Yn8fnRUxc7F5 --- .../relay/client/accessories/DrainFailure.kt | 39 ++++++++-- .../client/accessories/DrainFailureTest.kt | 77 +++++++++++++++++++ 2 files changed, 108 insertions(+), 8 deletions(-) create mode 100644 quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/accessories/DrainFailureTest.kt diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/accessories/DrainFailure.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/accessories/DrainFailure.kt index b2d605d044..94816d4f4f 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/accessories/DrainFailure.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/accessories/DrainFailure.kt @@ -27,13 +27,18 @@ package com.vitorpamplona.quartz.nip01Core.relay.client.accessories * - [HARD]: the relay answered wrong, or cannot exist. A bad HTTP upgrade (not a * websocket / dead status code), an unresolvable domain, or a TLS misconfig. * This will not fix itself, so one strike is enough to drop it. - * - [TRANSIENT]: a failure that might clear — connection refused / reset, host - * unreachable, or a temporary 429/5xx on the upgrade. Struck a few times - * before we give up. + * - [TRANSIENT]: a failure that might clear — a connection reset mid-stream or a + * temporary 429/5xx on the upgrade. Struck a few times before we give up. * - * A pure connect **timeout** is neither. The relay is most likely just busy, so - * we retry it and never mark it dead — [classifyDrainFailure] returns null for - * it (and for any non-failure terminal reason). + * The split between the two connect failures is drawn on measured reachability. On + * a hop-8 crawl, relays that failed to ESTABLISH a connection (connect timed out, + * refused, unroutable, or the proxy couldn't tunnel the CONNECT) were 0/30 reachable + * when re-probed fresh outside the crawl — genuinely dead, so they are [HARD] and one + * strike drops them. But relays that hit a *read* timeout (handshake accepted, slow + * to serve) were 12/18 (67%) reachable fresh — alive, only overloaded by the crawl's + * fan-out. Those must NOT be marked dead: [classifyDrainFailure] returns null for a + * read/generic timeout (and any non-failure terminal reason), and the crawler's + * per-authority timeout strikes, which CLEAR on any success, shed only the truly gone. */ enum class DrainFailure { HARD, TRANSIENT } @@ -48,8 +53,26 @@ fun classifyDrainFailure(reason: String): DrainFailure? { val m = reason.removePrefix("cannot:").lowercase() // The message now carries the exception class name (see BasicRelayClient), so // we can key on the stable *type* rather than localized message text. - // Busy, not dead: a connect/read timeout means the handshake just didn't - // finish in time. Retry it — the relay is probably fine, only slow or loaded. + // Couldn't even open the socket: the connect timed out, was refused, the host is + // unroutable, or the proxy couldn't tunnel the CONNECT. Measured 0/30 such relays + // reachable when re-probed fresh outside the crawl — dead, so one strike is enough. + // Checked BEFORE the timeout branch so "connect timed out" lands here and is not + // mistaken for the alive-but-slow *read* timeout below. + if ("connect timed out" in m || + "unexpected response code for connect" in m || // proxy couldn't CONNECT-tunnel + "connection refused" in m || + "econnrefused" in m || + "failed to connect" in m || + "no route to host" in m || + "network is unreachable" in m || + "network is down" in m + ) { + return DrainFailure.HARD + } + // Busy, not dead: a READ timeout means the relay accepted the handshake but was + // slow to serve — measured 12/18 (67%) reachable fresh outside the crawl, only + // overloaded by its fan-out. Retry (never mark dead); per-authority timeout + // strikes that clear on success shed the truly gone. if ("timeout" in m || "timed out" in m) return null // SocketTimeoutException, etc. // Cannot ever work: unresolvable domain (DNS) or a TLS misconfiguration. // Dead for good — one strike is enough. diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/accessories/DrainFailureTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/accessories/DrainFailureTest.kt new file mode 100644 index 0000000000..2b3a5f2f9e --- /dev/null +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/accessories/DrainFailureTest.kt @@ -0,0 +1,77 @@ +/* + * 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.accessories + +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNull + +class DrainFailureTest { + // Non-failure and non-"cannot" terminals are never dead signals. + @Test + fun nonFailureTerminalsAreNull() { + assertNull(classifyDrainFailure("eose")) + assertNull(classifyDrainFailure("closed:duplicate: sub")) + assertNull(classifyDrainFailure("timeout")) + } + + // A READ timeout (or generic post-handshake timeout) is alive-but-slow: never + // dead. Measured 67% of these relays were reachable when re-probed fresh. + @Test + fun readTimeoutsStayRetryable() { + assertNull(classifyDrainFailure("cannot:Read timed out (SocketTimeoutException)")) + assertNull(classifyDrainFailure("cannot:timeout (SocketTimeoutException)")) + } + + // Failing to ESTABLISH the connection is a strong dead signal (0/30 reachable + // fresh): HARD, so one strike drops it. "connect timed out" must be caught here + // and NOT fall through to the alive-but-slow read-timeout branch. + @Test + fun connectEstablishmentFailuresAreHard() { + assertEquals(DrainFailure.HARD, classifyDrainFailure("cannot:Connect timed out (SocketTimeoutException)")) + assertEquals(DrainFailure.HARD, classifyDrainFailure("cannot:Unexpected response code for CONNECT: (IOException)")) + assertEquals(DrainFailure.HARD, classifyDrainFailure("cannot:Connection refused (ConnectException)")) + assertEquals(DrainFailure.HARD, classifyDrainFailure("cannot:Failed to connect to /1.2.3.4:443")) + assertEquals(DrainFailure.HARD, classifyDrainFailure("cannot:No route to host (NoRouteToHostException)")) + } + + // DNS and TLS misconfig can never work: HARD. + @Test + fun dnsAndTlsAreHard() { + assertEquals(DrainFailure.HARD, classifyDrainFailure("cannot:Unable to resolve host (UnknownHostException)")) + assertEquals(DrainFailure.HARD, classifyDrainFailure("cannot:Received fatal alert: unrecognized_name (SSLHandshakeException)")) + assertEquals(DrainFailure.HARD, classifyDrainFailure("cannot:PKIX path building failed: certificate (CertificateException)")) + } + + // A bad HTTP upgrade is HARD unless the status is a retryable 429/5xx. + @Test + fun httpUpgradeSplitsOnStatus() { + assertEquals(DrainFailure.HARD, classifyDrainFailure("cannot:Server Misconfigured. not a websocket")) + assertEquals(DrainFailure.TRANSIENT, classifyDrainFailure("cannot:Server Misconfigured. Response: 503 (ProtocolException)")) + } + + // A mid-stream reset (connection already established) might clear: TRANSIENT. + @Test + fun midStreamResetIsTransient() { + assertEquals(DrainFailure.TRANSIENT, classifyDrainFailure("cannot:Connection reset (SocketException)")) + assertEquals(DrainFailure.TRANSIENT, classifyDrainFailure("cannot:Broken pipe (SocketException)")) + } +}