fix(relay): stop a publish retry silently dialing nothing

The transport retry assumed the relay it is retrying is still in the connection
pool. `reconnect()` walks the pool's current relays, so when it is not, the
retry is issued, logged, dials nothing, and is reported at the deadline as the
same hang-up we already knew about — the one failure mode a retry must not
have, because it is indistinguishable from having tried.

A relay leaves the pool when nothing wants it any more. `NostrClient` recomputes
that set through `combine(...).sample(300)` and `RelayPool.updatePool` retires
whatever the sampled snapshot omits, socket included. A snapshot taken before
this publish claimed the relay therefore retires a relay with an event in
flight. The outbox still holds the event and would re-send it on the next
connect, so the only thing actually missing is pool membership.

`ensureInPool` restores it before the reconnect. Defaults to a no-op on
`INostrClient` rather than reusing `getOrCreateRelay`, which throws for clients
that expose no pool, and it is a no-op in the ordinary case where the relay
never left.

Not covered by a new test, deliberately rather than by omission: every
deterministic route to "relay absent from the pool" runs through the outbox
exhausting its own retry budget, and at that point the event has been abandoned
and NOT re-sending is correct. The one route that reaches this branch is the
300ms sampling window, which cannot be forced through the public API. So this
is defence in depth on an inferred cause, and the evidence for the inference is
the interop harness's `disconnected before OK` on test 22: the event
`a153a582…` IS stored in the harness relay's database, so the relay took it and
only the OK was lost; and the relay did not hang up (no rate limits configured,
a 20-minute idle timeout, and a 1024-message slow-client queue against a
database holding 107 events total), which leaves a client-side teardown.

Existing publish suites green.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016kCuA6tc4JQzHPCDd39GHq
This commit is contained in:
Claude
2026-09-11 11:45:09 +00:00
parent 3a1947e733
commit 0b3671f7c7
3 changed files with 36 additions and 3 deletions
@@ -57,6 +57,22 @@ interface INostrClient : AutoCloseable {
*/
fun resetBackoff() { }
/**
* Puts [url] back in the connection pool if it is no longer there, without dialing it.
*
* [reconnect] can only act on relays the pool still holds, so a caller that means to
* revive one it stopped hearing from has to restore that precondition first —
* otherwise the reconnect iterates past an empty pool and does nothing at all, which
* is indistinguishable from a relay that was asked and stayed silent.
*
* A relay leaves the pool when nothing wants it any more (no subscription, no count,
* no pending publish). That is normally the right call and normally permanent, so
* this is deliberately narrow: it restores membership and leaves dialing, backoff and
* filter syncing to [reconnect]. Defaults to a no-op — a client with no pool has
* nothing to restore and should not be forced to implement one.
*/
fun ensureInPool(url: NormalizedRelayUrl) { }
fun isActive(): Boolean
/**
@@ -274,6 +274,12 @@ class NostrClient(
relayPool.resetBackoff()
}
override fun ensureInPool(url: NormalizedRelayUrl) {
// Membership only. Connecting is reconnect()'s job, and going through the pool's
// own create keeps the relay client identical to the one publish would have made.
relayPool.createRelayIfAbsent(url)
}
override fun subscribe(
subId: String,
filters: Map<NormalizedRelayUrl, List<Filter>>,
@@ -273,9 +273,20 @@ suspend fun INostrClient.publishAndCollectResults(
}
// The event is still in the pool's outbox for this relay,
// so the dial is the whole job: the pool flushes what it
// owes the relay once the socket is back. Ignore the
// accumulated backoff — this is a user-visible publish
// waiting on it, not a background refresh.
// owes the relay once the socket is back.
//
// Except that a reconnect can only dial relays the pool
// still holds, and a relay can leave it — the desired set
// is sampled, so a snapshot taken before this publish
// claimed the relay retires it, socket and all. Restoring
// membership first is what keeps the retry from being a
// silent no-op: issued, logged, dialing nothing, and
// reported at the deadline as the hang-up we already knew
// about. A no-op when the relay is still there, which is
// the ordinary case.
ensureInPool(result.relay)
// Ignore the accumulated backoff — this is a user-visible
// publish waiting on it, not a background refresh.
resetBackoff()
reconnect(onlyIfChanged = false, ignoreRetryDelays = true)
}