mirror of
https://github.com/PR0M3TH3AN/bitvid.git
synced 2026-10-27 07:52:24 +00:00
[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:
1 parent
a9271fd0e6
commit
47f2a9a865
7 files changed
+138
-105
No files matched your search
@@ -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) {
|
||||
|
||||
@@ -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) {
|
||||
|
||||
+12
-19
@@ -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) {
|
||||
|
||||
@@ -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 }) => {
|
||||
|
||||
+11
-46
@@ -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) {
|
||||
|
||||
+12
-18
@@ -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) {
|
||||
|
||||
@@ -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,
|
||||
);
|
||||
});
|
||||
Reference in new issue
Block a user