fix(pool): SimplePool.publish() should reject, not resolve, on connection failure

In publish(), the two early-exit failure cases (duplicate url,
allowConnectingToRelay returning false) correctly Promise.reject(). But the
ensureRelay() catch block instead returned a fulfilled string
("connection failure: " + err) — the one inconsistent case in the same
function.

This means Promise.any(pool.publish(...)) (the pattern this repo's own
README documents), or any other fulfilled-vs-rejected check, reports
success even when every relay connection failed, since Promise.any only
cares whether a promise settled as fulfilled, never what it resolved to.

Fix: reject with the same message instead of resolving with it, matching
the other two failure paths in this function. Added a test reproducing
the bug against an unreachable relay (confirmed failing before the fix,
passing after) using the existing mock-socket test infra — no new test
helpers needed, an unregistered mock URL already fails to connect the
same way a real unreachable relay would.

Verified: full suite passes except two pre-existing, unrelated failures
confirmed present on a clean checkout of master — nip77's live-network
test (unreachable from a sandboxed environment) and the timing-sensitive
ping-pong test.
This commit is contained in:
phoenix-server
2026-08-15 18:37:45 -03:00
committed by fiatjaf
parent 2e22f00a98
commit 5352a0187b
2 changed files with 27 additions and 1 deletions
+1 -1
View File
@@ -407,7 +407,7 @@ export class AbstractSimplePool {
})
} catch (err) {
this.onRelayConnectionFailure?.(url)
return String('connection failure: ' + String(err))
return Promise.reject('connection failure: ' + String(err))
}
return r
+26
View File
@@ -389,6 +389,32 @@ test('track relays when publishing', async () => {
expect(pool.seenOn.get(event2.id)).toBeUndefined()
})
test('publish() rejects (does not resolve) when a relay is unreachable', async () => {
// ensureRelay()'s failure was previously swallowed and turned into a
// *resolved* string ("connection failure: ..."), so callers using the
// documented `Promise.any(pool.publish(...))` pattern (or any other
// fulfilled-vs-rejected check) would see success even when every relay
// was unreachable. It must reject like the pool's other early failure
// paths (duplicate url, allowConnectingToRelay) already do.
let event = finalizeEvent(
{
kind: 1,
created_at: Math.floor(Date.now() / 1000),
tags: [],
content: 'hello',
},
generateSecretKey(),
)
const unreachable = 'wss://nobody-is-listening.invalid.mock/nothing'
const [settled] = await Promise.allSettled(pool.publish([unreachable], event))
expect(settled.status).toBe('rejected')
if (settled.status === 'rejected') {
expect(String(settled.reason)).toContain('connection failure')
}
})
test('oninvalidevent is called through the pool for invalid events', async done => {
const mockRelay = mockRelays[0]
const relay = await pool.ensureRelay(mockRelay.url)