diff --git a/document/06-traps.md b/document/06-traps.md index 344b826..04d2836 100755 --- a/document/06-traps.md +++ b/document/06-traps.md @@ -743,3 +743,25 @@ in a search for the parameter's name. `needsReconstitution` was the same sentence one line further down: with the container finally arriving, it had to stop being true for a pack, which has nothing to make up. + +## Every trigger was about this device + +A dose logged on a phone at 20:22 was still showing as due on a desktop at +20:47. Both devices online, both signed in, and the desktop had received the +phone's earlier changes perfectly well: the protocol edited on the desktop at +19:01 was already on the phone. + +`SyncRunner` starts a run on four things: this device's own data changing, this +tab becoming visible, this browser coming back online, and startup. Every one +of them is about this device. A tab that is open, in the foreground and +untouched matches none of them, and `visibilitychange` does not fire for a tab +you never looked away from. So the one state in which somebody is staring at +the screen was the one state that never asked the server anything. + +Fixed with a one minute poll while the tab is visible. The shape to remember: +**a list of triggers assembled from local events has no entry for "somebody +else did something", and that gap is invisible in testing** because a developer +reloads constantly and a reload is a trigger. + +Worth checking against any future trigger list: at least one entry has to come +from outside this device, whether that is a poll, a push channel, or a button. diff --git a/src/components/SyncRunner.tsx b/src/components/SyncRunner.tsx index f42ce21..34ec552 100644 --- a/src/components/SyncRunner.tsx +++ b/src/components/SyncRunner.tsx @@ -15,7 +15,7 @@ import { useEffect } from "react"; import { useStore } from "@/lib/store"; -import { createSyncEngine, type SyncPorts } from "@/lib/sync/engine"; +import { POLL_MS, createSyncEngine, type SyncPorts } from "@/lib/sync/engine"; import { fetchBlob, isNative, pushData, session, cryptoAvailable } from "@/lib/sync/client"; import { open } from "@/lib/sync/crypto"; import { accountRequired, HOSTED } from "@/lib/sync/hosted"; @@ -209,12 +209,30 @@ export function SyncRunner() { }; const onOnline = () => engine.request("now"); + /* + * And a heartbeat while somebody is looking. + * + * Reported as a dose logged on a phone at 20:22 and a desktop still + * showing it as due at 20:47, with both devices online and syncing + * correctly in the other direction the whole time. The desktop had pushed + * its own last change at 19:01 and then sat open and untouched, and + * nothing above asks the server anything in that state: `visibilitychange` + * does not fire for a tab you never looked away from. + * + * Skipped while hidden. A hidden tab has nothing to show, becoming visible + * is already a trigger, and browsers throttle timers there anyway. + */ + const poll = setInterval(() => { + if (document.visibilityState === "visible") engine.request("now"); + }, POLL_MS); + document.addEventListener("visibilitychange", onVisibility); window.addEventListener("online", onOnline); window.addEventListener("pagehide", () => void engine.flush()); return () => { unsubscribe(); + clearInterval(poll); document.removeEventListener("visibilitychange", onVisibility); window.removeEventListener("online", onOnline); engine.stop(); diff --git a/src/lib/sync/engine.test.ts b/src/lib/sync/engine.test.ts index ff1f909..922f4c7 100644 --- a/src/lib/sync/engine.test.ts +++ b/src/lib/sync/engine.test.ts @@ -296,6 +296,36 @@ describe("sync engine", () => { expect(h.state.remote).toBeNull(); }); + /* + * The incident behind POLL_MS. A dose logged on a phone at 20:22 was still + * showing as due on a desktop at 20:47: both devices online, the desktop's + * own last change already sent, and nothing in the app asking the server + * anything in that state. + * + * The engine's half of it is this: asked, a device with nothing to send + * takes what the other device left. What was missing was anybody asking, + * which is why POLL_MS exists and why the tab does the asking. + */ + it("takes what another device sent, when it has nothing of its own to send", async () => { + const h = harness(); + h.state.dirty = true; + h.engine.start(); + await tick(0); + const sent = h.state.remoteSeenAt; + expect(sent).not.toBeNull(); + + // The other device pushes. Nothing happens here: no edit, no tab switch. + h.state.remote = h.blob(h.state.clock + 50); + await tick(QUIET_MS * 4); + expect(h.state.pulls).toHaveLength(0); + + // One ask is all it takes, and it is a clean pull rather than a question. + h.engine.request("now"); + await tick(0); + expect(h.state.pulls).toEqual([sent! + 50]); + expect(h.engine.getStatus().phase).toBe("idle"); + }); + it("flushes immediately when the tab is going away", async () => { const h = harness(); h.engine.start(); diff --git a/src/lib/sync/engine.ts b/src/lib/sync/engine.ts index 092937b..6e57c69 100644 --- a/src/lib/sync/engine.ts +++ b/src/lib/sync/engine.ts @@ -73,6 +73,21 @@ export interface SyncPorts { /** How long to wait after the last change before sending. */ export const QUIET_MS = 2500; +/** + * How often an open tab asks the server whether anything happened elsewhere. + * + * Every other trigger is about this device: its own edits, its own return to + * the foreground, its own network coming back. None of them fires when the + * change was made somewhere else and this tab has simply been sitting open, + * which is the one moment somebody is looking straight at a screen that is + * quietly out of date. + * + * A minute is chosen against the cost: the request is a fetch of one small + * record, and when nothing has moved `decideSync` answers "in-step" and no + * data is transferred either way. The caller skips it while the tab is hidden, + * so a phone in a pocket costs nothing and coming back has its own trigger. + */ +export const POLL_MS = 60_000; /** First wait after an unreachable server, doubling to the ceiling. */ export const RETRY_MIN_MS = 5_000; export const RETRY_MAX_MS = 5 * 60_000;