From 0107abb32dd630f23a4415a021003b801c234a8b Mon Sep 17 00:00:00 2001 From: Bohdan Triapitsyn Date: Thu, 3 Sep 2026 00:21:04 +0300 Subject: [PATCH] fix: order concurrent instance probes and honour the probe budget MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Startup warm-up, opening the switcher and the refresh button can all probe at once, and a relay host working through tunnel retries takes an order of magnitude longer than a loopback one — so a slow older run landed last and replaced a fresh "ok" with its own stale "unreachable". Each host now records which run owns its status; a status from the switch flow outranks any probe still running for it. Each relay attempt is also capped by what is left of the 15s budget rather than the full per-request timeout, so an attempt started just under the deadline can no longer run the whole 8s past it. --- packages/ui/src/lib/desktopHostStatus.test.ts | 37 +++++++++++++++++++ packages/ui/src/lib/desktopHostStatus.ts | 22 ++++++++++- packages/ui/src/lib/desktopHosts.ts | 10 ++++- 3 files changed, 65 insertions(+), 4 deletions(-) diff --git a/packages/ui/src/lib/desktopHostStatus.test.ts b/packages/ui/src/lib/desktopHostStatus.test.ts index d2fa63a9..f609b27f 100644 --- a/packages/ui/src/lib/desktopHostStatus.test.ts +++ b/packages/ui/src/lib/desktopHostStatus.test.ts @@ -98,4 +98,41 @@ describe('desktop host statuses', () => { expect(getDesktopHostStatusSnapshot()).not.toBe(before); expect(before.byHostId.remote).toBe(undefined); }); + + test('a slow older run cannot overwrite a newer result', async () => { + // Startup warm-up, opening the switcher and the refresh button all probe; + // whichever finishes last must not be whichever started first. + probeResults['https://remote.example'] = { status: 'unreachable', latencyMs: 0 }; + let releaseSlow!: () => void; + probeGate = new Promise((resolve) => { releaseSlow = resolve; }); + + const slowRun = probeDesktopHosts([host('remote', 'https://remote.example')]); + await Promise.resolve(); + + probeGate = null; + probeResults['https://remote.example'] = { status: 'ok', latencyMs: 30 }; + await probeDesktopHosts([host('remote', 'https://remote.example')]); + expect(getDesktopHostStatusSnapshot().byHostId.remote?.status).toBe('ok'); + + releaseSlow(); + await slowRun; + + expect(getDesktopHostStatusSnapshot().byHostId.remote?.status).toBe('ok'); + expect(getDesktopHostStatusSnapshot().byHostId.remote?.latencyMs).toBe(30); + }); + + test('a status recorded by the switch flow outranks a probe already running', async () => { + probeResults['https://remote.example'] = { status: 'unreachable', latencyMs: 0 }; + let releaseSlow!: () => void; + probeGate = new Promise((resolve) => { releaseSlow = resolve; }); + + const slowRun = probeDesktopHosts([host('remote', 'https://remote.example')]); + await Promise.resolve(); + setDesktopHostStatus('remote', { status: 'ok', latencyMs: 7, via: 'relay' }); + + releaseSlow(); + await slowRun; + + expect(getDesktopHostStatusSnapshot().byHostId.remote?.status).toBe('ok'); + }); }); diff --git a/packages/ui/src/lib/desktopHostStatus.ts b/packages/ui/src/lib/desktopHostStatus.ts index 303f1c5a..2fb3c9f7 100644 --- a/packages/ui/src/lib/desktopHostStatus.ts +++ b/packages/ui/src/lib/desktopHostStatus.ts @@ -40,6 +40,13 @@ type DesktopHostStatusSnapshot = { * them first. */ const statuses = new Map(); +// Startup warm-up, opening the switcher and the refresh button can all be in +// flight at once, and a probe's duration varies by an order of magnitude +// between a loopback host and a relay host working through tunnel retries. +// Without ordering, a slow older run lands last and replaces a fresh "ok" with +// its own stale "unreachable". Each host remembers which run owns its status. +let probeRunSequence = 0; +const owningRunByHostId = new Map(); let activeProbeRuns = 0; let snapshot: DesktopHostStatusSnapshot = { byHostId: {}, isProbing: false }; const listeners = new Set<() => void>(); @@ -69,8 +76,13 @@ const setStatus = (hostId: string, status: DesktopHostStatus): void => { publishSnapshot(); }; -/** Record a status learned outside a probe run — the switch flow probes too. */ +/** + * Record a status learned outside a probe run — the switch flow probes too, and + * its result is the freshest thing anyone has, so it takes ownership away from + * any probe run still running for that host. + */ export const setDesktopHostStatus = (hostId: string, status: DesktopHostStatus): void => { + owningRunByHostId.set(hostId, ++probeRunSequence); setStatus(hostId, status); }; @@ -85,6 +97,7 @@ export const pruneDesktopHostStatuses = (configuredHostIds: readonly string[]): for (const hostId of Array.from(statuses.keys())) { if (keep.has(hostId)) continue; statuses.delete(hostId); + owningRunByHostId.delete(hostId); changed = true; } if (changed) publishSnapshot(); @@ -134,12 +147,17 @@ const probeHost = async (host: DesktopHost, localClientToken: string): Promise => { if (!isDesktopShell()) return; + const run = ++probeRunSequence; + for (const host of hosts) owningRunByHostId.set(host.id, run); activeProbeRuns += 1; publishSnapshot(); try { const localClientToken = await getLocalClientToken(); await Promise.all(hosts.map(async (host) => { - setStatus(host.id, await probeHost(host, localClientToken)); + const status = await probeHost(host, localClientToken); + // A newer run (or a switch) claimed this host while we were probing. + if (owningRunByHostId.get(host.id) !== run) return; + setStatus(host.id, status); })); } finally { activeProbeRuns -= 1; diff --git a/packages/ui/src/lib/desktopHosts.ts b/packages/ui/src/lib/desktopHosts.ts index da846415..ef15adf5 100644 --- a/packages/ui/src/lib/desktopHosts.ts +++ b/packages/ui/src/lib/desktopHosts.ts @@ -415,10 +415,11 @@ const RELAY_PROBE_RETRY_DELAY_MS = 400; const fetchRelayProbe = async ( tunnel: ReturnType, path: string, + timeoutMs: number, init?: RequestInit, ): Promise => { const controller = new AbortController(); - const timer = window.setTimeout(() => controller.abort(), RELAY_PROBE_TIMEOUT_MS); + const timer = window.setTimeout(() => controller.abort(), timeoutMs); try { return await tunnel.fetch(path, { ...init, signal: controller.signal }); } finally { @@ -448,8 +449,13 @@ const fetchRelayProbeUntilDeadline = async ( init?: RequestInit, ): Promise => { for (;;) { + // Every attempt is capped by what is LEFT of the budget, not by the full + // per-request timeout: an attempt started just under the deadline would + // otherwise run the whole 8s past it, and the switch flow waits on this. + const remainingMs = deadline - Date.now(); + if (remainingMs <= 0) throw new Error('relay probe deadline exceeded'); try { - return await fetchRelayProbe(tunnel, path, init); + return await fetchRelayProbe(tunnel, path, Math.min(RELAY_PROBE_TIMEOUT_MS, remainingMs), init); } catch (error) { if (tunnel.getStatus().state === 'error') throw error; if (Date.now() >= deadline) throw error;