diff --git a/apps/fxblox-web/src/app/__tests__/guards.test.tsx b/apps/fxblox-web/src/app/__tests__/guards.test.tsx index 36d2c8a..099d73e 100644 --- a/apps/fxblox-web/src/app/__tests__/guards.test.tsx +++ b/apps/fxblox-web/src/app/__tests__/guards.test.tsx @@ -46,8 +46,14 @@ import { useBloxsStore, useUserProfileStore } from '@/stores'; * slowly under a loaded full-suite run hit the TEST timeout before its 15 s `waitFor` could resolve — so the * wait could never actually be waited out, and the file failed roughly one run in two (on baseline too, not * only with changes). Giving the file room makes the route waits the thing that decides the outcome. + * + * 30 s, not 15: even with the real libp2p boot stubbed out (which was most of it), "/ lands on /blox" still + * missed a 15 s budget about one full-suite run in three, on CI and locally, with nothing else competing — + * the Blox screen's chunk is the largest in the app and Vitest transforms it in a worker that is sharing the + * machine with every other file. Alone it takes ~2.4 s. These tests assert which route matched, never how + * fast; a budget that a busy runner can miss turns them into a coin toss on every merge. */ -const ROUTE_TIMEOUT = 15_000; +const ROUTE_TIMEOUT = 30_000; vi.setConfig({ testTimeout: ROUTE_TIMEOUT * 3 }); function deferred(): Deferred { diff --git a/apps/fxblox-web/src/wallet/__tests__/relayWake.test.ts b/apps/fxblox-web/src/wallet/__tests__/relayWake.test.ts index 0dd11fc..133a073 100644 --- a/apps/fxblox-web/src/wallet/__tests__/relayWake.test.ts +++ b/apps/fxblox-web/src/wallet/__tests__/relayWake.test.ts @@ -1,6 +1,6 @@ import { renderHook } from '@testing-library/react'; import { afterEach, describe, expect, it, vi } from 'vitest'; -import { BACKGROUND_STINT_MS, useRelayWake, wakeRelay, WAKE_TIMEOUT_MS } from '../relayWake'; +import { BACKGROUND_STINT_MS, parkRelay, useRelayWake, wakeRelay, WAKE_TIMEOUT_MS } from '../relayWake'; interface RelayerState { connected?: boolean; @@ -10,6 +10,8 @@ interface RelayerState { /** Flip `connected` to true once `transportOpen` resolves, the way a healthy dial behaves. */ opensSuccessfully?: boolean; withRestart?: boolean; + /** Expose `transportClose` — what parking on `hidden` calls. Off models a relayer without it. */ + withClose?: boolean; /** * Expose `onProviderDisconnect` — the relayer's own "socket closed, dial a fresh one" path. `fresh` says * whether that fresh dial comes up (after a short delay, like the real 100 ms reconnect timer) or never. @@ -24,9 +26,10 @@ function providerWithRelayer(state: RelayerState = {}) { wedged = false, opensSuccessfully = false, withRestart = true, + withClose = true, withFastDisconnect = false, } = state; - const relayer: Record = { connected, connecting }; + const relayer: Record = { connected, connecting, provider: { socket: 'original' } }; const transportOpen = vi.fn(() => wedged ? new Promise(() => undefined) @@ -35,17 +38,30 @@ function providerWithRelayer(state: RelayerState = {}) { }), ); const restartTransport = vi.fn(async () => { + relayer.provider = { socket: 'restarted' }; relayer.connected = true; }); - const onProviderDisconnect = vi.fn(async () => { + const transportClose = vi.fn(async () => { relayer.connected = false; - if (withFastDisconnect === 'fresh') setTimeout(() => (relayer.connected = true), 120); + relayer.connecting = false; + }); + const onProviderDisconnect = vi.fn(async () => { + // The real one leaves the dead socket in place — still reporting OPEN — and dials a fresh one 100 ms + // later. 'never' models a dial that does not come up: the old socket keeps lying the whole time. + if (withFastDisconnect === 'fresh') { + setTimeout(() => { + relayer.provider = { socket: 'fresh' }; + relayer.connected = true; + }, 120); + } }); relayer.transportOpen = transportOpen; + if (withClose) relayer.transportClose = transportClose; if (withRestart) relayer.restartTransport = restartTransport; if (withFastDisconnect) relayer.onProviderDisconnect = onProviderDisconnect; return { transportOpen, + transportClose, restartTransport, onProviderDisconnect, relayer, @@ -104,6 +120,8 @@ describe('wakeRelay', () => { }); it('falls back to the polite restart when the fresh socket does not come up within the bound', async () => { + // The dead socket keeps reporting OPEN throughout ('never' leaves it in place), so a check on `connected` + // alone would declare victory at once. The reporter's log showed exactly that: "fresh socket up in 1ms". vi.useFakeTimers(); const { provider, restartTransport, onProviderDisconnect, relayer } = providerWithRelayer({ connected: true, @@ -115,6 +133,7 @@ describe('wakeRelay', () => { expect(onProviderDisconnect).toHaveBeenCalledTimes(1); expect(restartTransport).toHaveBeenCalledTimes(1); expect(relayer.connected).toBe(true); + expect(relayer.provider).toEqual({ socket: 'restarted' }); }); it('after a background stint, a socket that is honestly down takes the normal path', async () => { @@ -187,7 +206,44 @@ describe('wakeRelay', () => { }); }); +describe('parkRelay', () => { + it('closes an open socket so nothing dials while the tab cannot reach the network', async () => { + const { provider, transportClose } = providerWithRelayer({ connected: true }); + await parkRelay(provider); + expect(transportClose).toHaveBeenCalledTimes(1); + }); + + it('also closes a socket mid-dial, so no pending attempt is left for the return to wait on', async () => { + const { provider, transportClose } = providerWithRelayer({ connecting: true }); + await parkRelay(provider); + expect(transportClose).toHaveBeenCalledTimes(1); + }); + + it('leaves a socket that is already down alone', async () => { + const { provider, transportClose } = providerWithRelayer(); + await parkRelay(provider); + expect(transportClose).not.toHaveBeenCalled(); + }); + + it('is inert for a wallet with no relay, and swallows a failed close', async () => { + await expect(parkRelay(undefined)).resolves.toBeUndefined(); + const { provider, transportClose } = providerWithRelayer({ connected: true }); + transportClose.mockRejectedValueOnce(new Error('already closing')); + await expect(parkRelay(provider)).resolves.toBeUndefined(); + }); +}); + describe('useRelayWake', () => { + it('parks the socket the moment the tab goes hidden', () => { + // The reporter's log: 29 s in the wallet, the library dialling a dead network the whole time, and the + // return spent 8 s waiting on the backoff it had accrued. Close on the way out; nothing accrues. + const { provider, transportClose } = providerWithRelayer({ connected: true }); + renderHook(() => useRelayWake(provider)); + setVisibility('hidden'); + document.dispatchEvent(new Event('visibilitychange')); + expect(transportClose).toHaveBeenCalledTimes(1); + }); + it('wakes the socket when the tab comes back to the front', () => { const { provider, transportOpen } = providerWithRelayer(); renderHook(() => useRelayWake(provider)); @@ -201,9 +257,38 @@ describe('useRelayWake', () => { expect(transportOpen).toHaveBeenCalledTimes(1); }); - it('restarts a socket that claims to be open when the tab was away long enough to have been in a wallet', () => { + it('a trip to the wallet is: park on the way out, one clean dial on the way back', async () => { + // The whole point of parking. The socket was closed on `hidden`, so the return finds it honestly down and + // simply opens the transport — no restart, no waiting on a reconnect the library started in the dark. vi.useFakeTimers(); - const { provider, restartTransport, transportOpen } = providerWithRelayer({ connected: true }); + // `opensSuccessfully`: the single dial on the way back comes up, as it does on a working network. + const { provider, transportClose, transportOpen, restartTransport } = providerWithRelayer({ + connected: true, + opensSuccessfully: true, + }); + renderHook(() => useRelayWake(provider)); + + setVisibility('hidden'); + document.dispatchEvent(new Event('visibilitychange')); + await vi.advanceTimersByTimeAsync(BACKGROUND_STINT_MS + 500); + expect(transportClose).toHaveBeenCalledTimes(1); + + setVisibility('visible'); + document.dispatchEvent(new Event('visibilitychange')); + await vi.advanceTimersByTimeAsync(0); + + expect(transportOpen).toHaveBeenCalledTimes(1); + expect(restartTransport).not.toHaveBeenCalled(); + }); + + it('without transportClose to park with, a socket still claiming OPEN on return is replaced', () => { + // A relayer shape without `transportClose` cannot be parked, so the socket comes back reporting OPEN + // after a real stint — the zombie case — and is replaced rather than believed. + vi.useFakeTimers(); + const { provider, restartTransport, transportOpen } = providerWithRelayer({ + connected: true, + withClose: false, + }); renderHook(() => useRelayWake(provider)); setVisibility('hidden'); diff --git a/apps/fxblox-web/src/wallet/relayWake.ts b/apps/fxblox-web/src/wallet/relayWake.ts index 2eaebee..b987338 100644 --- a/apps/fxblox-web/src/wallet/relayWake.ts +++ b/apps/fxblox-web/src/wallet/relayWake.ts @@ -59,12 +59,40 @@ * is Chrome's TCP stack, eventually, which is the several seconds of "Connecting Wallet…" a user watches * after they have already approved. * - * So when the tab comes back from a real stint in the background, `readyState` is not asked. The transport - * is restarted outright. `restartTransport()` tears the socket down, dials again, re-subscribes every topic, - * and its subscriber then calls `batchFetchMessages` — which is precisely the fetch of whatever the wallet - * published while we were dead. A socket that was in fact healthy pays one reconnect, well under a second on - * a working network; a socket that was not pays nothing more than it already owed. A tab hidden for less than - * `BACKGROUND_STINT_MS` is a flick between tabs, not a trip to a wallet, and is left alone. + * So when the tab comes back from a real stint in the background, `readyState` is not asked. A socket that + * claims OPEN is dropped for a fresh one — see `wakeRelay` for the two ways of doing that and why the fast one + * is preferred. A tab hidden for less than `BACKGROUND_STINT_MS` is a flick between tabs, not a trip to a + * wallet, and is left alone. + * + * ## Why the socket is closed on the way OUT, not only reopened on the way back + * + * A diagnostic log from the reporter's phone (build 7187fba) showed where the seconds actually went on the + * return from the connect approval. The socket was not a zombie. It was DOWN, with `connecting === true`: + * + * [tab] visible after 28875ms hidden + * [relay] socket is down (connecting=true) — opening the transport + * [relay] transportOpen did not get the socket up within the bound — restarting the transport (+2503ms) + * [relay] transport restart finished in 8272ms, connected=true (+8273ms) + * [wallet] connected=true (+8651ms) + * + * What happened while the tab was hidden: Android took the network, the socket closed, and the library did + * what it does on a close — scheduled a reconnect, dialled, failed (no network), slept its backoff, dialled + * again, failed, slept longer. `connect()` retries five times with a sleep of `attempt` seconds between them. + * The tab came back in the middle of one of those sleeps, with `connectPromise` pending, and everything — + * the library's own `transportOpen()`, and every lever here — awaits that promise. Nothing can cancel a + * `setTimeout` inside the library. So the return paid the rest of the sleep, then one dial, plus 2.5 s of + * this code waiting for a promise that was never going to resolve inside the bound. Nine seconds, none of + * them a dial that reached the relay. + * + * The third return in the same log, which landed at a luckier point in the loop, took 1.5 s — and that is + * the floor: one dial on that network. + * + * So: the moment the tab goes hidden, `transportClose()`. It sets `transportExplicitlyClosed`, which is the + * one flag every auto-reconnect path checks first, so no dial is attempted while there is no network to dial + * on, no backoff accrues, and no promise is left pending for the return to trip over. `connect()` clears the + * flag on the way back in, so `transportOpen()` on `visible` is a single clean dial. The wallet's approval or + * signature, published while we were away, is queued by the relay against the topic and pushed on + * re-subscribe. Closing costs nothing the user can see: it happens while they are in the wallet. */ import { useEffect, useRef } from 'react'; import { diag, markReturn } from './diag'; @@ -98,7 +126,11 @@ interface RelayerLike { /** True only when the underlying socket's readyState is OPEN. */ connected: boolean; connecting: boolean; + /** The JSON-RPC provider wrapping the current socket; a fresh dial replaces the object. */ + provider?: unknown; transportOpen(): Promise; + /** Closes the socket AND flags the transport explicitly closed, which suppresses every auto-reconnect. */ + transportClose?(): Promise; /** Present since core 2.x; guarded anyway so a shape change degrades to the polite path. */ restartTransport?(): Promise; /** @@ -139,11 +171,32 @@ const settleAfter = (ms: number): Promise => * has no relay). Never rejects: a relay that cannot be reached is not something the caller can act on, and the * next real request reports it properly, with the context of what the user was trying to do. */ -/** Poll `connected` until it flips or the bound expires. */ -async function untilConnected(relayer: RelayerLike, boundMs: number): Promise { +/** + * Poll until a FRESH socket is up, or the bound expires. + * + * `connected` alone is not the test: the socket being replaced still reports OPEN (that is the whole problem), + * so it must be a new provider object AND open. Checking only the flag reported "fresh socket up in 1ms" on + * the reporter's phone — the same dead socket, congratulated. + */ +async function untilFreshSocket(relayer: RelayerLike, before: unknown, boundMs: number): Promise { const deadline = Date.now() + boundMs; - while (!relayer.connected && Date.now() < deadline) await settleAfter(100); - return relayer.connected; + const fresh = () => relayer.connected && relayer.provider !== before; + while (!fresh() && Date.now() < deadline) await settleAfter(100); + return fresh(); +} + +/** + * The tab is going to the background: close the socket now, on purpose, so the library does not spend the + * stint dialling a network that is not there and leave a pending attempt for the return to wait on. File + * header, last section. Never rejects; nothing the caller can do about a close that fails. + */ +export async function parkRelay(provider: unknown): Promise { + const relayer = relayerFrom(provider); + if (!relayer || typeof relayer.transportClose !== 'function') return; + if (!relayer.connected && !relayer.connecting) return; + const startedAt = Date.now(); + await relayer.transportClose().catch(() => undefined); + diag(`[relay] parked the socket for the background in ${Date.now() - startedAt}ms`); } export async function wakeRelay(provider: unknown, opts: WakeRelayOptions = {}): Promise { @@ -165,8 +218,9 @@ export async function wakeRelay(provider: unknown, opts: WakeRelayOptions = {}): // up within the bound. if (typeof relayer.onProviderDisconnect === 'function') { diag('[relay] back from the background: socket claims OPEN — dropping it for a fresh one'); + const before = relayer.provider; await relayer.onProviderDisconnect().catch(() => undefined); - if (await untilConnected(relayer, WAKE_TIMEOUT_MS)) { + if (await untilFreshSocket(relayer, before, WAKE_TIMEOUT_MS)) { diag(`[relay] fresh socket up in ${Date.now() - startedAt}ms`); return; } @@ -207,6 +261,7 @@ export function useRelayWake(provider: unknown): void { if (document.visibilityState === 'hidden') { hiddenAt = Date.now(); diag('[tab] hidden'); + void parkRelay(latest.current); return; } if (document.visibilityState !== 'visible') return;