diff --git a/web/src/lib/__tests__/staleBuild.test.ts b/web/src/lib/__tests__/staleBuild.test.ts index a265d36..2064f82 100644 --- a/web/src/lib/__tests__/staleBuild.test.ts +++ b/web/src/lib/__tests__/staleBuild.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; -import { reloadIfServerRebuilt } from "@/lib/staleBuild"; +import { reloadIfServerRebuilt, makeConnectionWatcher, startBuildWatch } from "@/lib/staleBuild"; import { APP_VERSION } from "@/lib/version"; function healthReplies(body: unknown, ok = true) { @@ -65,3 +65,62 @@ describe("reloadIfServerRebuilt", () => { expect(reload).not.toHaveBeenCalled(); }); }); + +describe("noticing without being asked", () => { + it("checks when the push stream drops, but not before it has connected", async () => { + const fetchMock = healthReplies({ ok: true, version: APP_VERSION }); + vi.stubGlobal("fetch", fetchMock); + const onState = makeConnectionWatcher(); + + // never connected: a disconnect is not news + onState("connecting"); + await new Promise((r) => setTimeout(r, 0)); + expect(fetchMock).not.toHaveBeenCalled(); + + onState("connected"); + onState("connecting"); + await new Promise((r) => setTimeout(r, 0)); + expect(fetchMock).toHaveBeenCalled(); + }); + + it("asks the server once when several things notice at the same moment", async () => { + const fetchMock = healthReplies({ ok: true, version: APP_VERSION }); + vi.stubGlobal("fetch", fetchMock); + await Promise.all([reloadIfServerRebuilt(), reloadIfServerRebuilt(), reloadIfServerRebuilt()]); + expect(fetchMock).toHaveBeenCalledOnce(); + }); +}); + +describe("the poll is what the guarantee rests on", () => { + it("checks on its own while the tab is visible, with nobody touching it", async () => { + vi.useFakeTimers(); + const fetchMock = healthReplies({ ok: true, version: "9.9.9" }); + vi.stubGlobal("fetch", fetchMock); + Object.defineProperty(document, "visibilityState", { configurable: true, get: () => "visible" }); + + startBuildWatch(); + expect(fetchMock).not.toHaveBeenCalled(); + + await vi.advanceTimersByTimeAsync(60_000); + expect(fetchMock).toHaveBeenCalled(); + vi.useRealTimers(); + }); + + it("leaves a hidden tab alone until it is looked at", async () => { + vi.useFakeTimers(); + const fetchMock = healthReplies({ ok: true, version: APP_VERSION }); + vi.stubGlobal("fetch", fetchMock); + let visibility = "hidden"; + Object.defineProperty(document, "visibilityState", { configurable: true, get: () => visibility }); + + startBuildWatch(); + await vi.advanceTimersByTimeAsync(180_000); + expect(fetchMock).not.toHaveBeenCalled(); + + visibility = "visible"; + document.dispatchEvent(new Event("visibilitychange")); + await vi.advanceTimersByTimeAsync(0); + expect(fetchMock).toHaveBeenCalled(); + vi.useRealTimers(); + }); +}); diff --git a/web/src/lib/staleBuild.ts b/web/src/lib/staleBuild.ts index 0c208b3..568ed8e 100644 --- a/web/src/lib/staleBuild.ts +++ b/web/src/lib/staleBuild.ts @@ -1,4 +1,5 @@ import { APP_VERSION } from "./version"; +import { push, type PushState } from "@/jmap/push"; /** * Reload the page when the server is serving a build this one did not come @@ -13,10 +14,16 @@ import { APP_VERSION } from "./version"; * * `index.html` is served `no-cache` and the assets under it are content-hashed * and immutable, so a reload is all it takes; the only missing part was - * something to ask for one. Checking on a 401 rather than on a timer keeps it - * to the moment it matters and costs one small request, and comparing versions - * rather than reloading on every 401 means an ordinary session expiry still - * lands on the sign-in form with the page intact. + * something to ask for one. Comparing versions rather than reloading on every + * 401 means an ordinary session expiry still lands on the sign-in form with the + * page intact -- only a build that actually moved costs the page. + * + * The reload is unconditional once the versions differ. A compose window can + * be holding text that never reached the server, and after a deploy it cannot + * be saved either, since the session went with the container -- so this will + * sometimes take an unsent draft with it. That is a deliberate trade: a tab + * running code the server no longer speaks is the worse failure, and one that + * stays behind because someone left a draft open is not automatic at all. */ const TRIED_KEY = "ihasmail:reloaded-for"; @@ -45,12 +52,23 @@ function forget(): void { } } +let inFlight: Promise | null = null; + /** * True when a reload has been asked for and the caller should leave the page * alone. False for every other outcome, including not being able to tell -- * failing to reach the server is not a reason to throw away what is on screen. */ -export async function reloadIfServerRebuilt(): Promise { +export function reloadIfServerRebuilt(): Promise { + // Several things can notice a deploy at once -- the stream dropping and the + // request that follows it -- and they should not each ask the server. + inFlight ??= check().finally(() => { + inFlight = null; + }); + return inFlight; +} + +async function check(): Promise { let serverVersion: string; try { const res = await fetch("/api/health", { credentials: "same-origin", cache: "no-store" }); @@ -77,3 +95,55 @@ export async function reloadIfServerRebuilt(): Promise { window.location.reload(); return true; } + +/** + * Watch for a deploy without waiting to be asked. + * + * Checking on a 401 alone was not automatic, only deferred: it needs the tab to + * make a request, so one sitting idle keeps running the old build until someone + * touches it. + * + * The obvious signal turned out to be the wrong one. A deploy kills the + * EventSource behind `/api/events`, which looks like the perfect cue -- except + * it arrives while the container is still being replaced, so the check that + * follows cannot reach the server. Waiting for the stream to come back instead + * does not work either: the session died with the old container, so the + * reconnect is answered with a 401 and never reaches "connected" at all. The + * drop is kept below because it is free and sometimes lands early enough to be + * useful, but nothing depends on it. + * + * What the guarantee rests on is a slow poll while the tab is visible, plus a + * check when it becomes visible again. Neither cares what the stream is doing + * or whether anyone is at the keyboard: a tab left open through a deploy + * notices within a minute, and a backgrounded one notices the moment it is + * looked at. `/api/health` touches nothing upstream, so the cost is one small + * request a minute per open tab. + */ +const POLL_MS = 60_000; + +export function makeConnectionWatcher(): (state: PushState) => void { + let wasConnected = false; + return (state) => { + if (state === "connected") { + wasConnected = true; + return; + } + // Only a drop is news. Never having connected is not evidence of anything. + if (!wasConnected) return; + wasConnected = false; + void reloadIfServerRebuilt(); + }; +} + +export function startBuildWatch(): void { + push.onConnection(makeConnectionWatcher()); + + window.setInterval(() => { + // A hidden tab is not being read, and will be checked when it surfaces. + if (document.visibilityState === "visible") void reloadIfServerRebuilt(); + }, POLL_MS); + + document.addEventListener("visibilitychange", () => { + if (document.visibilityState === "visible") void reloadIfServerRebuilt(); + }); +} diff --git a/web/src/main.tsx b/web/src/main.tsx index 5983146..5c3f5ee 100644 --- a/web/src/main.tsx +++ b/web/src/main.tsx @@ -2,6 +2,9 @@ import { StrictMode } from "react"; import { createRoot } from "react-dom/client"; import "./styles/app.css"; import { App } from "./App"; +import { startBuildWatch } from "@/lib/staleBuild"; + +startBuildWatch(); createRoot(document.getElementById("root")!).render(