mirror of
https://github.com/nbd-wtf/nostr-tools.git
synced 2026-07-22 07:48:27 +00:00
fix(reconnect): don't permanently kill subscriptions on a single failed reconnect attempt
A single failed reconnect attempt (`ws.onerror` or connect timeout)
set `skipReconnection = true`, which made `handleHardClose` close
every subscription on the relay and remove it from the pool. The
intent of fb7de7f was to give up on the *initial* connect failure
(a relay that's dead from the start) — but the same code path also
fires for retries, so any outage longer than the first 10s backoff
slot turned a transient drop into permanent silence on every
long-running client.
Symptom in the wild: a long-running daemon goes silently deaf on a
subscription after the relay operator restarts the relay, with no
error from the SDK because the pool still considers the relay
connected after a later `ensureRelay`. Only `skipReconnection` on
the first attempt (`reconnectAttempts === 0`), so retries fall
through to `handleHardClose` and schedule another attempt as the
`resubscribeBackoff` array clearly intended.
This commit is contained in:
@@ -141,7 +141,11 @@ export class AbstractRelay {
|
||||
connectionTimeoutHandle = setTimeout(() => {
|
||||
reject('connection timed out')
|
||||
this.connectionPromise = undefined
|
||||
// Only give up on the initial connect; a slow reconnect
|
||||
// should fall through to the next backoff slot.
|
||||
if (this.reconnectAttempts === 0) {
|
||||
this.skipReconnection = true
|
||||
}
|
||||
this.onclose?.()
|
||||
this.handleHardClose('relay connection timed out')
|
||||
}, opts.timeout)
|
||||
@@ -193,7 +197,12 @@ export class AbstractRelay {
|
||||
clearTimeout(connectionTimeoutHandle)
|
||||
reject('connection failed')
|
||||
this.connectionPromise = undefined
|
||||
// Only give up on the initial connect. A failed reconnect
|
||||
// attempt must fall through so the next backoff slot fires;
|
||||
// otherwise one failed retry tears down every subscription.
|
||||
if (this.reconnectAttempts === 0) {
|
||||
this.skipReconnection = true
|
||||
}
|
||||
this.onclose?.()
|
||||
this.handleHardClose('relay connection failed')
|
||||
}
|
||||
|
||||
@@ -338,6 +338,45 @@ test('reconnect on disconnect', async () => {
|
||||
expect(closes).toBe(1) // should not have closed again
|
||||
})
|
||||
|
||||
test('reconnect survives a failed reconnect attempt and recovers when the relay returns', async () => {
|
||||
const mockRelay = new MockRelay()
|
||||
const relay = new Relay(mockRelay.url, { enableReconnect: true })
|
||||
relay.resubscribeBackoff = [50, 50, 100, 100] // short backoff for testing
|
||||
|
||||
await relay.connect()
|
||||
expect(relay.connected).toBeTrue()
|
||||
|
||||
relay.subscribe([{ kinds: [1] }], { onevent: () => {} })
|
||||
expect(relay.openSubs.size).toBe(1)
|
||||
|
||||
// Drop server + close live socket so the scheduled reconnect fails.
|
||||
;(mockRelay as any)._server.stop()
|
||||
;(relay as any).ws?.close()
|
||||
|
||||
// Past one failed reconnect attempt: the sub must still be alive
|
||||
// (buggy code clears openSubs here via skipReconnection).
|
||||
await new Promise(resolve => setTimeout(resolve, 300))
|
||||
expect(relay.connected).toBeFalse()
|
||||
expect(relay.openSubs.size).toBe(1)
|
||||
|
||||
// Bring the relay back; the next backoff slot must reconnect.
|
||||
new MockRelay(mockRelay.url)
|
||||
|
||||
await new Promise<void>((resolve, reject) => {
|
||||
const deadline = setTimeout(() => reject(new Error('relay never reconnected')), 2000)
|
||||
const interval = setInterval(() => {
|
||||
if (relay.connected) {
|
||||
clearTimeout(deadline)
|
||||
clearInterval(interval)
|
||||
resolve()
|
||||
}
|
||||
}, 10)
|
||||
})
|
||||
expect(relay.openSubs.size).toBe(1)
|
||||
|
||||
relay.close()
|
||||
})
|
||||
|
||||
test('oninvalidevent is called for malformed events', async done => {
|
||||
const mockRelay = new MockRelay()
|
||||
const relay = new Relay(mockRelay.url)
|
||||
|
||||
Reference in New Issue
Block a user