mirror of
https://github.com/PR0M3TH3AN/bitvid.git
synced 2026-10-29 00:42:30 +00:00
fix(nostr): circuit-break the dead nip-07 signer channel
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 <[email protected]>
This commit is contained in:
1 parent
71cd04b215
commit
ab57f8d596
4 files changed
+330
-10
No files matched your search
+22
-2
@@ -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).
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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({
|
||||
|
||||
@@ -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",
|
||||
);
|
||||
});
|
||||
Reference in new issue
Block a user