[nostr] Fix spurious "failed" errors on unconfirmed list publishes across the board

Same class of bug as the subscription fix, applied everywhere an idempotent,
replaceable list is published with an optimistic UI: when no relay ACKs within the
10s window (slow/flaky relays), the event was still sent and almost always
persists (it shows up after a refresh), but assertAnyRelayAccepted threw -> the UI
showed a scary error / reverted, even though it worked.

- nostrPublish.js: add shared helpers relayPublishFailuresAreAllTimeouts() and
  assertAnyRelayAcceptedOrUnconfirmed() — the latter returns a summary flagged
  { unconfirmed: true } on all-timeout instead of throwing; explicit relay
  rejections (and pre-publish signer/encryption errors) still throw.
- Applied to the replaceable-list publishes: subscriptions (kind 30000, refactored
  to the shared helper), user mute/block list (10000), relay list (10002), DM
  relay hints (10050), hashtag preferences (30015).

Left as-is for now (higher-stakes, separate follow-up): the generic content
publish path (video/edit/delete/revert/rebroadcast) in publishHelpers/client.js —
same latent issue but the video outcome-describer needs careful handling.

Tests: tests/subscription-publish-timeout.test.mjs (8) now cover both the
classifier and assertAnyRelayAcceptedOrUnconfirmed (confirmed->summary,
all-timeout->soft, rejection->throws). hashtag-preferences suite green (14).

Co-Authored-By: Claude Opus 4.8 <[email protected]>
This commit is contained in:
thePR0M3TH3ANandClaude Opus 4.8 committed 2026-07-01 15:20:58 -04:00
1 parent a9271fd0e6
commit 47f2a9a865
7 files changed
+138 -105

No files matched your search

+12 -18
View File
@@ -89,6 +89,7 @@ import { buildDmRelayListEvent, buildShareEvent } from "./nostrEventSchemas.js";
import { import {
publishEventToRelays, publishEventToRelays,
assertAnyRelayAccepted, assertAnyRelayAccepted,
assertAnyRelayAcceptedOrUnconfirmed,
describePublishOutcome, describePublishOutcome,
readRelayPublishSummary, readRelayPublishSummary,
} from "./nostrPublish.js"; } from "./nostrPublish.js";
@@ -2905,24 +2906,17 @@ class Application {
signedEvent, signedEvent,
); );
let publishSummary; // An all-timeout (unconfirmed) publish of the replaceable DM relay list is a
try { // soft success — sent + almost always persisted; don't error/revert. Explicit
publishSummary = assertAnyRelayAccepted(publishResults, { // rejections still throw.
context: "dm relay hints update", const publishSummary = assertAnyRelayAcceptedOrUnconfirmed(publishResults, {
message: "No relays accepted the DM relay list.", context: "dm relay hints update",
}); message: "No relays accepted the DM relay list.",
} catch (publishError) { });
if (publishError?.relayFailures?.length) { if (publishSummary.unconfirmed) {
publishError.relayFailures.forEach( userLogger.warn(
({ url, error: relayError, reason }) => { "[Application] DM relay list not acknowledged by any relay within the timeout; treating as optimistic success (reconciles on next load).",
userLogger.error( );
`[Application] Relay ${url} rejected DM relay list: ${reason}`,
relayError || reason,
);
},
);
}
throw publishError;
} }
if (publishSummary.failed.length) { if (publishSummary.failed.length) {
+42
View File
@@ -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 = {}) { export function assertAnyRelayAccepted(results = [], options = {}) {
const summary = summarizePublishResults(results); const summary = summarizePublishResults(results);
if (summary.accepted.length > 0) { if (summary.accepted.length > 0) {
+12 -19
View File
@@ -9,7 +9,7 @@ import { buildRelayListEvent } from "./nostrEventSchemas.js";
import { devLogger, userLogger } from "./utils/logger.js"; import { devLogger, userLogger } from "./utils/logger.js";
import { import {
publishEventToRelays, publishEventToRelays,
assertAnyRelayAccepted, assertAnyRelayAcceptedOrUnconfirmed,
} from "./nostrPublish.js"; } from "./nostrPublish.js";
import { profileCache } from "./state/profileCache.js"; import { profileCache } from "./state/profileCache.js";
@@ -812,24 +812,17 @@ class RelayPreferencesManager {
signedEvent signedEvent
); );
let publishSummary; // An all-timeout (unconfirmed) publish of the replaceable relay list is a soft
try { // success — sent + almost always persisted; don't error/revert. Explicit
publishSummary = assertAnyRelayAccepted(publishResults, { // rejections still throw.
context: "relay preferences update", const publishSummary = assertAnyRelayAcceptedOrUnconfirmed(publishResults, {
message: "No relays accepted the update.", context: "relay preferences update",
}); message: "No relays accepted the update.",
} catch (publishError) { });
if (publishError?.relayFailures?.length) { if (publishSummary.unconfirmed) {
publishError.relayFailures.forEach( userLogger.warn(
({ url, error: relayError, reason }) => { "[RelayPreferencesManager] Relay list not acknowledged by any relay within the timeout; treating as optimistic success (reconciles on next load).",
userLogger.error( );
`[RelayPreferencesManager] Relay ${url} rejected relay list: ${reason}`,
relayError || reason
);
}
);
}
throw publishError;
} }
if (publishSummary.failed.length) { if (publishSummary.failed.length) {
+10 -2
View File
@@ -12,7 +12,7 @@ import {
} from "../nostrEventSchemas.js"; } from "../nostrEventSchemas.js";
import { import {
publishEventToRelays, publishEventToRelays,
assertAnyRelayAccepted, assertAnyRelayAcceptedOrUnconfirmed,
} from "../nostrPublish.js"; } from "../nostrPublish.js";
import { userLogger, devLogger } from "../utils/logger.js"; import { userLogger, devLogger } from "../utils/logger.js";
import { import {
@@ -1453,9 +1453,17 @@ class HashtagPreferencesService {
signedEvent, 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", 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) { if (publishSummary.failed?.length) {
publishSummary.failed.forEach(({ url, error }) => { publishSummary.failed.forEach(({ url, error }) => {
+11 -46
View File
@@ -21,7 +21,7 @@ import {
import { CACHE_POLICIES, STORAGE_TIERS } from "./nostr/cachePolicies.js"; import { CACHE_POLICIES, STORAGE_TIERS } from "./nostr/cachePolicies.js";
import { import {
publishEventToRelays, publishEventToRelays,
assertAnyRelayAccepted assertAnyRelayAcceptedOrUnconfirmed,
} from "./nostrPublish.js"; } from "./nostrPublish.js";
import { getApplication } from "./applicationContext.js"; import { getApplication } from "./applicationContext.js";
import { VideoListView } from "./ui/views/VideoListView.js"; import { VideoListView } from "./ui/views/VideoListView.js";
@@ -175,25 +175,6 @@ const subscribeVideoViewEventsApi = (pointer, options) =>
* Also handles fetching and rendering subscribed channels' videos * Also handles fetching and rendering subscribed channels' videos
* in the same card style as your home page. * 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 { class SubscriptionsManager {
constructor() { constructor() {
this.subscribedPubkeys = new Set(); this.subscribedPubkeys = new Set();
@@ -1471,35 +1452,19 @@ class SubscriptionsManager {
signedEvent signedEvent
); );
let publishSummary = null; // Only an explicit relay rejection (or a pre-publish signer/encryption error,
let softSuccess = false; // thrown earlier) is a real failure. A result where every relay merely timed
try { // out means the event was sent to an idempotent, replaceable list and almost
publishSummary = assertAnyRelayAccepted(publishResults, { // certainly persisted — treated as a soft success (unconfirmed) so the
context: "subscription list" // optimistic UI is NOT reverted and the user doesn't see a spurious error.
}); const publishSummary = assertAnyRelayAcceptedOrUnconfirmed(publishResults, {
} catch (publishError) { context: "subscription list",
if (publishError?.relayFailures?.length) { });
publishError.relayFailures.forEach( const softSuccess = publishSummary.unconfirmed === true;
({ url, error: relayError, reason }) => { if (softSuccess) {
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;
}
userLogger.warn( userLogger.warn(
"[SubscriptionsManager] Subscription list not acknowledged by any relay within the timeout; treating as optimistic success (reconciles on next load).", "[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) { if (publishSummary?.failed?.length) {
+12 -18
View File
@@ -24,7 +24,7 @@ import {
import { STANDARD_TIMEOUT_MS, MAX_BLOCKLIST_ENTRIES } from "./constants.js"; import { STANDARD_TIMEOUT_MS, MAX_BLOCKLIST_ENTRIES } from "./constants.js";
import { import {
publishEventToRelays, publishEventToRelays,
assertAnyRelayAccepted, assertAnyRelayAcceptedOrUnconfirmed,
} from "./nostrPublish.js"; } from "./nostrPublish.js";
import { profileCache } from "./state/profileCache.js"; import { profileCache } from "./state/profileCache.js";
import { DEFAULT_RELAY_URLS, capReadRelays } from "./nostr/toolkit.js"; import { DEFAULT_RELAY_URLS, capReadRelays } from "./nostr/toolkit.js";
@@ -2383,23 +2383,17 @@ class UserBlockListManager {
signedEvent signedEvent
); );
let publishSummary; // An all-timeout (unconfirmed) publish of this idempotent, replaceable mute
try { // list is treated as a soft success — the event was sent and almost always
publishSummary = assertAnyRelayAccepted(publishResults, { // persists (it shows up after a refresh), so don't surface a spurious error /
context: "mute list", // revert the optimistic state. Explicit rejections still throw.
}); const publishSummary = assertAnyRelayAcceptedOrUnconfirmed(publishResults, {
} catch (publishError) { context: "mute list",
if (publishError?.relayFailures?.length) { });
publishError.relayFailures.forEach( if (publishSummary.unconfirmed) {
({ url, error: relayError, reason }) => { userLogger.warn(
userLogger.error( "[UserBlockList] Mute list not acknowledged by any relay within the timeout; treating as optimistic success (reconciles on next load).",
`[UserBlockList] Mute list rejected by ${url}: ${reason}`, );
relayError || reason
);
}
);
}
throw publishError;
} }
if (publishSummary.failed.length) { if (publishSummary.failed.length) {
+39 -2
View File
@@ -27,8 +27,11 @@
import test from "node:test"; import test from "node:test";
import { strict as assert } from "node:assert"; import { strict as assert } from "node:assert";
import { RelayPublishError } from "../js/nostrPublish.js"; import {
import { subscriptionPublishFailuresAreAllTimeouts } from "../js/subscriptions.js"; RelayPublishError,
relayPublishFailuresAreAllTimeouts as subscriptionPublishFailuresAreAllTimeouts,
assertAnyRelayAcceptedOrUnconfirmed,
} from "../js/nostrPublish.js";
// Build a RelayPublishError the same way assertAnyRelayAccepted does — from the // Build a RelayPublishError the same way assertAnyRelayAccepted does — from the
// per-relay result objects (success:false + an Error whose message is the reason). // 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")]); const e = errFrom([rejected("wss://a", "Publish Timeout")]);
assert.equal(subscriptionPublishFailuresAreAllTimeouts(e), true); 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,
);
});