Notice a new build without being told
Checking only on a 401 was not automatic, just deferred. It needs the tab to make a request, so one left open and idle went on running the old build until somebody touched it -- which is exactly the thing that cannot be relied on. The obvious signal turned out to be the wrong one, and testing is what showed it. A deploy kills the EventSource behind /api/events, which looks like the perfect cue, except it arrives while the container is still being replaced: the check that follows cannot reach the server, fails, and is never retried. Waiting for the stream to come back instead does not work either, because the session died with the old container, so the reconnect is answered with a 401 and never reaches "connected" at all. The drop is still watched, since it costs nothing and sometimes lands late 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. /api/health touches nothing upstream, so a minute between checks costs one small request per open tab. Reloading is now something that happens to people rather than something they ask for, which makes it able to destroy work. A compose window holds text that has not reached the server, and after a deploy it cannot be saved at all -- the session went with the container. Reloading would be the difference between signing in again and pressing send, and losing what was written. So anything holding such state can say so, and compose does; the tab stays on the old build until the draft is dealt with, and catches up on the next check afterwards.
This commit is contained in:
@@ -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();
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user