From 04524e17abe1231fee7d41428ac3845190da3dfb Mon Sep 17 00:00:00 2001 From: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 Date: Thu, 2 Jul 2026 12:23:56 -0400 Subject: [PATCH] fix(relay-reconnect): dismiss reconnect card on auto-reconnect without user click MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit connected state is now authoritative for recovery UI — a stale query error no longer pins the reconnect card while the relay is healthy. - useSidebarRelayConnectionCard: gate hasActiveRelayUnreachableError on !isRelayConnectionConnected so connected state clears the card even when channelsQuery.error has not yet been cleared - ConnectionBanner: add state !== connected guard to hasCollapsedRelayError for the same reason - RelayAutoHealScheduler: expand constructor parameter properties to explicit field declarations so node --experimental-strip-types can load the class in tests; initialize lastHealAt to -Infinity so the first recovery is never rate-limited - Add useRelayAutoHeal.test.mjs covering all scheduler state transitions Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- .../ui/useSidebarRelayConnectionCard.ts | 2 +- .../src/shared/api/useRelayAutoHeal.test.mjs | 167 ++++++++++++++++++ desktop/src/shared/api/useRelayAutoHeal.ts | 105 +++++++++-- desktop/src/shared/ui/ConnectionBanner.tsx | 1 + 4 files changed, 263 insertions(+), 12 deletions(-) create mode 100644 desktop/src/shared/api/useRelayAutoHeal.test.mjs diff --git a/desktop/src/features/sidebar/ui/useSidebarRelayConnectionCard.ts b/desktop/src/features/sidebar/ui/useSidebarRelayConnectionCard.ts index 27acca93b..3b20de321 100644 --- a/desktop/src/features/sidebar/ui/useSidebarRelayConnectionCard.ts +++ b/desktop/src/features/sidebar/ui/useSidebarRelayConnectionCard.ts @@ -82,7 +82,7 @@ export function useSidebarRelayConnectionCard( const [isWindowVisible, setIsWindowVisible] = React.useState(isDocumentVisible); const hasActiveRelayUnreachableError = - hasRelayUnreachableError && !hasSuccess; + hasRelayUnreachableError && !hasSuccess && !isRelayConnectionConnected; const isRelayConnectionActuallyDegraded = hasActiveRelayUnreachableError || isRelayConnectionStateDegraded; const isRelayConnectionSuccess = hasSuccess && isRelayConnectionConnected; diff --git a/desktop/src/shared/api/useRelayAutoHeal.test.mjs b/desktop/src/shared/api/useRelayAutoHeal.test.mjs new file mode 100644 index 000000000..8a1812b0d --- /dev/null +++ b/desktop/src/shared/api/useRelayAutoHeal.test.mjs @@ -0,0 +1,167 @@ +/** + * Unit tests for RelayAutoHealScheduler. + * + * Tests the rate-limited, deferred-heal state machine without React or DOM. + * Covers: first recovery fires immediately, rate-limit suppression schedules + * deferred heal, second recovery within window supersedes the deferred, heal + * fires after deferred window expires, dispose cancels pending heal. + */ + +import assert from "node:assert/strict"; +import test, { mock } from "node:test"; + +import { RelayAutoHealScheduler } from "./useRelayAutoHeal.ts"; + +function makeScheduler({ minIntervalMs = 100, now: initialNow = 0 } = {}) { + const heals = []; + const timers = []; + let timerSeq = 0; + let nowMs = initialNow; + + const sched = new RelayAutoHealScheduler( + () => heals.push(nowMs), + minIntervalMs, + mock.fn((fn, ms) => { + const id = ++timerSeq; + timers.push({ id, fn, ms }); + return id; + }), + mock.fn((id) => { + const idx = timers.findIndex((t) => t.id === id); + if (idx !== -1) timers.splice(idx, 1); + }), + () => nowMs, + ); + + return { + sched, + heals, + timers, + setNow: (ms) => { + nowMs = ms; + }, + fireTimer: (id) => timers.find((t) => t.id === id)?.fn(), + }; +} + +// ── First recovery — fires immediately ─────────────────────────────────────── + +test("first recovery fires onHeal immediately", () => { + const { sched, heals } = makeScheduler(); + + sched.onTransition("reconnecting", "connected"); + + assert.equal(heals.length, 1, "onHeal fired once"); +}); + +test("non-recovery transitions are ignored", () => { + const { sched, heals, timers } = makeScheduler(); + + sched.onTransition("connected", "reconnecting"); + sched.onTransition("reconnecting", "stalled"); + sched.onTransition("connected", "connected"); + + assert.equal(heals.length, 0, "no heals for non-recovery transitions"); + assert.equal(timers.length, 0, "no timers scheduled"); +}); + +// ── Rate limiting ───────────────────────────────────────────────────────────── + +test("second recovery within window is rate-limited and schedules deferred heal", () => { + const { sched, heals, timers, setNow } = makeScheduler({ + minIntervalMs: 100, + }); + + setNow(0); + sched.onTransition("reconnecting", "connected"); // fires immediately + assert.equal(heals.length, 1); + + // Second recovery 50ms later — within the 100ms window. + setNow(50); + sched.onTransition("stalled", "connected"); + + assert.equal(heals.length, 1, "onHeal not called again immediately"); + assert.equal(timers.length, 1, "deferred heal timer scheduled"); + assert.equal( + timers[0].ms, + 50, + "deferred fires after remaining window (100-50=50ms)", + ); +}); + +test("deferred heal fires onHeal when timer fires", () => { + const { sched, heals, timers, setNow, fireTimer } = makeScheduler({ + minIntervalMs: 100, + }); + + setNow(0); + sched.onTransition("reconnecting", "connected"); + + setNow(50); + sched.onTransition("stalled", "connected"); // rate-limited, timer id=1 + + setNow(100); + fireTimer(timers[0].id); + + assert.equal(heals.length, 2, "deferred onHeal fired after timer"); +}); + +test("second recovery supersedes deferred — old timer cancelled, new one scheduled", () => { + const { sched, heals, timers, setNow, fireTimer } = makeScheduler({ + minIntervalMs: 100, + }); + + setNow(0); + sched.onTransition("reconnecting", "connected"); // immediate heal + + setNow(40); + sched.onTransition("stalled", "connected"); // rate-limited → deferred at remaining=60ms + const firstTimerId = timers[0].id; + assert.equal(timers.length, 1); + + setNow(60); + sched.onTransition("reconnecting", "connected"); // supersedes — cancel first, schedule new + + assert.equal( + timers.findIndex((t) => t.id === firstTimerId), + -1, + "first timer cancelled", + ); + assert.equal(timers.length, 1, "new deferred timer scheduled"); + assert.equal( + timers[0].ms, + 40, + "new deferred fires after remaining window (100-60=40ms)", + ); + + setNow(100); + fireTimer(timers[0].id); + + assert.equal(heals.length, 2, "exactly two heals total (initial + deferred)"); +}); + +// ── Dispose ─────────────────────────────────────────────────────────────────── + +test("dispose cancels pending deferred heal", () => { + const { sched, heals, timers, setNow } = makeScheduler({ + minIntervalMs: 100, + }); + + setNow(0); + sched.onTransition("reconnecting", "connected"); + + setNow(50); + sched.onTransition("stalled", "connected"); // deferred scheduled + + sched.dispose(); + assert.equal(timers.length, 0, "timer cancelled on dispose"); + + // Firing the timer after dispose should be a no-op (timer is gone). + // The heals array should still be 1 (only the initial immediate heal). + assert.equal(heals.length, 1, "no additional heals after dispose"); +}); + +test("dispose with no pending timer is a no-op", () => { + const { sched } = makeScheduler(); + assert.doesNotThrow(() => sched.dispose()); +}); diff --git a/desktop/src/shared/api/useRelayAutoHeal.ts b/desktop/src/shared/api/useRelayAutoHeal.ts index 2f5e829f0..18fb1aaf5 100644 --- a/desktop/src/shared/api/useRelayAutoHeal.ts +++ b/desktop/src/shared/api/useRelayAutoHeal.ts @@ -2,12 +2,83 @@ import * as React from "react"; import { useQueryClient } from "@tanstack/react-query"; +import type { ConnectionState } from "@/shared/api/relayClientShared"; import { isRelayConnectionDegraded, useRelayConnection, } from "@/shared/api/useRelayConnection"; -const AUTO_HEAL_MIN_INTERVAL_MS = 15_000; +export const AUTO_HEAL_MIN_INTERVAL_MS = 15_000; + +/** + * Tracks degraded→connected transitions and fires `onHeal` at most once per + * `minIntervalMs`. When a transition is suppressed by the rate limiter, a + * deferred heal is scheduled for the remaining window so the *last* recovery + * always wins — stale query errors do not persist after reconnect. + * + * Injectable deps make this testable without React or DOM. + */ +export class RelayAutoHealScheduler { + private lastHealAt = -Infinity; + private deferredId: number | null = null; + private readonly onHeal: () => void; + private readonly minIntervalMs: number; + private readonly setTimeoutFn: (fn: () => void, ms: number) => number; + private readonly clearTimeoutFn: (id: number) => void; + private readonly nowFn: () => number; + + constructor( + onHeal: () => void, + minIntervalMs: number, + setTimeoutFn: (fn: () => void, ms: number) => number, + clearTimeoutFn: (id: number) => void, + nowFn: () => number = () => Date.now(), + ) { + this.onHeal = onHeal; + this.minIntervalMs = minIntervalMs; + this.setTimeoutFn = setTimeoutFn; + this.clearTimeoutFn = clearTimeoutFn; + this.nowFn = nowFn; + } + + /** + * Call on every connection-state change. Fires `onHeal` (immediately or + * deferred) when the transition is degraded→connected. + */ + onTransition(prev: ConnectionState, next: ConnectionState): void { + if (!(isRelayConnectionDegraded(prev) && next === "connected")) return; + + // A new recovery supersedes any pending deferred heal. + if (this.deferredId !== null) { + this.clearTimeoutFn(this.deferredId); + this.deferredId = null; + } + + const now = this.nowFn(); + const elapsed = now - this.lastHealAt; + + if (elapsed < this.minIntervalMs) { + // Rate-limited — schedule for when the window expires. + const remaining = this.minIntervalMs - elapsed; + this.deferredId = this.setTimeoutFn(() => { + this.deferredId = null; + this.lastHealAt = this.nowFn(); + this.onHeal(); + }, remaining); + } else { + this.lastHealAt = now; + this.onHeal(); + } + } + + /** Cancel any pending deferred heal (call on unmount). */ + dispose(): void { + if (this.deferredId !== null) { + this.clearTimeoutFn(this.deferredId); + this.deferredId = null; + } + } +} /** * Auto-heal: when the connection recovers from a degraded state, invalidate @@ -17,23 +88,35 @@ const AUTO_HEAL_MIN_INTERVAL_MS = 15_000; * Rate-limited to prevent a flappy connection (e.g. VPN toggling) from * firing an unfiltered invalidation — ~20-40 requests across active queries * with retry:1 — every time the relay briefly recovers. + * + * When a recovery is suppressed by the rate limiter (an earlier flap consumed + * the budget), a deferred heal is scheduled for the remaining window so the + * *last* recovery always wins and stale query errors do not persist. */ export function useRelayAutoHeal(): void { const queryClient = useQueryClient(); const connectionState = useRelayConnection(); const prevConnectionStateRef = React.useRef(connectionState); - const lastAutoHealAtRef = React.useRef(0); + const schedulerRef = React.useRef(null); + + if (schedulerRef.current === null) { + schedulerRef.current = new RelayAutoHealScheduler( + () => void queryClient.invalidateQueries(), + AUTO_HEAL_MIN_INTERVAL_MS, + window.setTimeout.bind(window), + window.clearTimeout.bind(window), + ); + } + + React.useEffect(() => { + return () => { + schedulerRef.current?.dispose(); + }; + }, []); React.useEffect(() => { const prev = prevConnectionStateRef.current; prevConnectionStateRef.current = connectionState; - if (isRelayConnectionDegraded(prev) && connectionState === "connected") { - const now = Date.now(); - if (now - lastAutoHealAtRef.current < AUTO_HEAL_MIN_INTERVAL_MS) { - return; - } - lastAutoHealAtRef.current = now; - void queryClient.invalidateQueries(); - } - }, [connectionState, queryClient]); + schedulerRef.current?.onTransition(prev, connectionState); + }, [connectionState]); } diff --git a/desktop/src/shared/ui/ConnectionBanner.tsx b/desktop/src/shared/ui/ConnectionBanner.tsx index 900a843eb..c461f5a00 100644 --- a/desktop/src/shared/ui/ConnectionBanner.tsx +++ b/desktop/src/shared/ui/ConnectionBanner.tsx @@ -33,6 +33,7 @@ export function ConnectionBanner({ errorMessage }: ConnectionBannerProps) { const { state: sidebarState } = useSidebar(); const hasCollapsedRelayError = sidebarState === "collapsed" && + state !== "connected" && Boolean(errorMessage && isRelayUnreachableError(errorMessage)); if (!isRelayConnectionDegraded(state) && !hasCollapsedRelayError) {