From ff1afa117594920798c1b970fa31497caf80ab17 Mon Sep 17 00:00:00 2001 From: thePR0M3TH3AN <53631862+PR0M3TH3AN@users.noreply.github.com> Date: Mon, 15 Jun 2026 09:11:40 -0400 Subject: [PATCH] fix(nostr): count 'message channel closed' toward the circuit breaker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Real-env console (2026-06-15) showed the breaker never opening even as decrypts failed for minutes. Cause: the breaker treated every non-timeout error as "channel alive" and RESET the timeout streak — but "message channel closed before a response was received" is the exact dead-channel signal, and it rejects as a (fast) non-timeout error. So each such rejection reset the counter and the circuit could never open. Classify channel-death messages (message channel closed / could not establish connection / receiving end does not exist / extension context invalidated / connection lost) as channel failures that count TOWARD opening the circuit, instead of as responsive failures that reset it. Genuine extension responses (user rejection, permission denied, etc.) still reset the streak. Test: extends nip07-channel-breaker with a synchronous channel-closed op asserting consecutive channel-death errors open the circuit and then fast-fail (no waits). Co-Authored-By: Claude Opus 4.8 --- js/nostr/nip07Permissions.js | 38 ++++++++++++++++++---- tests/nostr/nip07-channel-breaker.test.mjs | 31 ++++++++++++++++++ 2 files changed, 63 insertions(+), 6 deletions(-) diff --git a/js/nostr/nip07Permissions.js b/js/nostr/nip07Permissions.js index 8def718d..b96118c3 100644 --- a/js/nostr/nip07Permissions.js +++ b/js/nostr/nip07Permissions.js @@ -222,9 +222,29 @@ function recordChannelTimeout() { } } -// 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. +// Some non-timeout failures are actually dead-channel signals, NOT "the +// extension answered with an error." When the content-script message port is +// severed the call rejects synchronously with one of these messages — which is +// the very condition the breaker exists for, so they must count TOWARD opening +// the circuit (not reset it). Matching the real-env "message channel closed" +// case (KNOWN_BUGS #0) is the whole point. +const CHANNEL_DEATH_PATTERNS = [ + "message channel closed", + "could not establish connection", + "receiving end does not exist", + "extension context invalidated", + "connection lost", +]; +function isChannelDeathError(error) { + const message = + error && typeof error.message === "string" ? error.message.toLowerCase() : ""; + if (!message) return false; + return CHANNEL_DEATH_PATTERNS.some((pattern) => message.includes(pattern)); +} + +// A non-timeout failure where the extension genuinely RESPONDED (e.g. user +// rejection, "permission denied", unsupported method) means the channel is +// alive — clear the timeout streak so we don't open on unrelated errors. function recordChannelResponsiveFailure() { channelBreaker.consecutiveTimeouts = 0; } @@ -453,8 +473,14 @@ export async function runNip07WithRetry( error.message === NIP07_LOGIN_TIMEOUT_ERROR_MESSAGE; if (!isTimeoutError) { - // The extension responded (with an error) — the channel is alive. - onResponsiveFailure(); + // A severed message port rejects with a channel-death message — that IS + // the dead-channel condition, so count it toward opening the circuit. + // Any other error means the extension actually responded (channel alive). + if (isChannelDeathError(error)) { + onTimeout(); + } else { + onResponsiveFailure(); + } throw error; } @@ -485,7 +511,7 @@ export async function runNip07WithRetry( const retryTimedOut = retryError instanceof Error && retryError.message === NIP07_LOGIN_TIMEOUT_ERROR_MESSAGE; - if (retryTimedOut) { + if (retryTimedOut || isChannelDeathError(retryError)) { onTimeout(); } else { onResponsiveFailure(); diff --git a/tests/nostr/nip07-channel-breaker.test.mjs b/tests/nostr/nip07-channel-breaker.test.mjs index 678b009c..76a5f637 100644 --- a/tests/nostr/nip07-channel-breaker.test.mjs +++ b/tests/nostr/nip07-channel-breaker.test.mjs @@ -32,6 +32,13 @@ const successOp = async () => "ok"; const respondingErrorOp = async () => { throw new Error("user rejected"); }; +// A severed content-script port: rejects synchronously with a channel-death +// message. This is the real-env dead-channel signal (KNOWN_BUGS #0). +const channelClosedOp = async () => { + throw new Error( + "A listener indicated an asynchronous response by returning true, but the message channel closed before a response was received", + ); +}; // Small timeout so "hangs to full timeout" is fast in the test; no retry // extension so each attempt is exactly one timeout window. @@ -94,6 +101,30 @@ test("a successful probe closes the circuit (recovery without refresh)", async ( ); }); +test("'message channel closed' errors open the circuit (fast, no 15s waits)", async () => { + resetNip07ChannelBreaker(); + + // A severed port rejects FAST (no timeout wait). These must count toward + // opening the circuit — otherwise the real-env dead channel never trips it. + for (let i = 0; i < CIRCUIT_TIMEOUT_THRESHOLD; i++) { + await swallow(runNip07WithRetry(channelClosedOp, FAST)); + } + assert.equal( + getNip07ChannelBreakerState().open, + true, + "consecutive 'message channel closed' errors must open the circuit", + ); + + // And once open, further calls fast-fail rather than re-hitting the dead port. + await swallow(runNip07WithRetry(channelClosedOp, FAST)); // consume probe slot + const fastErr = await swallow(runNip07WithRetry(channelClosedOp, FAST)); + assert.equal( + fastErr.code, + "nip07-channel-unresponsive", + "calls after a channel-death streak must fast-fail", + ); +}); + test("extension errors (responsive failures) do not open the circuit", async () => { resetNip07ChannelBreaker();