diff --git a/web/src/lib/__tests__/staleBuild.test.ts b/web/src/lib/__tests__/staleBuild.test.ts index a265d36..8a62635 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, holdReloadWhile, makeConnectionWatcher, startBuildWatch } from "@/lib/staleBuild"; import { APP_VERSION } from "@/lib/version"; function healthReplies(body: unknown, ok = true) { @@ -65,3 +65,83 @@ describe("reloadIfServerRebuilt", () => { expect(reload).not.toHaveBeenCalled(); }); }); + +describe("unsaved work holds the page", () => { + it("does not reload while something says it has unsaved work", async () => { + const release = holdReloadWhile(() => true); + vi.stubGlobal("fetch", healthReplies({ ok: true, version: "9.9.9" })); + expect(await reloadIfServerRebuilt()).toBe(false); + expect(reload).not.toHaveBeenCalled(); + release(); + expect(await reloadIfServerRebuilt()).toBe(true); + expect(reload).toHaveBeenCalledOnce(); + }); + + it("treats a predicate that throws as a reason to wait", async () => { + const release = holdReloadWhile(() => { + throw new Error("broken"); + }); + vi.stubGlobal("fetch", healthReplies({ ok: true, version: "9.9.9" })); + expect(await reloadIfServerRebuilt()).toBe(false); + release(); + }); +}); + +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..9c69ae3 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 @@ -45,12 +46,52 @@ function forget(): void { } } +/** + * Reasons to leave a stale page alone for now. + * + * A reload throws away everything the tab has not sent anywhere, and on an + * immutable instance the session is gone by the time we get here, so a compose + * window holding text that never reached the server cannot save it either. + * Reloading would be the difference between the author signing in again and + * pressing send, and losing what they wrote. Whoever owns such state says so + * here; see the registration at the bottom of `store/compose.ts`. + */ +const holds = new Set<() => boolean>(); + +export function holdReloadWhile(fn: () => boolean): () => void { + holds.add(fn); + return () => holds.delete(fn); +} + +function held(): boolean { + for (const fn of holds) { + try { + if (fn()) return true; + } catch { + /* a broken predicate is not a reason to reload over someone's work */ + return true; + } + } + return false; +} + +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" }); @@ -73,7 +114,60 @@ export async function reloadIfServerRebuilt(): Promise { // still reports the old version -- a stale proxy cache, a half-finished // deploy -- this stops the two of them reloading each other in a loop. if (tried() === serverVersion) return false; + if (held()) return false; remember(serverVersion); 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( diff --git a/web/src/store/compose.ts b/web/src/store/compose.ts index 8330874..b73ae2d 100644 --- a/web/src/store/compose.ts +++ b/web/src/store/compose.ts @@ -10,6 +10,7 @@ import { useMail, FULL_PROPS, BODY_PROPS } from "./mail"; import { ensureScheduledMailbox, useScheduled } from "./scheduled"; import { formatScheduleTime, holdUntil } from "@/lib/schedule"; import { settings } from "./settings"; +import { holdReloadWhile } from "@/lib/staleBuild"; export interface ComposeAttachment { id: string; @@ -807,3 +808,8 @@ export function draftFromMailto(url: string): Partial { ...(body ? { html: body, text: m.body } : {}), }; } + +// A deploy can reload this tab out from under whoever is writing. Text that has +// not been autosaved lives only here, and once the session is gone it cannot be +// saved at all -- so say so, and let them sign in and send it instead. +holdReloadWhile(() => useCompose.getState().drafts.some((d) => d.dirty));