Merge pull request #120 from LINUXexpert-org/reload-without-waiting-for-the-user

Notice a new build without being told

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();
+  });
+});

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
+ * something to ask for one. Comparing versions rather than reloading on every
- * to the moment it matters and costs one small request, and comparing versions
+ * 401 means an ordinary session expiry still lands on the sign-in form with the
- * rather than reloading on every 401 means an ordinary session expiry still
+ * page intact -- only a build that actually moved costs the page.
- * lands on the sign-in form with the page intact.
+ *
+ * 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<boolean> | 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<boolean> {
+export function reloadIfServerRebuilt(): Promise<boolean> {
+  // 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<boolean> {
   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<boolean> {
   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();
+  });
+}

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(
   <StrictMode>