From de515462e674b7f2d3d0c10fec5fc9e5b3e2d04a Mon Sep 17 00:00:00 2001 From: thePR0M3TH3AN <53631862+PR0M3TH3AN@users.noreply.github.com> Date: Sat, 20 Jun 2026 12:14:03 -0400 Subject: [PATCH] fix(feed): always reserve live default relays in the feed set MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The video feed's getDefaultRelays only fell back to bitvid's default relays when the healthy set was COMPLETELY empty. A user whose own relays are all dead but not yet probed (so they still pass the liveness filter) would have the feed subscribe to that fully-broken list and hang forever on "Fetching…" — the default aggregators were never injected. Route the healthy set through capReadRelays (via new buildFeedRelaySet), which always reserves a couple of reliable aggregator slots. Now the feed can reach live relays regardless of how broken the user's personal list is, while still prioritizing the user's own relays and bounding REQ fan-out to MAX_SUBSCRIBE_RELAYS. Tests (buildFeedRelaySet): an all-dead user list still yields a default aggregator while keeping the user's relays; empty input falls back to defaults; an empty healthy set uses the configured fallback yet still reserves defaults; a huge list stays bounded. Mutation-verified — reverting to the old healthy-or-defaults logic fails 3 of the 4 scenarios. Build + full lint green. Co-Authored-By: Claude Opus 4.8 --- js/nostr/client.js | 7 ++- js/nostr/toolkit.js | 22 ++++++++ tests/feed-relay-fallback.test.mjs | 80 ++++++++++++++++++++++++++++++ 3 files changed, 108 insertions(+), 1 deletion(-) create mode 100644 tests/feed-relay-fallback.test.mjs diff --git a/js/nostr/client.js b/js/nostr/client.js index e20a169e..fb522f14 100644 --- a/js/nostr/client.js +++ b/js/nostr/client.js @@ -121,6 +121,7 @@ import { DEFAULT_RELAY_URLS, RELAY_URLS, capReadRelays, + buildFeedRelaySet, ensureNostrTools, getCachedNostrTools, nostrToolsBootstrapFailure, @@ -3574,7 +3575,11 @@ export class NostrClient { getPool: () => this.pool, getDefaultRelays: () => { const healthy = sanitizeRelayList(this.getHealthyRelays(this.relays)); - return healthy.length ? healthy : Array.from(DEFAULT_RELAY_URLS); + // Guarantee reliable default aggregators are always in the feed set, + // even when the user's own relays survive the health filter but are + // actually dead — otherwise the feed subscribes to a fully-broken + // list and hangs forever on "Fetching…". + return buildFeedRelaySet(healthy, this.relays); }, // Own registry, isolated from the global singleton (which other // subsystems use for their own keys) and from other client instances. diff --git a/js/nostr/toolkit.js b/js/nostr/toolkit.js index 501a6d2b..96361159 100644 --- a/js/nostr/toolkit.js +++ b/js/nostr/toolkit.js @@ -102,6 +102,28 @@ export function capReadRelays(urls) { ).slice(0, MAX_SUBSCRIBE_RELAYS); } +// Build the relay set for the main video feed. +// +// The subtle failure this guards: a user whose own relays are all DEAD but not +// yet probed (so they still pass the liveness filter) would otherwise have the +// feed subscribe only to those dead relays and hang on "Fetching…" forever, +// because the known-good defaults were never injected. capReadRelays always +// reserves a couple of reliable aggregator slots, so routing the healthy set +// through it guarantees the feed can reach live relays even when the user's +// entire personal list is broken. Falls back to the full default set if nothing +// survives at all. +export function buildFeedRelaySet(healthyRelays, fallbackRelays = []) { + const healthy = Array.isArray(healthyRelays) + ? healthyRelays.filter((u) => typeof u === "string" && u.trim()) + : []; + const fallback = Array.isArray(fallbackRelays) + ? fallbackRelays.filter((u) => typeof u === "string" && u.trim()) + : []; + const base = healthy.length ? healthy : fallback; + const capped = capReadRelays(base); + return capped.length ? capped : Array.from(DEFAULT_RELAY_URLS); +} + const globalScope = typeof window !== "undefined" ? window diff --git a/tests/feed-relay-fallback.test.mjs b/tests/feed-relay-fallback.test.mjs new file mode 100644 index 00000000..dc217a3d --- /dev/null +++ b/tests/feed-relay-fallback.test.mjs @@ -0,0 +1,80 @@ +// The main video feed must never starve itself onto a fully-broken relay list. +// A user whose own relays are all dead (but not yet probed, so they still pass +// the liveness filter) would otherwise have the feed subscribe only to those +// dead relays and hang on "Fetching…" forever. buildFeedRelaySet guarantees the +// reliable default aggregators are always present so the feed can reach live +// relays regardless of how broken the user's personal list is. + +import assert from "node:assert/strict"; +import test from "node:test"; +import { + buildFeedRelaySet, + DEFAULT_RELAY_URLS, + MAX_SUBSCRIBE_RELAYS, +} from "../js/nostr/toolkit.js"; + +const containsAnyDefault = (relays) => + relays.some((url) => DEFAULT_RELAY_URLS.includes(url)); + +test("a healthy-looking but all-dead user list still yields default aggregators", () => { + // None of these are bitvid defaults — they're the user's own (dead) relays + // that happened to survive the health filter because they weren't probed yet. + const deadUserRelays = [ + "wss://dead-one.example.com", + "wss://dead-two.example.com", + "wss://dead-three.example.com", + ]; + + const result = buildFeedRelaySet(deadUserRelays, deadUserRelays); + + assert.ok( + containsAnyDefault(result), + `feed set must include at least one default aggregator; got ${JSON.stringify(result)}`, + ); + // The user's own relays are still prioritized — they aren't discarded. + assert.ok( + deadUserRelays.every((url) => result.includes(url)), + "the user's own relays should remain in the feed set", + ); +}); + +test("falls back to defaults when the healthy set is empty", () => { + // Healthy is empty AND the fallback (this.relays) is empty too. + const result = buildFeedRelaySet([], []); + assert.ok(result.length > 0, "must never return an empty feed relay set"); + assert.ok( + containsAnyDefault(result), + "an empty input must fall back to default aggregators", + ); +}); + +test("uses the fallback list when the healthy set is empty but relays exist", () => { + // Simulates getHealthyRelays() filtering everything out, but this.relays still + // holds the (capped) configured set. + const configured = ["wss://configured.example.com"]; + const result = buildFeedRelaySet([], configured); + assert.ok( + result.includes("wss://configured.example.com"), + "configured relays should be used when nothing is currently healthy", + ); + assert.ok( + containsAnyDefault(result), + "defaults are still reserved alongside the fallback list", + ); +}); + +test("bounds the feed set so a huge user list can't flood REQ fan-out", () => { + const many = Array.from( + { length: 30 }, + (_, i) => `wss://user-relay-${i}.example.com`, + ); + const result = buildFeedRelaySet(many, many); + assert.ok( + result.length <= MAX_SUBSCRIBE_RELAYS, + `feed set must be bounded to ${MAX_SUBSCRIBE_RELAYS}; got ${result.length}`, + ); + assert.ok( + containsAnyDefault(result), + "even a full user list must reserve default aggregators", + ); +});