fix(relay): evict connection-establishment failures on the first strike

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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MSW59hJtP4Yn8fnRUxc7F5
2026-08-12 01:07:46 +00:00 · 2026-07-08 19:01:09 +00:00
parent f6fa262017
commit 5312b61164
2 changed files with 108 additions and 8 deletions
@@ -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.
@@ -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)"))
+    }
+}