From ab57f8d596ccc137ab0bf8bdf5d8c04cec2fb67d Mon Sep 17 00:00:00 2001 From: thePR0M3TH3AN <53631862+PR0M3TH3AN@users.noreply.github.com> Date: Mon, 15 Jun 2026 08:49:05 -0400 Subject: [PATCH] fix(nostr): circuit-break the dead nip-07 signer channel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Real-env console (2026-06-15) showed that once the extension's message port drops under the post-login burst ("message channel closed"), EVERY subsequent decrypt call hangs to its full ~15s timeout. Each list service (blocks, hashtags, subscriptions, watch history) independently retries on that timeout, so the app sits in a permanent loop of 15s-blocking extension calls — pinning the CPU ("fans at max") and never recovering until a manual page refresh. Add a shared circuit breaker at the runNip07WithRetry chokepoint: - After CIRCUIT_TIMEOUT_THRESHOLD (3) consecutive timeouts the circuit OPENS; further calls fail fast with code "nip07-channel-unresponsive" (no multi-second hang), so the retry loops become cheap and the CPU settles. - While open, a single probe per CIRCUIT_PROBE_INTERVAL_MS is allowed through; one success CLOSES the circuit and normal decryption resumes — no page refresh required. - Responsive failures (extension answered with an error) reset the timeout streak — only genuine channel hangs open the circuit. - Interactive permission prompts bypass the breaker entirely: a user taking their time at the extension UI can neither trip it nor be blocked by it, and such a prompt can itself heal the channel. Increment 2 of the "cut the login burst, then add a safety net" work for KNOWN_BUGS #0. Test: tests/nostr/nip07-channel-breaker.test.mjs (cheat-resistant — drives the real queue with hanging/responsive/success ops; asserts open-after-threshold, fast-fail, probe-recovery, responsive errors don't open, and bypass is never fast-failed). Full unit suite green. Co-Authored-By: Claude Opus 4.8 --- docs/KNOWN_BUGS.md | 24 ++- js/nostr/nip07Permissions.js | 172 ++++++++++++++++++++- tests/app/login-defers-dms.test.mjs | 14 +- tests/nostr/nip07-channel-breaker.test.mjs | 130 ++++++++++++++++ 4 files changed, 330 insertions(+), 10 deletions(-) create mode 100644 tests/nostr/nip07-channel-breaker.test.mjs diff --git a/docs/KNOWN_BUGS.md b/docs/KNOWN_BUGS.md index a070ab0f..50cbf126 100644 --- a/docs/KNOWN_BUGS.md +++ b/docs/KNOWN_BUGS.md @@ -42,8 +42,28 @@ refactor work in that doc — these are defects. harnesses stay isolated. Regression coverage: `tests/relay-subscribe-cap.test.mjs` (bounded + reserved-defaults + writes-uncapped), and the spec-corrected `user-blocks`, `subscriptions-manager`, and `nostr/client` fetch tests. -- **Still deferred:** liveness-ranked health (see #4) and DM decrypt resilience - (don't hard-fail DMs on a transient permission miss — retry). +- **Real-env follow-up (2026-06-15 console):** the relay cap WORKED (every sub + now fans out to exactly 8 relays) but exposed the true blocker: under the + post-login burst the nip-07 extension's message port drops ("message channel + closed"), after which EVERY decrypt hangs to its ~15s timeout, each list + service retries forever, and the app never recovers without a page refresh + (CPU pinned — "fans at max"). The signer-readiness gate also took ~67s. + Two-part fix in progress ("cut the burst, then add a safety net"): + - **Burst reduction (SHIPPED, increment 1):** stop eagerly loading DM history + at login (`authSessionCoordinator` `dmStatePromise` now defers; DMs load + lazily when the Messages tab opens). Removes a 50-message decrypt burst that + competed for the channel. Test: `tests/app/login-defers-dms.test.mjs`. + - **Channel circuit breaker (SHIPPED, increment 2):** `js/nostr/nip07Permissions.js` + now opens a circuit after `CIRCUIT_TIMEOUT_THRESHOLD` consecutive call + timeouts — subsequent calls fail FAST (`code: "nip07-channel-unresponsive"`) + instead of each hanging ~15s, so the per-list retry loops stop pinning the + CPU. A single periodic probe detects recovery; one success closes the + circuit (no refresh). Responsive errors don't open it; interactive permission + prompts bypass it. Test: `tests/nostr/nip07-channel-breaker.test.mjs`. +- **Still open:** further burst reduction (collapse each list's multi-variant + fan-out into one multi-filter REQ per relay; investigate the ~67s signer gate); + liveness-ranked relay health (see #4); DM decrypt resilience (don't hard-fail + DMs on a transient permission miss — retry). ### 1. Logged-out: video grid stays empty until a manual refresh - **Severity:** medium (first-load UX, logged-out). diff --git a/js/nostr/nip07Permissions.js b/js/nostr/nip07Permissions.js index ccc6d1d1..8def718d 100644 --- a/js/nostr/nip07Permissions.js +++ b/js/nostr/nip07Permissions.js @@ -140,6 +140,95 @@ class Nip07RequestQueue { // manifests as "message channel closed" and repeated decrypt timeouts. const requestQueue = new Nip07RequestQueue(2); +export const NIP07_CHANNEL_UNRESPONSIVE_ERROR_MESSAGE = + "The NIP-07 signer channel is unresponsive. It usually recovers shortly; if it persists, refresh the page or re-open your extension."; + +// Circuit breaker for a dead/unresponsive nip-07 message channel. +// +// Real-env failure mode (KNOWN_BUGS #0): under the post-login burst the +// extension's single content-script message port drops ("message channel +// closed"). After that EVERY decrypt call hangs to its full timeout (~15s). +// Each list service (blocks, hashtags, subscriptions, watch history) +// independently retries on that timeout, so the app sits in a permanent loop of +// 15s-blocking extension calls — pinning the CPU ("fans at max") and never +// recovering until a manual page refresh. +// +// The breaker converts that into: a few timeouts OPEN the circuit, after which +// calls fail FAST (no 15s wait) so the retry loops become cheap and the CPU +// settles. While open, a single periodic PROBE is allowed through to detect +// recovery; one success CLOSES the circuit and normal decryption resumes — no +// refresh required. Interactive permission prompts bypass the breaker so a user +// taking their time at the extension UI can never trip or be blocked by it. +const CIRCUIT_TIMEOUT_THRESHOLD = 3; // consecutive timeouts before opening +const CIRCUIT_OPEN_MS = 30_000; // how long the circuit stays open +const CIRCUIT_PROBE_INTERVAL_MS = 8_000; // min spacing between recovery probes +const channelBreaker = { + consecutiveTimeouts: 0, + openUntil: 0, + lastProbeAt: 0, +}; + +export function getNip07ChannelBreakerState() { + const now = Date.now(); + return { + open: channelBreaker.openUntil > now, + consecutiveTimeouts: channelBreaker.consecutiveTimeouts, + openUntil: channelBreaker.openUntil, + }; +} + +export function resetNip07ChannelBreaker() { + channelBreaker.consecutiveTimeouts = 0; + channelBreaker.openUntil = 0; + channelBreaker.lastProbeAt = 0; +} + +function isCircuitOpen(now) { + return channelBreaker.openUntil > now; +} + +// While open, allow exactly one probe per CIRCUIT_PROBE_INTERVAL_MS so we can +// detect recovery without re-flooding the channel. +function claimProbeSlot(now) { + if (now - channelBreaker.lastProbeAt >= CIRCUIT_PROBE_INTERVAL_MS) { + channelBreaker.lastProbeAt = now; + return true; + } + return false; +} + +function recordChannelSuccess() { + const wasOpen = channelBreaker.openUntil > Date.now(); + channelBreaker.consecutiveTimeouts = 0; + channelBreaker.openUntil = 0; + if (wasOpen) { + userLogger.info( + "[nostr] NIP-07 signer channel recovered; resuming normal requests.", + ); + } +} + +function recordChannelTimeout() { + channelBreaker.consecutiveTimeouts += 1; + if ( + channelBreaker.consecutiveTimeouts >= CIRCUIT_TIMEOUT_THRESHOLD && + channelBreaker.openUntil <= Date.now() + ) { + channelBreaker.openUntil = Date.now() + CIRCUIT_OPEN_MS; + userLogger.warn( + `[nostr] NIP-07 signer channel unresponsive after ${channelBreaker.consecutiveTimeouts} timeouts; ` + + `failing fast for ${CIRCUIT_OPEN_MS / 1000}s and probing for recovery.`, + ); + } +} + +// Non-timeout failures (e.g. user rejection, explicit extension errors) mean the +// channel IS alive and responding — clear the timeout streak so we don't open on +// unrelated errors. +function recordChannelResponsiveFailure() { + channelBreaker.consecutiveTimeouts = 0; +} + export function getEnableVariantTimeoutMs() { const overrideValue = typeof globalThis !== "undefined" && @@ -301,27 +390,78 @@ export async function runNip07WithRetry( timeoutMs = NIP07_LOGIN_TIMEOUT_MS, retryMultiplier = 2, priority = NIP07_PRIORITY.NORMAL, + // Interactive permission prompts bypass the circuit breaker: a user taking + // their time at the extension UI is not a dead channel, and the prompt must + // always be allowed through (and can itself heal the channel). + bypassCircuitBreaker = false, } = {}, ) { // We wrap the entire retry logic in a queue task to ensure only one // request hits the extension at a time (per slot). const executeTask = async () => { + // Circuit breaker: when the channel has been declared unresponsive, fail + // fast (no multi-second hang) for everything except a single periodic probe + // — this stops the per-list retry loops from pinning the CPU on a dead + // channel, while still detecting recovery without a page refresh. + let isProbe = false; + if (!bypassCircuitBreaker) { + const now = Date.now(); + if (isCircuitOpen(now)) { + if (claimProbeSlot(now)) { + isProbe = true; + } else { + const fastFail = new Error( + NIP07_CHANNEL_UNRESPONSIVE_ERROR_MESSAGE, + ); + fastFail.code = "nip07-channel-unresponsive"; + throw fastFail; + } + } + } + // We wrap the operation invocation to ensure it returns a fresh promise each time // we attempt it. This is critical for retries: if the first attempt stalls (dropped // by extension), re-awaiting the same promise would just keep waiting on the dead request. const invokeOperation = () => Promise.resolve(operation()); + const onResult = (result) => { + if (!bypassCircuitBreaker) { + recordChannelSuccess(); + } + return result; + }; + const onTimeout = () => { + if (!bypassCircuitBreaker) { + recordChannelTimeout(); + } + }; + const onResponsiveFailure = () => { + if (!bypassCircuitBreaker) { + recordChannelResponsiveFailure(); + } + }; + try { - return await withNip07Timeout(invokeOperation, { + const result = await withNip07Timeout(invokeOperation, { timeoutMs, message: NIP07_LOGIN_TIMEOUT_ERROR_MESSAGE, }); + return onResult(result); } catch (error) { const isTimeoutError = error instanceof Error && error.message === NIP07_LOGIN_TIMEOUT_ERROR_MESSAGE; - if (!isTimeoutError || retryMultiplier <= 1) { + if (!isTimeoutError) { + // The extension responded (with an error) — the channel is alive. + onResponsiveFailure(); + throw error; + } + + // A probe that times out leaves the circuit open; don't waste a longer + // retry on a channel we already believe is dead. + if (retryMultiplier <= 1 || isProbe) { + onTimeout(); throw error; } @@ -335,10 +475,23 @@ export async function runNip07WithRetry( ); // On retry, we invoke operation() again to send a fresh request to the extension. - return withNip07Timeout(invokeOperation, { - timeoutMs: extendedTimeout, - message: NIP07_LOGIN_TIMEOUT_ERROR_MESSAGE, - }); + try { + const result = await withNip07Timeout(invokeOperation, { + timeoutMs: extendedTimeout, + message: NIP07_LOGIN_TIMEOUT_ERROR_MESSAGE, + }); + return onResult(result); + } catch (retryError) { + const retryTimedOut = + retryError instanceof Error && + retryError.message === NIP07_LOGIN_TIMEOUT_ERROR_MESSAGE; + if (retryTimedOut) { + onTimeout(); + } else { + onResponsiveFailure(); + } + throw retryError; + } } }; @@ -409,6 +562,8 @@ export async function requestEnablePermissions( { label: `extension.${methodName}`, ...variantTimeoutOverrides, + // Interactive grant — never gated by (and can heal) the breaker. + bypassCircuitBreaker: true, }, ); return true; @@ -463,6 +618,11 @@ export const __testExports = { writeStoredNip07Permissions, clearStoredNip07Permissions, normalizePermissionMethod, + getNip07ChannelBreakerState, + resetNip07ChannelBreaker, + CIRCUIT_TIMEOUT_THRESHOLD, + CIRCUIT_OPEN_MS, + CIRCUIT_PROBE_INTERVAL_MS, }; export function waitForNip07Extension(timeoutMs = NIP07_EXTENSION_WAIT_TIMEOUT_MS) { diff --git a/tests/app/login-defers-dms.test.mjs b/tests/app/login-defers-dms.test.mjs index 95f3b649..6f37003d 100644 --- a/tests/app/login-defers-dms.test.mjs +++ b/tests/app/login-defers-dms.test.mjs @@ -12,10 +12,14 @@ import { test } from "node:test"; import assert from "node:assert/strict"; -// Minimal browser shim: handleAuthLogin reads window.location.search. +// Minimal browser shim: handleAuthLogin reads window.location.search. The unit +// suite shares one process, so a sibling test may have already installed a +// window (possibly without a usable location, or carrying a "?v=" param that +// routes login differently) — normalize it for this scenario either way. if (typeof globalThis.window === "undefined") { - globalThis.window = { location: { search: "", href: "http://localhost/" } }; + globalThis.window = {}; } +globalThis.window.location = { search: "", href: "http://localhost/" }; const { createAuthSessionCoordinator } = await import( "../../js/app/authSessionCoordinator.js" @@ -104,6 +108,12 @@ function makeCoordinator({ loadDirectMessages, refreshUnreadDmIndicator }) { } test("login does not eagerly load DM history; it is deferred", async () => { + // Re-normalize at call time in case a sibling test mutated window/location. + if (typeof globalThis.window === "undefined") { + globalThis.window = {}; + } + globalThis.window.location = { search: "", href: "http://localhost/" }; + let loadDmCalls = 0; let unreadIndicatorCalls = 0; const coord = makeCoordinator({ diff --git a/tests/nostr/nip07-channel-breaker.test.mjs b/tests/nostr/nip07-channel-breaker.test.mjs new file mode 100644 index 00000000..678b009c --- /dev/null +++ b/tests/nostr/nip07-channel-breaker.test.mjs @@ -0,0 +1,130 @@ +// Scenario (SCN-nip07-channel-breaker): +// Given the NIP-07 signer channel that lists decrypt through, +// When it stops responding (consecutive call timeouts, as happens when the +// extension's message port drops under the post-login burst — KNOWN_BUGS #0), +// Then after a threshold of consecutive timeouts the circuit OPENS and further +// calls fail FAST (code "nip07-channel-unresponsive") instead of each hanging +// to its full timeout — so the per-list retry loops stop pinning the CPU. +// And while open a single periodic PROBE is allowed through to detect recovery; +// one successful call CLOSES the circuit (no page refresh required). +// And a call that ERRORS (extension responded) does NOT count toward opening. +// And interactive permission prompts (bypassCircuitBreaker) are never fast-failed. + +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { + NIP07_LOGIN_TIMEOUT_ERROR_MESSAGE, + __testExports, +} from "../../js/nostr/nip07Permissions.js"; + +const { + runNip07WithRetry, + resetNip07ChannelBreaker, + getNip07ChannelBreakerState, + CIRCUIT_TIMEOUT_THRESHOLD, +} = __testExports; + +// A NIP-07 call that never responds -> withNip07Timeout fires the timeout. +const hangingOp = () => new Promise(() => {}); +// A NIP-07 call the extension answers successfully (channel alive). +const successOp = async () => "ok"; +// A NIP-07 call the extension answers with an error (channel alive, but refused). +const respondingErrorOp = async () => { + throw new Error("user rejected"); +}; + +// Small timeout so "hangs to full timeout" is fast in the test; no retry +// extension so each attempt is exactly one timeout window. +const FAST = { timeoutMs: 20, retryMultiplier: 1 }; + +const swallow = (p) => p.then(() => null).catch((e) => e); + +test("circuit opens after consecutive timeouts and then fails fast", async () => { + resetNip07ChannelBreaker(); + + for (let i = 0; i < CIRCUIT_TIMEOUT_THRESHOLD; i++) { + await swallow(runNip07WithRetry(hangingOp, FAST)); + } + assert.equal( + getNip07ChannelBreakerState().open, + true, + "circuit should be open after the timeout threshold", + ); + + // First call after opening is the recovery PROBE (still runs -> times out). + const probeErr = await swallow(runNip07WithRetry(hangingOp, FAST)); + assert.equal( + probeErr.message, + NIP07_LOGIN_TIMEOUT_ERROR_MESSAGE, + "the probe should actually run (and time out here), not fast-fail", + ); + + // The next call within the probe interval must FAIL FAST. + const start = Date.now(); + const fastErr = await swallow(runNip07WithRetry(hangingOp, FAST)); + const elapsed = Date.now() - start; + assert.equal( + fastErr.code, + "nip07-channel-unresponsive", + "calls while the circuit is open (outside the probe slot) must fast-fail", + ); + assert.ok( + elapsed < FAST.timeoutMs, + `fast-fail must not wait for the timeout window (got ${elapsed}ms)`, + ); +}); + +test("a successful probe closes the circuit (recovery without refresh)", async () => { + resetNip07ChannelBreaker(); + + for (let i = 0; i < CIRCUIT_TIMEOUT_THRESHOLD; i++) { + await swallow(runNip07WithRetry(hangingOp, FAST)); + } + assert.equal(getNip07ChannelBreakerState().open, true, "precondition: open"); + + // First post-open call is the probe; let it succeed -> channel recovered. + const result = await runNip07WithRetry(successOp, FAST); + assert.equal(result, "ok"); + const state = getNip07ChannelBreakerState(); + assert.equal(state.open, false, "a successful probe must close the circuit"); + assert.equal( + state.consecutiveTimeouts, + 0, + "the timeout streak must reset on success", + ); +}); + +test("extension errors (responsive failures) do not open the circuit", async () => { + resetNip07ChannelBreaker(); + + // Far more than the threshold, but the extension is RESPONDING (with errors), + // so the channel is alive and the breaker must stay closed. + for (let i = 0; i < CIRCUIT_TIMEOUT_THRESHOLD + 2; i++) { + await swallow(runNip07WithRetry(respondingErrorOp, FAST)); + } + const state = getNip07ChannelBreakerState(); + assert.equal(state.open, false, "responsive errors must not open the circuit"); + assert.equal(state.consecutiveTimeouts, 0, "no timeout streak should accrue"); +}); + +test("interactive (bypass) calls are never fast-failed while open", async () => { + resetNip07ChannelBreaker(); + + for (let i = 0; i < CIRCUIT_TIMEOUT_THRESHOLD; i++) { + await swallow(runNip07WithRetry(hangingOp, FAST)); + } + // Consume the probe slot so a NON-bypass call would now fast-fail. + await swallow(runNip07WithRetry(hangingOp, FAST)); + assert.equal(getNip07ChannelBreakerState().open, true, "precondition: open"); + + // A bypass call (interactive permission prompt) must still RUN — it can't be + // blocked by the breaker and can itself heal the channel. + const err = await swallow( + runNip07WithRetry(hangingOp, { ...FAST, bypassCircuitBreaker: true }), + ); + assert.equal( + err.message, + NIP07_LOGIN_TIMEOUT_ERROR_MESSAGE, + "bypass call should run to its timeout, not be fast-failed by the breaker", + ); +});