diff --git a/js/app.js b/js/app.js index 8c3fea39..b078395b 100644 --- a/js/app.js +++ b/js/app.js @@ -89,6 +89,7 @@ import { buildDmRelayListEvent, buildShareEvent } from "./nostrEventSchemas.js"; import { publishEventToRelays, assertAnyRelayAccepted, + assertAnyRelayAcceptedOrUnconfirmed, describePublishOutcome, readRelayPublishSummary, } from "./nostrPublish.js"; @@ -2905,24 +2906,17 @@ class Application { signedEvent, ); - let publishSummary; - try { - publishSummary = assertAnyRelayAccepted(publishResults, { - context: "dm relay hints update", - message: "No relays accepted the DM relay list.", - }); - } catch (publishError) { - if (publishError?.relayFailures?.length) { - publishError.relayFailures.forEach( - ({ url, error: relayError, reason }) => { - userLogger.error( - `[Application] Relay ${url} rejected DM relay list: ${reason}`, - relayError || reason, - ); - }, - ); - } - throw publishError; + // An all-timeout (unconfirmed) publish of the replaceable DM relay list is a + // soft success — sent + almost always persisted; don't error/revert. Explicit + // rejections still throw. + const publishSummary = assertAnyRelayAcceptedOrUnconfirmed(publishResults, { + context: "dm relay hints update", + message: "No relays accepted the DM relay list.", + }); + if (publishSummary.unconfirmed) { + userLogger.warn( + "[Application] DM relay list not acknowledged by any relay within the timeout; treating as optimistic success (reconciles on next load).", + ); } if (publishSummary.failed.length) { diff --git a/js/nostrPublish.js b/js/nostrPublish.js index 5b561d55..a9a7a751 100644 --- a/js/nostrPublish.js +++ b/js/nostrPublish.js @@ -335,6 +335,48 @@ export function describePublishOutcome({ accepted = 0, total = 0 } = {}) { }; } +// A RelayPublishError whose per-relay failures are ALL "publish timeout" means the +// event was sent but not ACKed in the window — NOT that any relay rejected it. For +// an idempotent, replaceable event this almost always persisted (users see it +// after a refresh), so callers that publish such events can treat this as a soft +// success instead of a hard error. An explicit rejection has a different reason +// and returns false, so it still fails loudly. +export function relayPublishFailuresAreAllTimeouts(error) { + const failures = Array.isArray(error?.relayFailures) ? error.relayFailures : []; + if (!failures.length) { + return false; + } + return failures.every((failure) => { + const reason = typeof failure?.reason === "string" ? failure.reason : ""; + return /timeout/i.test(reason); + }); +} + +// Like assertAnyRelayAccepted, but for idempotent/replaceable publishes: when the +// only reason nothing was accepted is that every relay timed out (unconfirmed), +// don't throw — return a summary flagged `{ unconfirmed: true }` so the caller can +// treat it as an optimistic success (reconciles on next load). An explicit relay +// rejection (or an empty/other failure set) still throws exactly as before. +export function assertAnyRelayAcceptedOrUnconfirmed(results = [], options = {}) { + try { + const summary = assertAnyRelayAccepted(results, options); + return { ...summary, unconfirmed: false }; + } catch (error) { + if (error instanceof RelayPublishError && relayPublishFailuresAreAllTimeouts(error)) { + return { + accepted: [], + failed: (error.relayFailures || []).map((f) => ({ + url: f.url, + error: f.error || null, + reason: f.reason, + })), + unconfirmed: true, + }; + } + throw error; + } +} + export function assertAnyRelayAccepted(results = [], options = {}) { const summary = summarizePublishResults(results); if (summary.accepted.length > 0) { diff --git a/js/relayManager.js b/js/relayManager.js index 9d5d2d3b..f66b69ac 100644 --- a/js/relayManager.js +++ b/js/relayManager.js @@ -9,7 +9,7 @@ import { buildRelayListEvent } from "./nostrEventSchemas.js"; import { devLogger, userLogger } from "./utils/logger.js"; import { publishEventToRelays, - assertAnyRelayAccepted, + assertAnyRelayAcceptedOrUnconfirmed, } from "./nostrPublish.js"; import { profileCache } from "./state/profileCache.js"; @@ -812,24 +812,17 @@ class RelayPreferencesManager { signedEvent ); - let publishSummary; - try { - publishSummary = assertAnyRelayAccepted(publishResults, { - context: "relay preferences update", - message: "No relays accepted the update.", - }); - } catch (publishError) { - if (publishError?.relayFailures?.length) { - publishError.relayFailures.forEach( - ({ url, error: relayError, reason }) => { - userLogger.error( - `[RelayPreferencesManager] Relay ${url} rejected relay list: ${reason}`, - relayError || reason - ); - } - ); - } - throw publishError; + // An all-timeout (unconfirmed) publish of the replaceable relay list is a soft + // success — sent + almost always persisted; don't error/revert. Explicit + // rejections still throw. + const publishSummary = assertAnyRelayAcceptedOrUnconfirmed(publishResults, { + context: "relay preferences update", + message: "No relays accepted the update.", + }); + if (publishSummary.unconfirmed) { + userLogger.warn( + "[RelayPreferencesManager] Relay list not acknowledged by any relay within the timeout; treating as optimistic success (reconciles on next load).", + ); } if (publishSummary.failed.length) { diff --git a/js/services/hashtagPreferencesService.js b/js/services/hashtagPreferencesService.js index 68817795..07ad6e30 100644 --- a/js/services/hashtagPreferencesService.js +++ b/js/services/hashtagPreferencesService.js @@ -12,7 +12,7 @@ import { } from "../nostrEventSchemas.js"; import { publishEventToRelays, - assertAnyRelayAccepted, + assertAnyRelayAcceptedOrUnconfirmed, } from "../nostrPublish.js"; import { userLogger, devLogger } from "../utils/logger.js"; import { @@ -1453,9 +1453,17 @@ class HashtagPreferencesService { signedEvent, ); - const publishSummary = assertAnyRelayAccepted(publishResults, { + // All-timeout (unconfirmed) publish of the replaceable preferences list is a + // soft success — sent + almost always persisted; don't throw. Explicit + // rejections still throw. + const publishSummary = assertAnyRelayAcceptedOrUnconfirmed(publishResults, { context: "hashtag-preferences", }); + if (publishSummary.unconfirmed) { + userLogger.warn( + `${LOG_PREFIX} Preferences not acknowledged by any relay within the timeout; treating as optimistic success (reconciles on next load).`, + ); + } if (publishSummary.failed?.length) { publishSummary.failed.forEach(({ url, error }) => { diff --git a/js/subscriptions.js b/js/subscriptions.js index 8fa6e846..96a271ab 100644 --- a/js/subscriptions.js +++ b/js/subscriptions.js @@ -21,7 +21,7 @@ import { import { CACHE_POLICIES, STORAGE_TIERS } from "./nostr/cachePolicies.js"; import { publishEventToRelays, - assertAnyRelayAccepted + assertAnyRelayAcceptedOrUnconfirmed, } from "./nostrPublish.js"; import { getApplication } from "./applicationContext.js"; import { VideoListView } from "./ui/views/VideoListView.js"; @@ -175,25 +175,6 @@ const subscribeVideoViewEventsApi = (pointer, options) => * Also handles fetching and rendering subscribed channels' videos * in the same card style as your home page. */ -// A RelayPublishError whose failures are ALL "publish timeout" (no relay ACKed in -// the window) means the event was sent but not confirmed — not that it was -// rejected. For an idempotent, replaceable list this almost always persisted (the -// user sees it after a refresh), so we treat it as a soft success rather than a -// hard error that would revert the optimistic UI. An explicit relay rejection has -// a different reason and is NOT a timeout, so it still fails loudly. -export function subscriptionPublishFailuresAreAllTimeouts(publishError) { - const failures = Array.isArray(publishError?.relayFailures) - ? publishError.relayFailures - : []; - if (!failures.length) { - return false; - } - return failures.every((failure) => { - const reason = typeof failure?.reason === "string" ? failure.reason : ""; - return /timeout/i.test(reason); - }); -} - class SubscriptionsManager { constructor() { this.subscribedPubkeys = new Set(); @@ -1471,35 +1452,19 @@ class SubscriptionsManager { signedEvent ); - let publishSummary = null; - let softSuccess = false; - try { - publishSummary = assertAnyRelayAccepted(publishResults, { - context: "subscription list" - }); - } catch (publishError) { - if (publishError?.relayFailures?.length) { - publishError.relayFailures.forEach( - ({ url, error: relayError, reason }) => { - userLogger.warn( - `[SubscriptionsManager] Subscription list not acked by ${url}: ${reason}`, - relayError || reason - ); - } - ); - } - // Only an explicit relay rejection (or a pre-publish signer/encryption - // error, thrown earlier) is a real failure. A result where every relay - // merely timed out means the event was sent to an idempotent, replaceable - // list and almost certainly persisted — surface it as a soft success so the - // optimistic UI is NOT reverted and the user doesn't see a spurious error. - if (!subscriptionPublishFailuresAreAllTimeouts(publishError)) { - throw publishError; - } + // Only an explicit relay rejection (or a pre-publish signer/encryption error, + // thrown earlier) is a real failure. A result where every relay merely timed + // out means the event was sent to an idempotent, replaceable list and almost + // certainly persisted — treated as a soft success (unconfirmed) so the + // optimistic UI is NOT reverted and the user doesn't see a spurious error. + const publishSummary = assertAnyRelayAcceptedOrUnconfirmed(publishResults, { + context: "subscription list", + }); + const softSuccess = publishSummary.unconfirmed === true; + if (softSuccess) { userLogger.warn( "[SubscriptionsManager] Subscription list not acknowledged by any relay within the timeout; treating as optimistic success (reconciles on next load).", ); - softSuccess = true; } if (publishSummary?.failed?.length) { diff --git a/js/userBlocks.js b/js/userBlocks.js index 6becbb2a..22fcd970 100644 --- a/js/userBlocks.js +++ b/js/userBlocks.js @@ -24,7 +24,7 @@ import { import { STANDARD_TIMEOUT_MS, MAX_BLOCKLIST_ENTRIES } from "./constants.js"; import { publishEventToRelays, - assertAnyRelayAccepted, + assertAnyRelayAcceptedOrUnconfirmed, } from "./nostrPublish.js"; import { profileCache } from "./state/profileCache.js"; import { DEFAULT_RELAY_URLS, capReadRelays } from "./nostr/toolkit.js"; @@ -2383,23 +2383,17 @@ class UserBlockListManager { signedEvent ); - let publishSummary; - try { - publishSummary = assertAnyRelayAccepted(publishResults, { - context: "mute list", - }); - } catch (publishError) { - if (publishError?.relayFailures?.length) { - publishError.relayFailures.forEach( - ({ url, error: relayError, reason }) => { - userLogger.error( - `[UserBlockList] Mute list rejected by ${url}: ${reason}`, - relayError || reason - ); - } - ); - } - throw publishError; + // An all-timeout (unconfirmed) publish of this idempotent, replaceable mute + // list is treated as a soft success — the event was sent and almost always + // persists (it shows up after a refresh), so don't surface a spurious error / + // revert the optimistic state. Explicit rejections still throw. + const publishSummary = assertAnyRelayAcceptedOrUnconfirmed(publishResults, { + context: "mute list", + }); + if (publishSummary.unconfirmed) { + userLogger.warn( + "[UserBlockList] Mute list not acknowledged by any relay within the timeout; treating as optimistic success (reconciles on next load).", + ); } if (publishSummary.failed.length) { diff --git a/tests/subscription-publish-timeout.test.mjs b/tests/subscription-publish-timeout.test.mjs index 8ae8b357..0f354174 100644 --- a/tests/subscription-publish-timeout.test.mjs +++ b/tests/subscription-publish-timeout.test.mjs @@ -27,8 +27,11 @@ import test from "node:test"; import { strict as assert } from "node:assert"; -import { RelayPublishError } from "../js/nostrPublish.js"; -import { subscriptionPublishFailuresAreAllTimeouts } from "../js/subscriptions.js"; +import { + RelayPublishError, + relayPublishFailuresAreAllTimeouts as subscriptionPublishFailuresAreAllTimeouts, + assertAnyRelayAcceptedOrUnconfirmed, +} from "../js/nostrPublish.js"; // Build a RelayPublishError the same way assertAnyRelayAccepted does — from the // per-relay result objects (success:false + an Error whose message is the reason). @@ -64,3 +67,37 @@ test("case-insensitive: 'Publish Timeout' still counts as a timeout", () => { const e = errFrom([rejected("wss://a", "Publish Timeout")]); assert.equal(subscriptionPublishFailuresAreAllTimeouts(e), true); }); + +// assertAnyRelayAcceptedOrUnconfirmed is the shared helper every list-publish +// caller now uses (subscriptions/blocks/relay-list/dm-hints/hashtags). +const okResult = (url) => ({ url, success: true, error: null }); + +test("shared helper: a confirmed ack returns unconfirmed:false with accepted", () => { + const summary = assertAnyRelayAcceptedOrUnconfirmed( + [okResult("wss://a"), timeout("wss://b")], + { context: "list" }, + ); + assert.equal(summary.unconfirmed, false); + assert.equal(summary.accepted.length, 1); +}); + +test("shared helper: all-timeout returns unconfirmed:true and does NOT throw", () => { + const summary = assertAnyRelayAcceptedOrUnconfirmed( + [timeout("wss://a"), timeout("wss://b")], + { context: "list" }, + ); + assert.equal(summary.unconfirmed, true); + assert.equal(summary.accepted.length, 0); + assert.equal(summary.failed.length, 2); +}); + +test("shared helper: an explicit rejection still throws RelayPublishError", () => { + assert.throws( + () => + assertAnyRelayAcceptedOrUnconfirmed( + [timeout("wss://a"), rejected("wss://b", "blocked: spam")], + { context: "list" }, + ), + RelayPublishError, + ); +});