From 5352a0187badf1bcfab7af14a4cb2d2539f698af Mon Sep 17 00:00:00 2001 From: phoenix-server Date: Sat, 15 Aug 2026 13:08:07 -0400 Subject: [PATCH] fix(pool): SimplePool.publish() should reject, not resolve, on connection failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- abstract-pool.ts | 2 +- pool.test.ts | 26 ++++++++++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/abstract-pool.ts b/abstract-pool.ts index 9b5a65c..77d86c4 100644 --- a/abstract-pool.ts +++ b/abstract-pool.ts @@ -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 diff --git a/pool.test.ts b/pool.test.ts index 8d4ff3d..4278ee3 100644 --- a/pool.test.ts +++ b/pool.test.ts @@ -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)