From 54194d5d06ddf9982ec8194f332f49c1aa265f80 Mon Sep 17 00:00:00 2001 From: npub1x4hk035p3p9q39a3fcrd2fe30lpkrhr5dwe0cqzzjphxyyh8m0gsq4vqap <356f67c681884a0897b14e06d527317fc361dc746bb2fc0042906e6212e7dbd1@sprout-oss.stage.blox.sqprod.co> Date: Fri, 24 Jul 2026 13:28:38 -0700 Subject: [PATCH] fix(relay): preserve reconnect backoff Co-authored-by: npub1x4hk035p3p9q39a3fcrd2fe30lpkrhr5dwe0cqzzjphxyyh8m0gsq4vqap <356f67c681884a0897b14e06d527317fc361dc746bb2fc0042906e6212e7dbd1@sprout-oss.stage.blox.sqprod.co> Signed-off-by: npub1x4hk035p3p9q39a3fcrd2fe30lpkrhr5dwe0cqzzjphxyyh8m0gsq4vqap <356f67c681884a0897b14e06d527317fc361dc746bb2fc0042906e6212e7dbd1@sprout-oss.stage.blox.sqprod.co> --- .../api/relayReconnectController.test.mjs | 106 ++++++------------ .../shared/api/relayReconnectController.ts | 58 +++------- desktop/src/shared/api/useReconnectRelay.ts | 2 - 3 files changed, 50 insertions(+), 116 deletions(-) diff --git a/desktop/src/shared/api/relayReconnectController.test.mjs b/desktop/src/shared/api/relayReconnectController.test.mjs index 0a3a1c9355..3a3890f5a5 100644 --- a/desktop/src/shared/api/relayReconnectController.test.mjs +++ b/desktop/src/shared/api/relayReconnectController.test.mjs @@ -7,7 +7,10 @@ */ import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; +import path from "node:path"; import test, { mock } from "node:test"; +import { fileURLToPath } from "node:url"; import { DEFAULT_RECONNECT_TIMING_POLICY, @@ -24,9 +27,7 @@ function makeDeps({ connectionStateListener = null, } = {}) { const timers = []; - const intervals = []; let timerSeq = 0; - let intervalSeq = 0; const deps = { preconnect: mock.fn(preconnectResult), @@ -47,38 +48,44 @@ function makeDeps({ const idx = timers.findIndex((t) => t.id === id); if (idx !== -1) timers.splice(idx, 1); }), - setInterval: mock.fn((fn, ms) => { - const id = ++intervalSeq; - intervals.push({ id, fn, ms }); - return id; - }), - clearInterval: mock.fn((id) => { - const idx = intervals.findIndex((t) => t.id === id); - if (idx !== -1) intervals.splice(idx, 1); - }), - // Test helpers to fire timers/intervals. + // Test helper to fire timers. _fireTimer: (id) => timers.find((t) => t.id === id)?.fn(), - _fireInterval: (id) => intervals.find((t) => t.id === id)?.fn(), _timers: timers, - _intervals: intervals, }; return deps; } // ── Timing policy ───────────────────────────────────────────────────────────── +const testDir = path.dirname(fileURLToPath(import.meta.url)); + +test("fast path outlives the native websocket connection timeout", () => { + const rustSource = readFileSync( + path.resolve(testDir, "../../../src-tauri/src/native_websocket.rs"), + "utf8", + ); + const match = rustSource.match( + /const CONNECT_TIMEOUT: Duration = Duration::from_secs\((\d+)\);/, + ); + assert.ok(match, "native websocket connect timeout is declared in seconds"); + + const nativeConnectTimeoutMs = Number(match[1]) * 1_000; + assert.ok( + DEFAULT_RECONNECT_TIMING_POLICY.fastPathTimeoutMs > nativeConnectTimeoutMs, + "fast path must wait for the native websocket attempt to settle", + ); +}); + test("default timing policy preserves current reconnect timings", () => { assert.deepEqual(DEFAULT_RECONNECT_TIMING_POLICY, { - fastPathTimeoutMs: 4_000, - pollIntervalMs: 3_000, + fastPathTimeoutMs: 11_000, backstopMs: 120_000, }); }); -test("injected timing policy drives fast-path, poll, and backstop timers", async () => { +test("injected timing policy drives fast-path and backstop timers", async () => { const ctrl = new RelayReconnectController({ fastPathTimeoutMs: 11, - pollIntervalMs: 22, backstopMs: 33, }); const deps = makeDeps({ @@ -91,7 +98,6 @@ test("injected timing policy drives fast-path, poll, and backstop timers", async await ctrl.start(deps); assert.equal(deps.setTimeout.mock.calls[0].arguments[1], 11); - assert.equal(deps.setInterval.mock.calls[0].arguments[1], 22); assert.equal(deps.setTimeout.mock.calls[1].arguments[1], 33); }); @@ -113,13 +119,12 @@ test("fast-path success — hook never invoked, onSuccess fires, state resets", }); }); -test("fast-path success — no poll interval or backstop scheduled", async () => { +test("fast-path success — no backstop scheduled", async () => { const ctrl = new RelayReconnectController(); const deps = makeDeps({ preconnectResult: async () => {} }); await ctrl.start(deps); - assert.equal(deps.setInterval.mock.calls.length, 0, "no poll interval"); // The fast-path withDeadline schedules one timeout for the deadline, but // that is cleared by finally(). After success no backstop should remain. assert.equal( @@ -136,7 +141,7 @@ test("escalation fires only when fast path fails and hook is configured", async let callIdx = 0; const deps = makeDeps({ preconnectResult: async () => { - // First call (fast path) fails; subsequent calls (poll) succeed. + // First call (fast path) fails. if (++callIdx === 1) throw new Error("relay unreachable"); }, hookConfiguredResult: async () => true, @@ -174,7 +179,7 @@ test("escalation skipped when hook not configured", async () => { ); }); -test("hook failure is non-fatal — polling still starts", async () => { +test("hook failure is non-fatal — background wait still starts", async () => { const ctrl = new RelayReconnectController(); const deps = makeDeps({ preconnectResult: async () => { @@ -195,48 +200,12 @@ test("hook failure is non-fatal — polling still starts", async () => { } assert.equal(threw, false, "hook failure does not propagate"); - assert.ok( - deps._intervals.length > 0 || deps._timers.length > 0, - "phase 3 timers scheduled", - ); + assert.ok(deps._timers.length > 0, "phase 3 backstop scheduled"); }); -// ── Phase 3: poll-until-connected ───────────────────────────────────────────── +// ── Phase 3: wait for background reconnect ─────────────────────────────────── -test("poll preempts backstop — onSuccess fires when poll wins", async () => { - const ctrl = new RelayReconnectController(); - let callIdx = 0; - const deps = makeDeps({ - preconnectResult: async () => { - if (++callIdx <= 2) throw new Error("not yet"); - // Third call succeeds — simulates poll winning. - }, - hookConfiguredResult: async () => false, - }); - - // Phase 1 fails, enters phase 3. start() returns false. - const promise = ctrl.start(deps); - await promise; - - // Fire the poll interval twice to reach the succeeding call. - const intervalId = deps._intervals[0]?.id; - assert.ok(intervalId !== undefined, "poll interval is active"); - - await deps._fireInterval(intervalId); // call 2 — fails - await deps._fireInterval(intervalId); // call 3 — succeeds - - assert.equal( - deps.onSuccess.mock.calls.length, - 1, - "onSuccess fired after poll win", - ); - assert.deepEqual(ctrl.getState(), { - isPending: false, - isWaitingOnReconnectHook: false, - }); -}); - -test("connection-state emitter fires onSuccess and cancels poll/backstop", async () => { +test("connection-state emitter fires onSuccess and cancels backstop", async () => { let capturedListener = null; const ctrl = new RelayReconnectController(); const deps = makeDeps({ @@ -253,7 +222,7 @@ test("connection-state emitter fires onSuccess and cancels poll/backstop", async await ctrl.start(deps); - // Emitter fires "connected" — should resolve without waiting for poll. + // Emitter fires "connected" — should resolve without waiting for backstop. assert.ok(capturedListener !== null, "connection-state listener registered"); capturedListener("connected"); @@ -266,7 +235,6 @@ test("connection-state emitter fires onSuccess and cancels poll/backstop", async isPending: false, isWaitingOnReconnectHook: false, }); - assert.equal(deps._intervals.length, 0, "poll interval cancelled"); assert.equal(deps._timers.length, 0, "backstop cancelled"); }); @@ -299,7 +267,7 @@ test("backstop fires onBackstop, not onSuccess, and resets state", async () => { // ── Cancellation token ──────────────────────────────────────────────────────── -test("cancellation token — superseded poll callback does not call onSuccess", async () => { +test("cancellation token — superseded attempt does not call onSuccess", async () => { const ctrl = new RelayReconnectController(); let preconnectCallCount = 0; let resolveFirstPreconnect; @@ -375,11 +343,6 @@ test("cancel mid fast-path — preconnect resolves SUCCESSFULLY after cancel but "state is idle", ); assert.equal(deps._timers.length, 0, "no timers outstanding after cancel"); - assert.equal( - deps._intervals.length, - 0, - "no intervals outstanding after cancel", - ); }); test("last subscriber unsubscribe cancels in-flight attempt", async () => { @@ -505,7 +468,7 @@ test("OSS build (hookConfigured returns false) — runHook never called", async // ── Synchronous connected emission ─────────────────────────────────────────── -test("sync connected emission — onSuccess fires once, no interval/backstop installed, subscription cleaned up", async () => { +test("sync connected emission — onSuccess fires once, no backstop installed, subscription cleaned up", async () => { const ctrl = new RelayReconnectController(); // subscribeToConnectionState fake that invokes the listener synchronously @@ -538,7 +501,6 @@ test("sync connected emission — onSuccess fires once, no interval/backstop ins 1, "onSuccess fires exactly once", ); - assert.equal(deps._intervals.length, 0, "no poll interval installed"); assert.equal(deps._timers.length, 0, "no backstop timer installed"); assert.equal(cleanupCalled, true, "subscription cleanup handle was called"); assert.deepEqual( diff --git a/desktop/src/shared/api/relayReconnectController.ts b/desktop/src/shared/api/relayReconnectController.ts index fcea2f29dc..b16cc73dfd 100644 --- a/desktop/src/shared/api/relayReconnectController.ts +++ b/desktop/src/shared/api/relayReconnectController.ts @@ -12,11 +12,9 @@ * returns quickly; any browser-based reconnect flow it triggers runs * asynchronously. * - * 3. **Poll-until-connected** — retry `preconnect()` every POLL_INTERVAL_MS. - * The moment the relay becomes reachable the next poll succeeds and - * success is declared — the backstop is a ceiling, not a delay. The - * connection-state emitter is also watched: if the session's background - * retry loop reconnects first, we catch it immediately. + * 3. **Wait for background reconnect** — observe the session's exponential- + * backoff reconnect loop. Success is declared when its connection-state + * emitter reports connected; the backstop is a UI ceiling, not a delay. * * The controller is a module-level singleton so all hook instances in the * same app share a single in-flight state. A cancellation token is bumped on @@ -32,16 +30,13 @@ export type ReconnectTimingPolicy = { /** Short deadline for the optimistic fast-path attempt. */ fastPathTimeoutMs: number; - /** Interval between poll attempts during phase 3. */ - pollIntervalMs: number; - /** Maximum total time to keep polling before giving up. */ + /** Maximum total time to wait before returning control to the UI. */ backstopMs: number; }; /** Current production reconnect timings. */ export const DEFAULT_RECONNECT_TIMING_POLICY: ReconnectTimingPolicy = { - fastPathTimeoutMs: 4_000, - pollIntervalMs: 3_000, + fastPathTimeoutMs: 11_000, backstopMs: 120_000, }; @@ -61,8 +56,6 @@ export type ReconnectDeps = { onBackstop: () => void; setTimeout: (fn: () => void, ms: number) => number; clearTimeout: (id: number) => void; - setInterval: (fn: () => void, ms: number) => number; - clearInterval: (id: number) => void; }; function withDeadline( @@ -101,14 +94,12 @@ export class RelayReconnectController { // All async continuations capture the token at their creation point and // bail if it has since been superseded. private attemptToken = 0; - // Active timers and subscription for the current attempt. The timer-clear - // functions are stored from the deps of the most recent start() call so - // that cancel() and teardown do not need the caller to supply deps again. - private pollIntervalId: number | null = null; + // Active timer and subscription for the current attempt. The timer-clear + // function is stored from the deps of the most recent start() call so that + // cancel() and teardown do not need the caller to supply deps again. private backstopId: number | null = null; private unsubscribeConnectionState: (() => void) | null = null; private clearTimeoutFn: ((id: number) => void) | null = null; - private clearIntervalFn: ((id: number) => void) | null = null; /** Subscribe to state changes. Fires immediately with the current state. */ subscribe(listener: Listener): () => void { @@ -117,7 +108,7 @@ export class RelayReconnectController { return () => { this.listeners.delete(listener); // Cancel the in-flight attempt when the last subscriber leaves — no UI - // is watching the result, so letting poll/backstop callbacks fire + // is watching the result, so letting backstop callbacks fire // (possibly mutating state or invoking onSuccess/onBackstop into a // stale closure) would be a resource and correctness leak. if (this.listeners.size === 0) { @@ -141,7 +132,6 @@ export class RelayReconnectController { // Store timer-clear fns so cancel() can clear timers without caller deps. this.clearTimeoutFn = deps.clearTimeout; - this.clearIntervalFn = deps.clearInterval; // Bump the cancellation token before any await. const token = ++this.attemptToken; @@ -164,7 +154,7 @@ export class RelayReconnectController { return true; } catch { if (cancelled()) return false; - // Fast path failed — continue to escalation or polling. + // Fast path failed — continue to escalation or background reconnect. } // ── Phase 2: escalation (hook-configured builds only) ─────────────────── @@ -184,7 +174,7 @@ export class RelayReconnectController { await deps.runHook(); } catch (err) { // Non-fatal — hook failure means the browser-based reconnect flow may - // not have opened, but we still poll for relay reachability. + // not have opened, but the session still retries in the background. console.warn( "[RelayReconnectController] transport recovery hook failed:", err, @@ -194,11 +184,10 @@ export class RelayReconnectController { this.setState({ isPending: true, isWaitingOnReconnectHook: true }); } - // ── Phase 3: poll-until-connected ──────────────────────────────────────── - // Retry preconnect on a fixed interval. The moment the relay becomes - // reachable the next poll fires success. The connection-state emitter is - // also watched; if the session's background retry loop reconnects first, - // we catch it here too. + // ── Phase 3: wait for background reconnect ────────────────────────────── + // The fast-path preconnect arms the session's exponential-backoff loop. + // Observe it rather than issuing fixed-cadence attempts that consume its + // pending retry timer. let resolved = false; // Capture onSuccess/onBackstop at phase-3 entry so that cancel() (which @@ -236,21 +225,10 @@ export class RelayReconnectController { return false; } - this.pollIntervalId = deps.setInterval(() => { - if (resolved || cancelled()) return; - void deps - .preconnect() - .then(onConnected) - .catch(() => { - // Poll failed — keep trying. - }); - }, this.timingPolicy.pollIntervalMs); - this.backstopId = deps.setTimeout(() => { if (resolved || cancelled()) return; resolved = true; - // keepAliveRequested remains true via preconnect() — the session's - // background retry loop keeps running. The notification is soft. + // The session's exponential-backoff reconnect loop remains armed. onBackstop(); this.finish(() => {}, false); }, this.timingPolicy.backstopMs); @@ -281,10 +259,6 @@ export class RelayReconnectController { } private cancelTimers(): void { - if (this.pollIntervalId !== null && this.clearIntervalFn !== null) { - this.clearIntervalFn(this.pollIntervalId); - this.pollIntervalId = null; - } if (this.backstopId !== null && this.clearTimeoutFn !== null) { this.clearTimeoutFn(this.backstopId); this.backstopId = null; diff --git a/desktop/src/shared/api/useReconnectRelay.ts b/desktop/src/shared/api/useReconnectRelay.ts index 0dbdb832fe..6de4a82825 100644 --- a/desktop/src/shared/api/useReconnectRelay.ts +++ b/desktop/src/shared/api/useReconnectRelay.ts @@ -30,8 +30,6 @@ function buildDeps(onSuccess: () => void, onBackstop: () => void) { onBackstop, setTimeout: window.setTimeout.bind(window), clearTimeout: window.clearTimeout.bind(window), - setInterval: window.setInterval.bind(window), - clearInterval: window.clearInterval.bind(window), }; }