diff --git a/apps/web/src/chat/chat.tsx b/apps/web/src/chat/chat.tsx index 8b1ab28bf..cab45f0d8 100644 --- a/apps/web/src/chat/chat.tsx +++ b/apps/web/src/chat/chat.tsx @@ -9,9 +9,15 @@ import { useEffect, useId, useLayoutEffect, useReducer, useRef, useState } from "react"; -import { MAX_RESEARCH_BRIEF, SendAction, usePopoverDismissal } from "@chopin/editor"; +import { + CONNECTION_GRACE, + MAX_RESEARCH_BRIEF, + SendAction, + useConnectionNotice, + usePopoverDismissal, +} from "@chopin/editor"; import { MENTION } from "@chopin/protocol/address"; -import { ArchiveIcon, InfoIcon, LoaderIcon, LockIcon, PlusIcon, WarningIcon } from "@chopin/icons"; +import { ArchiveIcon, InfoIcon, LockIcon, PlusIcon, WarningIcon } from "@chopin/icons"; import { DraftInput } from "./draft-input"; import type { DraftInputHandle } from "./draft-input"; import { ModeSwitch } from "./mode-switch"; @@ -132,6 +138,44 @@ export function runCounts(runs: Wire.Runs | undefined): { active: number; paused }; } +function draftKey(room: string): string { + return `chopin:chat-draft:${room}`; +} + +/** Holds an unsent message across a reload of this tab; storage may refuse. */ +function keepDraft(room: string, draft: ComposerDraft): void { + try { + if (!draft.text.trim()) return sessionStorage.removeItem(draftKey(room)); + sessionStorage.setItem( + draftKey(room), + JSON.stringify({ text: draft.text, references: draft.references }), + ); + } catch { + // Private windows and blocked storage: the draft is lost as it was before. + } +} + +/** The message a reload interrupted; anything malformed starts empty. */ +function restoreDraft(room: string): ComposerDraft { + let empty = { text: "", references: [] }; + try { + let raw = sessionStorage.getItem(draftKey(room)); + if (!raw) return empty; + let saved = JSON.parse(raw) as Partial; + if (typeof saved.text !== "string") return empty; + let references = Array.isArray(saved.references) + && saved.references.every(reference => + typeof reference?.start === "number" && typeof reference.end === "number" + && reference.end <= saved.text!.length + ) + ? saved.references + : []; + return { text: saved.text, references }; + } catch { + return empty; + } +} + export function Chat( { active = true, @@ -170,11 +214,11 @@ export function Chat( let [busy, setBusy] = useState(false); let [runs, setRuns] = useState(); let counts = runCounts(runs); - let [draft, setDraft] = useState({ - text: "", - references: [], - }); + let [draft, setDraft] = useState(() => restoreDraft(room)); let [submitting, setSubmitting] = useState(false); + // A send pressed during a blip, made once the connection is back. + let [held, setHeld] = useState(false); + let sending = submitting || held; let [sendError, setSendError] = useState(); let [researchBlock, setResearchBlock] = useState(); let researching = useRef(false); @@ -190,11 +234,16 @@ export function Chat( let submission = useRef(undefined); let draftRef = useRef(draft); draftRef.current = draft; + // A reload, including the header's fallback when reconnecting keeps + // failing, must not take an unsent message with it. Kept as it changes + // rather than on unload, which a browser does not promise to announce. + useEffect(() => keepDraft(room, draft), [room, draft]); let pickerId = useId(); let mentionPickerId = useId(); let commandPickerId = useId(); let instructionsId = useId(); let cueId = useId(); + let connectionId = useId(); let synchronized = useRef(undefined); let activity = useRef(onActivity); let reportedBusy = useRef(false); @@ -204,7 +253,16 @@ export function Chat( if (!connected) synchronized.current = undefined; let transcriptReady = connected && synchronized.current === wire; let composerReady = transcriptReady && !readonly && !archived; - let connectionLost = !connected && ["reconnecting", "closed"].includes(wire?.status ?? ""); + // A draft never waits on the connection; only sending does. + let draftable = !readonly && !archived; + let connectionNotice = useConnectionNotice(!connected); + let connectionLabel = connectionNotice === "none" + ? undefined + : connectionNotice === "offline" || wire?.status === "closed" || wire?.status === "denied" + ? "Offline" + : wire?.status === "connecting" + ? "Connecting…" + : "Reconnecting…"; let effectiveMode = agent && (mode || addressedOutsideReferences(draft.text, draft.references)); let workingTurn = transcriptReady ? turn : undefined; let suspendedWork = !transcriptReady && turn && transcript.activeAnchorId @@ -387,7 +445,7 @@ export function Chat( }; let submit = () => { - if (submission.current || !composerReady || !wire) return; + if (submission.current || !draftable) return; let current = draftRef.current; if (draftCommand(current.text)) { // `#` references stay in the brief as their visible titles. @@ -395,6 +453,12 @@ export function Chat( return; } if (!current.text.trim()) return; + if (!composerReady || !wire?.connected) { + // Inside the grace period nothing says the connection is down, so + // the send waits for it rather than being refused. + if (connectionNotice === "none") setHeld(true); + return; + } let submitted = prepareDraftSubmission(current); let prefix = effectiveMode && !addressedOutsideReferences(submitted.text, submitted.references) ? `${MENTION} ` @@ -449,8 +513,38 @@ export function Chat( }); }; + // Stop and Resume pressed during a blip are sent once it is over, or never. + let heldControl = useRef<"chat:abort" | "chat:resume">(undefined); + let control = (kind: "chat:abort" | "chat:resume") => { + if (wire?.connected) wire.send(kind); + else heldControl.current = kind; + }; + useEffect(() => { + if (connectionNotice !== "none") heldControl.current = undefined; + else if (connected && heldControl.current && wire?.connected) { + wire.send(heldControl.current); + heldControl.current = undefined; + } + }); + + // Never held longer than the grace period, however the wait ends. + useEffect(() => { + if (!held) return; + let timer = setTimeout(() => setHeld(false), CONNECTION_GRACE); + return () => clearTimeout(timer); + }, [held]); + useEffect(() => { + if (!held) return; + if (connectionNotice !== "none") { + setHeld(false); + } else if (composerReady && wire?.connected) { + setHeld(false); + submit(); + } + }); + let toggleMode = () => { - if (!composerReady || submitting || !agent) return; + if (!draftable || sending || !agent) return; let current = draftRef.current; let at = selection.start; let end = selection.end; @@ -593,7 +687,7 @@ export function Chat(

)} {!readonly && !archived - && (sendError || !composerReady || researchBlock || blockedCommand || !agent) && ( + && (sendError || researchBlock || blockedCommand || !agent) && (
{sendError ? - : connectionLost - ? - : !composerReady - ? : } - {sendError ?? (!composerReady - ? connected ? "Synchronizing…" : connectionLost ? "Connection lost" : "Connecting…" - : researchBlock + {sendError ?? (researchBlock ? researchCopy[researchBlock] : blockedCommand ? "Research isn’t available in this document" @@ -627,15 +715,6 @@ export function Chat( Retry ) - : !composerReady - ? wire && ( - - ) : researchBlock === "drafting" ? onShowResearch && (
)}
@@ -898,13 +978,34 @@ export function Chat( )}
+ {/* Always mounted, so the live region hears its first word. */} + + {connectionLabel && ( + + {connectionLabel} + + )} + + {/* Where the document header is out of view; see composer.css. */} + {connectionNotice === "offline" && wire && ( + + )} {agent && (busy || counts.active > 0) && ( +
+ )} + { + setReconnects(count => count + 1); + wire.reconnect(); + } + : undefined} + synced={planState.synced} + /> + } ids={workspaceIds} identity={room} @@ -923,7 +970,7 @@ export function RoomWorkspace( { }], ["apps/web/src/chat/chat.tsx", { action: "Stop Chopin", - marker: 'wire?.send("chat:abort")', + marker: 'control("chat:abort")', size: "btn-icon", tiers: ["btn-secondary"], }], ["apps/web/src/chat/chat.tsx", { action: "Resume Planner", - marker: 'wire?.send("chat:resume")', + marker: 'control("chat:resume")', size: "btn-icon", tiers: ["btn-secondary"], }], diff --git a/apps/web/src/wire.test.ts b/apps/web/src/wire.test.ts index 8a4a53cf3..8f767b949 100644 --- a/apps/web/src/wire.test.ts +++ b/apps/web/src/wire.test.ts @@ -243,6 +243,110 @@ describe("status", () => { expect(wire.status).toBe("closed"); }); + it("skips the backoff when the network comes back", async () => { + let service = restartable(); + let seen: Status[] = []; + let wire = connect(service.port, seen); + await until(() => wire.status === "connected", "connected"); + // A wake while connected is not a reason to reconnect. + dispatchEvent(new Event("focus")); + await Bun.sleep(50); + expect(seen.filter(status => status === "connected")).toHaveLength(1); + + let random = Math.random; + Math.random = () => 100; + try { + service.drop(); + await until(() => wire.status === "reconnecting", "backoff"); + let woken = Date.now(); + dispatchEvent(new Event("online")); + await until(() => wire.status === "connected", "woken reconnect"); + // Well before the attempt the backoff would otherwise make at 1.2 s. + expect(Date.now() - woken).toBeLessThan(500); + } finally { + Math.random = random; + } + await Bun.sleep(100); + expect(seen.filter(status => status === "connected")).toHaveLength(2); + }); + + it("lands an attempt inside the grace period however long the backoff", async () => { + let service = restartable(); + let seen: Status[] = []; + let wire = connect(service.port, seen); + await until(() => wire.status === "connected", "connected"); + + let random = Math.random; + Math.random = () => 100; + try { + let lost = Date.now(); + service.drop(); + await until(() => seen.filter(status => status === "connected").length === 2, "rescued"); + expect(Date.now() - lost).toBeLessThan(1500); + } finally { + Math.random = random; + } + }); + + it("does not turn a burst of focus events into a burst of attempts", async () => { + let attempts = 0; + let server = Bun.serve({ + port: 0, + hostname: "127.0.0.1", + fetch(request) { + if (request.headers.get("x-chopin-socket-probe") !== "1") attempts++; + return new Response("try again", { status: 503 }); + }, + }); + servers.push(server); + let wire = connect(server.port!, []); + await until(() => wire.status === "reconnecting", "backoff"); + + let random = Math.random; + Math.random = () => 100; + try { + let before = attempts; + for (let i = 0; i < 30; i++) { + dispatchEvent(new Event(i % 2 ? "focus" : "visibilitychange")); + await Bun.sleep(100); + } + // One per second at most, where every event used to start one. + expect(attempts - before).toBeLessThanOrEqual(4); + } finally { + Math.random = random; + } + }, 10_000); + + it("starts the backoff over when asked to reconnect", async () => { + let attempts = 0; + let server = Bun.serve({ + port: 0, + hostname: "127.0.0.1", + fetch(request) { + if (request.headers.get("x-chopin-socket-probe") !== "1") attempts++; + return new Response("try again", { status: 503 }); + }, + }); + servers.push(server); + let random = Math.random; + Math.random = () => 0.999; + try { + let wire = connect(server.port!, []); + await until(() => wire.status === "reconnecting", "backoff"); + // Each failed ask would otherwise double the wait after it. + for (let i = 0; i < 5; i++) { + let before = attempts; + wire.reconnect(); + await until(() => attempts > before, "asked attempt"); + await Bun.sleep(50); + } + let before = attempts; + await until(() => attempts > before, "automatic attempt soon after"); + } finally { + Math.random = random; + } + }); + it("ignores a superseded refusal probe after manual reconnection", async () => { let probe = Promise.withResolvers(); let release = Promise.withResolvers(); diff --git a/apps/web/src/wire.ts b/apps/web/src/wire.ts index dced5f2df..cf058cf5d 100644 --- a/apps/web/src/wire.ts +++ b/apps/web/src/wire.ts @@ -40,6 +40,16 @@ type Refusal = { reason: string; authentication: boolean }; const BASE_DELAY = 500; const MAX_DELAY = 15_000; +/** The least time between attempts that a wake may start. */ +const WAKE_GAP = 1000; +/** + * When, after a loss, one attempt is guaranteed to start. + * + * Just inside the 1.5 s grace the page waits out before saying anything + * (`CONNECTION_GRACE` in the editor), so a short outage reconnects before it + * is shown rather than flashing a notice, and a lock, for a moment past it. + */ +const RESCUE_AT = 1200; function endpoint(options: WireOptions): string { let url = new URL("/ws", location.href); @@ -58,6 +68,9 @@ export class Wire { #listeners = new Map>(); #pending = new Map(); #attempts = 0; + #attemptedAt = 0; + /** When the current outage began, or undefined while connected. */ + #lostAt: number | undefined; #timer: ReturnType | undefined; /** * Undefined until the first transition, so the first one always announces. @@ -77,9 +90,43 @@ export class Wire { constructor(options: WireOptions) { this.#options = options; + addEventListener("online", this.#wake); + addEventListener("focus", this.#wake); + globalThis.document?.addEventListener("visibilitychange", this.#wake); this.#connect(); } + /** + * Skip the rest of the backoff when the network or the person comes back. + * + * The timer alone can sit out most of its 15 s cap after the network has + * already returned, with the page still saying it is offline. Only a wire + * waiting on that timer is woken; an attempt already in flight is left to + * finish. + * + * Focus and visibility say only that the person is back, not the network, + * and they can fire in bursts. So they never wake the wire sooner than + * `WAKE_GAP` after the last attempt, and only `online` forgets the + * backoff: a deploy outage must not turn every tab switch on every client + * into a fresh run of fast retries. + */ + #wake = (event: Event) => { + if (this.#disposed || this.#terminal || this.#timer === undefined) return; + if (globalThis.document?.visibilityState === "hidden") return; + let online = event.type === "online"; + if (!online && Date.now() - this.#attemptedAt < WAKE_GAP) return; + clearTimeout(this.#timer); + this.#timer = undefined; + if (online) this.#attempts = 0; + this.#connect(); + }; + + #unlisten(): void { + removeEventListener("online", this.#wake); + removeEventListener("focus", this.#wake); + globalThis.document?.removeEventListener("visibilitychange", this.#wake); + } + get status(): Status { return this.#status ?? "connecting"; } @@ -95,10 +142,18 @@ export class Wire { return this.#socket?.readyState === WebSocket.OPEN; } + /** + * Try again now, at a person's request. + * + * The backoff starts over: someone who asks to reconnect is saying the + * network may be back, and after a few failed asks the next automatic + * attempt must not still be waiting out the cap. + */ reconnect(): void { if (this.#disposed || this.#terminal) return; clearTimeout(this.#timer); this.#timer = undefined; + this.#attempts = 0; let previous = this.#socket; this.#socket = undefined; this.#abandon("connection restarted"); @@ -115,6 +170,7 @@ export class Wire { #connect(): void { if (this.#disposed || this.#terminal) return; this.#connectionGeneration++; + this.#attemptedAt = Date.now(); this.#set(this.#everConnected ? "reconnecting" : "connecting"); let socket = new WebSocket(endpoint(this.#options)); @@ -124,6 +180,7 @@ export class Wire { if (this.#socket !== socket) return; this.#everConnected = true; this.#attempts = 0; + this.#lostAt = undefined; this.#set("connected"); }); @@ -141,6 +198,7 @@ export class Wire { this.#deleted(); return; } + this.#lostAt ??= Date.now(); this.#abandon("connection lost"); void this.#retry(); }); @@ -183,9 +241,16 @@ export class Wire { } let delay = Math.min(BASE_DELAY * 2 ** this.#attempts, MAX_DELAY) * (0.5 + Math.random()); + if (this.#lostAt !== undefined) { + let rescue = this.#lostAt + RESCUE_AT - Date.now(); + if (rescue > 0) delay = Math.min(delay, rescue); + } this.#attempts++; this.#set("reconnecting"); - this.#timer = setTimeout(() => this.#connect(), delay); + this.#timer = setTimeout(() => { + this.#timer = undefined; + this.#connect(); + }, delay); } #receive(raw: string): void { @@ -229,6 +294,7 @@ export class Wire { #deleted(): void { if (this.#disposed || this.#terminal) return; this.#terminal = true; + this.#unlisten(); if (this.#timer) clearTimeout(this.#timer); this.#timer = undefined; let socket = this.#socket; @@ -280,6 +346,7 @@ export class Wire { dispose(): void { this.#disposed = true; + this.#unlisten(); if (this.#timer) clearTimeout(this.#timer); this.#abandon("disposed"); this.#socket?.close(); diff --git a/e2e/callout.e2e.ts b/e2e/callout.e2e.ts index dd217f111..9881e9571 100644 --- a/e2e/callout.e2e.ts +++ b/e2e/callout.e2e.ts @@ -51,7 +51,9 @@ test("a callout type menu has one keyboard path", async ({ join, seed }) => { test("locking the plan closes an open callout type menu", async ({ join, page, seed }) => { let sockets: WebSocketRoute[] = []; + let offline = false; await page.routeWebSocket("**/ws?**", route => { + if (offline) return route.close(); route.connectToServer(); sockets.push(route); }); @@ -63,6 +65,8 @@ test("locking the plan closes an open callout type menu", async ({ join, page, s await trigger.click(); await expect(page.getByRole("listbox", { name: "Callout type" })).toBeVisible(); + // Held down past the grace period, which a blip would not outlast. + offline = true; await sockets.at(-1)!.close(); await expect(content(page)).toHaveAttribute("contenteditable", "false"); diff --git a/e2e/chat-mentions.e2e.ts b/e2e/chat-mentions.e2e.ts index bd10b3f90..cdc6bcb73 100644 --- a/e2e/chat-mentions.e2e.ts +++ b/e2e/chat-mentions.e2e.ts @@ -217,7 +217,10 @@ test("without a Planner a manually addressed message gets a local notice", async await expect(chat.getByRole("listbox", { name: "Mentions" })).toHaveCount(0); await input.fill("@chopin are you there?"); await chat.getByRole("button", { name: "Send message" }).click(); - await expect(chat.getByText("@chopin are you there?", { exact: true })).toBeVisible(); + // The transcript drops the leading mention from what it shows, so find the + // sent message by what was sent rather than by the draft still on screen. + await expectChatValue(input, ""); + await expect(chat.locator('[data-chat-raw="@chopin are you there?"]')).toBeVisible(); let notice = chat.getByText("Chopin is off on this server. Your message went to the room only.", { exact: true, }); diff --git a/e2e/chat-references.e2e.ts b/e2e/chat-references.e2e.ts index ef5e5473f..30bc45276 100644 --- a/e2e/chat-references.e2e.ts +++ b/e2e/chat-references.e2e.ts @@ -379,7 +379,7 @@ test("legacy delivery clears immediately without waiting for an acknowledgement" await expect(draft).toBeFocused(); }); -test("the composer stays read-only until fresh chat history arrives", async ({ join, page, seed }) => { +test("a send made before fresh chat history arrives waits for it", async ({ join, page, seed }) => { await seed("# Delayed chat history\n"); let releaseHistory: (() => void) | undefined; await page.routeWebSocket("**/ws?**", route => { @@ -400,12 +400,14 @@ test("the composer stays read-only until fresh chat history arrives", async ({ j let chat = chatPane(await join("ana")); let draft = chatInput(chat); await expect.poll(() => releaseHistory !== undefined).toBe(true); - await expect(draft).toHaveAttribute("contenteditable", "false"); - await expect(draft).toHaveAttribute("aria-disabled", "true"); - await expect(chat.getByRole("button", { name: "Send message" })).toBeDisabled(); - releaseHistory!(); await expect(draft).toBeEditable(); await expect(draft).toHaveAttribute("aria-disabled", "false"); await draft.fill("Now synchronized"); - await expect(chat.getByRole("button", { name: "Send message" })).toBeEnabled(); + await draft.press("Enter"); + // Held, not refused, and not sent against a transcript that is not current. + await expect(chat.locator(".composer-surface")).toHaveAttribute("aria-busy", "true"); + await expectChatValue(draft, "Now synchronized"); + releaseHistory!(); + await expectChatValue(draft, ""); + await expect(chat.getByText("Now synchronized", { exact: true })).toBeVisible(); }); diff --git a/e2e/connection.e2e.ts b/e2e/connection.e2e.ts new file mode 100644 index 000000000..2b81084bc --- /dev/null +++ b/e2e/connection.e2e.ts @@ -0,0 +1,236 @@ +/** + * What a dropped connection looks like from the page. + * + * Routed rather than `context.setOffline`, which leaves an established socket + * alone. Proxying the socket is the only way to be the thing that drops it. + */ + +import { $importPlan } from "../packages/dialect/src/index"; + +import * as PlanRoom from "../apps/server/src/plan/room"; +import * as Y from "../apps/server/node_modules/yjs"; +import { REGISTRY } from "../apps/server/src/testing/peer"; +import { chatInput, expectChatValue } from "./chat-input"; +import { readSource } from "./database"; +import { content, expect, ready, status, test } from "./room"; + +import type { Plan } from "../packages/protocol/index"; +import type { Page, WebSocketRoute } from "@playwright/test"; + +function chatPane(page: Page) { + return page.getByRole("complementary", { name: "Chat", exact: true }); +} + +function route(page: Page) { + let sockets: WebSocketRoute[] = []; + let state = { offline: false }; + let ready = page.routeWebSocket("**/ws?**", socket => { + if (state.offline) return socket.close(); + socket.connectToServer(); + sockets.push(socket); + }); + return { sockets, state, ready }; +} + +test("a blip that recovers inside the grace period shows nothing", async ({ join, page }) => { + let wire = route(page); + await wire.ready; + await join("ana"); + let chat = chatPane(page); + let connection = chat.locator(".composer-connection"); + await expect(connection).toBeEmpty(); + + await wire.sockets.at(-1)!.close(); + await expect.poll(() => wire.sockets.length).toBeGreaterThan(1); + + // Past the moment the old client raised its alarms, and still quiet. + let until = Date.now() + 1800; + while (Date.now() < until) { + await expect(content(page)).toHaveAttribute("contenteditable", "true"); + await expect(status(page)).toHaveAttribute("data-level", "hidden"); + await expect(connection).toBeEmpty(); + await expect(chat.getByText(/Connection lost|Synchronizing|Reconnecting|Offline/)) + .toHaveCount(0); + await page.waitForTimeout(150); + } +}); + +test("a long outage says so once, keeps the draft, and comes back on the online event", async ({ join, page }) => { + // The widest jitter, so a scheduled retry cannot pass for the online event. + await page.addInitScript(() => { + Math.random = () => 0.999; + }); + let wire = route(page); + await wire.ready; + await join("ana"); + let chat = chatPane(page); + let input = chatInput(chat); + let connection = chat.locator(".composer-connection"); + let composer = await chat.locator(".chat-composer").boundingBox(); + + wire.state.offline = true; + await wire.sockets.at(-1)!.close(); + + // Still draftable: the composer only holds back the send. + await input.click(); + await page.keyboard.type("Written while offline"); + await expect(connection).toHaveText("Reconnecting…"); + await expect(content(page)).toHaveAttribute("contenteditable", "false"); + await expect(page.locator(".plan[data-plan-offline]")).toHaveCount(1); + await expect(page.getByRole("button", { name: "Send message" })).toBeDisabled(); + await expect(input).toHaveAttribute("contenteditable", "true"); + // Nothing was inserted above the composer to push the transcript. + await expect(chat.getByText(/Connection lost|Synchronizing/)).toHaveCount(0); + expect(await chat.locator(".chat-composer").boundingBox()).toEqual(composer); + + await expect(connection).toHaveText("Offline", { timeout: 10_000 }); + // One action for one state: the document header's, not a second one here. + await expect(chat.getByRole("button", { name: /Reconnect|Reload/ })).toHaveCount(0); + await page.keyboard.type(" and kept"); + + wire.state.offline = false; + let attempts = wire.sockets.length; + let back = Date.now(); + await page.evaluate(() => dispatchEvent(new Event("online"))); + await ready(page); + expect(wire.sockets.length).toBe(attempts + 1); + expect(Date.now() - back).toBeLessThan(3000); + + await expect(connection).toBeEmpty(); + await expectChatValue(input, "Written while offline and kept"); + await input.press("Enter"); + await expectChatValue(input, ""); + await expect(chat.getByText("Written while offline and kept", { exact: true })).toBeVisible(); +}); + +test("a send pressed during a blip goes once the connection is back", async ({ join, page }) => { + let wire = route(page); + await wire.ready; + await join("ana"); + let chat = chatPane(page); + let input = chatInput(chat); + await input.fill("Pressed during a blip"); + + await wire.sockets.at(-1)!.close(); + await input.press("Enter"); + + await expectChatValue(input, ""); + await expect(chat.getByText("Pressed during a blip", { exact: true })).toBeVisible(); + await expect(chat.locator(".composer-connection")).toBeEmpty(); +}); + +/* + * The server rebuilt the document while Ana was away, so she never heard the + * reset. What she typed in the meantime belongs to a history that no longer + * exists; keeping it forked her document, and everything she typed afterwards + * was acknowledged and never reached anyone. + */ +test("edits made during a blip are dropped, and said to be, when the document was rebuilt meanwhile", async ({ join, page, room, seed }) => { + await seed("# Rebuilt\n\nShared prose.\n"); + let ana = route(page); + await ana.ready; + await join("ana"); + + let opened: Plan.Open.Reply | undefined; + let benServer: WebSocketRoute | undefined; + let reset = Promise.withResolvers(); + let ben = await join("ben", {}); + await ben.routeWebSocket("**/ws?**", socket => { + let server = socket.connectToServer(); + benServer = server; + socket.onMessage(message => server.send(message)); + server.onMessage(message => { + if (typeof message === "string") { + let frame = JSON.parse(message) as { kind: string }; + if (frame.kind === "plan:open") opened = frame as Plan.Open.Reply; + if (frame.kind === "plan:reset") reset.resolve(); + } + socket.send(message); + }); + }); + await ben.reload(); + await ready(ben); + await expect.poll(() => opened !== undefined).toBe(true); + + ana.state.offline = true; + await ana.sockets.at(-1)!.close(); + await content(page).getByText("Shared prose.").click(); + await page.keyboard.press("End"); + await page.keyboard.type(" STALE"); + + // A Callout with no id is well-formed MDX and outside the dialect, so the + // room rejects the batch and rebuilds under a fresh epoch. + let port = Number(new URL(ben.url()).port); + let peer = await PlanRoom.restore( + opened!.epoch, + Buffer.from(opened!.update, "base64"), + await readSource(port, room), + [], + ); + let before = Y.encodeStateVector(peer.doc); + peer.editor.update(() => { + $importPlan('\n\tText.\n\n', { + registry: REGISTRY, + validate: false, + }); + }, { discrete: true }); + await PlanRoom.settle(); + let update = Y.encodeStateAsUpdate(peer.doc, before); + peer.doc.destroy(); + benServer!.send(JSON.stringify({ + kind: "plan:update", + ts: 0, + rid: crypto.randomUUID(), + id: crypto.randomUUID(), + epoch: opened!.epoch, + update: Buffer.from(update).toString("base64"), + })); + await reset.promise; + + ana.state.offline = false; + await page.evaluate(() => dispatchEvent(new Event("online"))); + await ready(page); + await expect(page.getByRole("alert")).toContainText( + "Your last edits couldn't be saved because the document changed while you were offline.", + ); + await expect(content(page)).not.toContainText("STALE"); + + // Back in step: what Ana types now reaches Ben. + await content(page).getByText("Shared prose.").click(); + await page.keyboard.press("End"); + await page.keyboard.type(" AFTER"); + await expect(content(ben)).toContainText("Shared prose. AFTER"); + await expect(content(ben)).not.toContainText("STALE"); + + await page.getByRole("button", { name: "Dismiss", exact: true }).click(); + await expect(page.getByRole("alert")).toHaveCount(0); +}); + +test("a reload keeps an unsent Chat message", async ({ join, page }) => { + await join("ana"); + let input = chatInput(chatPane(page)); + await input.fill("Not sent before the reload"); + await page.reload(); + await ready(page); + await expectChatValue(chatInput(chatPane(page)), "Not sent before the reload"); +}); + +test("on a phone, Chat offers Reconnect when offline", async ({ join, page }) => { + await page.setViewportSize({ width: 390, height: 844 }); + let wire = route(page); + await wire.ready; + await join("ana"); + await page.getByRole("navigation", { name: "Workspace view" }) + .getByRole("button", { name: /^Chat/ }).click(); + let chat = chatPane(page); + + wire.state.offline = true; + await wire.sockets.at(-1)!.close(); + await expect(chat.locator(".composer-connection")).toHaveText("Offline", { timeout: 10_000 }); + let reconnect = chat.getByRole("button", { name: "Reconnect", exact: true }); + await expect(reconnect).toBeVisible(); + + wire.state.offline = false; + await reconnect.click(); + await expect(chat.locator(".composer-connection")).toBeEmpty(); +}); diff --git a/e2e/editing.e2e.ts b/e2e/editing.e2e.ts index a14b9ea8b..c8f17689f 100644 --- a/e2e/editing.e2e.ts +++ b/e2e/editing.e2e.ts @@ -53,7 +53,9 @@ test("losing the connection locks the plan, and getting it back unlocks it", asy * the only way to be the thing that drops it. */ let sockets: WebSocketRoute[] = []; + let offline = false; await page.routeWebSocket("**/ws?**", route => { + if (offline) return route.close(); route.connectToServer(); sockets.push(route); }); @@ -63,21 +65,22 @@ test("losing the connection locks the plan, and getting it back unlocks it", asy await content(page).click(); await page.keyboard.type("Before the wire went."); + offline = true; await sockets.at(-1)!.close(); - // Read-only is the point: an editor that keeps taking keystrokes it cannot - // send is worse than one that stops, because the typing looks like it - // worked right up until the reload that loses it. + // Read-only is the point once the loss outlasts a blip: an editor that + // keeps taking keystrokes it cannot send is worse than one that stops, + // because the typing looks like it worked right up until the reload that + // loses it. await expect(content(page)).toHaveAttribute("contenteditable", "false"); - await expect(page.locator(".plan-status")).toHaveAttribute( - "data-level", - "notice", - ); + await expect(page.locator(".plan[data-plan-offline]")).toHaveCount(1); + await expect(page.locator(".plan-status")).toHaveAttribute("data-level", /^(notice|alert)$/); // The client retries on its own; nothing here reconnects it. Opening is // driven by the connection rather than by the mount, and a socket that // comes back without re-opening the document would leave the editor // unlocked over a plan quietly short of everyone else's edits. + offline = false; await ready(page); expect(sockets.length).toBeGreaterThan(1); await expect(content(page)).toContainText("Before the wire went."); @@ -212,8 +215,12 @@ test("Tab over a selection from a list into a paragraph leaves it alone", async test("a lost connection is said in the document header and the composer", async ({ join, page }) => { let sockets: WebSocketRoute[] = []; let offline = false; + let refused = 0; await page.routeWebSocket("**/ws?**", route => { - if (offline) return route.close(); + if (offline) { + refused++; + return route.close(); + } route.connectToServer(); sockets.push(route); }); @@ -233,23 +240,37 @@ test("a lost connection is said in the document header and the composer", async await expect(status).toContainText("Reconnecting…"); await expect(spoken).toHaveText("Reconnecting…"); await expect(page.locator(".plan[data-plan-offline]")).toHaveCount(1); - await expect(chat.getByText("Connection lost", { exact: true })).toBeVisible(); - await expect(chatInput(chat)).toHaveAttribute("contenteditable", "false"); + // Chat says the same thing in its footer, and keeps the draft editable. + await expect(chat.locator(".composer-connection")).toHaveText("Reconnecting…"); + await expect(chatInput(chat)).toHaveAttribute("contenteditable", "true"); await expect(page.getByRole("button", { name: "Send message" })).toBeDisabled(); // Lost for long enough, it stops promising and offers a way out. await expect(status).toHaveAttribute("data-level", "alert", { timeout: 10_000 }); await expect(spoken).toHaveText("Offline"); - let reload = status.getByRole("button", { name: "Reload" }); - await expect(reload).toBeVisible(); - await expect(reload).toHaveAccessibleDescription(/Editing resumes once connected/); - + await expect(chat.locator(".composer-connection")).toHaveText("Offline"); + // Reconnecting in place keeps a Chat draft a reload would lose. Only after + // it keeps failing does the page offer to reload. + let reconnect = status.getByRole("button", { name: "Reconnect", exact: true }); + await expect(reconnect).toBeVisible(); + await expect(reconnect).toHaveAccessibleDescription(/Editing resumes once connected/); + await expect(status.getByRole("button", { name: "Reload" })).toHaveCount(0); + for (let attempt = 0; attempt < 3; attempt++) { + let before = refused; + await status.getByRole("button", { name: "Reconnect", exact: true }).click(); + await expect.poll(() => refused).toBeGreaterThan(before); + } + await expect(status.getByRole("button", { name: "Reload" })).toBeVisible(); + await expect(status.getByRole("button", { name: "Reconnect", exact: true })).toHaveCount(0); + + // No online event here: the wire's own retry has to bring it back, and + // asking to reconnect restarted the backoff rather than adding to it. offline = false; await ready(page); await expect(status).toHaveAttribute("data-level", "hidden"); await expect(spoken).toHaveText("Reconnected"); await expect(page.locator(".plan[data-plan-offline]")).toHaveCount(0); - await expect(chatInput(chat)).toHaveAttribute("contenteditable", "true"); + await expect(chat.locator(".composer-connection")).toBeEmpty(); }); const PASSAGES = Array.from({ length: 40 }, (_, index) => `Passage ${index + 1}.`).join("\n\n") diff --git a/e2e/smoke.e2e.ts b/e2e/smoke.e2e.ts index 4c3510091..ca253730f 100644 --- a/e2e/smoke.e2e.ts +++ b/e2e/smoke.e2e.ts @@ -414,9 +414,11 @@ test("chat keeps both ends of a tall transcript clear across layouts", async ({ expect(position.messageBottom).toBeLessThanOrEqual(position.scrollerBottom); }); -test("chat disables Send when its socket disconnects", async ({ join, page }) => { +test("chat disables Send once a lost socket outlasts a blip", async ({ join, page }) => { let sockets: WebSocketRoute[] = []; + let offline = false; await page.routeWebSocket("**/ws?**", route => { + if (offline) return route.close(); route.connectToServer(); sockets.push(route); }); @@ -427,8 +429,13 @@ test("chat disables Send when its socket disconnects", async ({ join, page }) => await draft.fill("A draft left during reconnect."); await expect(send).toBeEnabled(); + offline = true; await sockets.at(-1)!.close(); + // Inside the grace period nothing changes; after it, Send waits and the + // draft stays. + await expect(send).toBeEnabled(); await expect(send).toBeDisabled(); + await expectChatValue(draft, "A draft left during reconnect."); }); test("chat routes one Send action by @chopin without blocking room messages or its queue", async ({ join, page }) => { diff --git a/packages/editor/src/connection-notice.test.ts b/packages/editor/src/connection-notice.test.ts new file mode 100644 index 000000000..028dd233c --- /dev/null +++ b/packages/editor/src/connection-notice.test.ts @@ -0,0 +1,17 @@ +import { describe, expect, test } from "bun:test"; + +import { CONNECTION_GRACE, CONNECTION_STALL, connectionNotice } from "./connection-notice"; + +describe("connectionNotice", () => { + test("says nothing while connected or inside the grace period", () => { + expect(connectionNotice(undefined)).toBe("none"); + expect(connectionNotice(0)).toBe("none"); + expect(connectionNotice(CONNECTION_GRACE - 1)).toBe("none"); + }); + + test("reads as reconnecting, then offline once the loss outlasts the stall", () => { + expect(connectionNotice(CONNECTION_GRACE)).toBe("reconnecting"); + expect(connectionNotice(CONNECTION_GRACE + CONNECTION_STALL - 1)).toBe("reconnecting"); + expect(connectionNotice(CONNECTION_GRACE + CONNECTION_STALL)).toBe("offline"); + }); +}); diff --git a/packages/editor/src/connection-notice.ts b/packages/editor/src/connection-notice.ts new file mode 100644 index 000000000..1be01fb67 --- /dev/null +++ b/packages/editor/src/connection-notice.ts @@ -0,0 +1,42 @@ +/** When a lost connection is worth mentioning, shared by every surface that mentions it. */ + +import { useEffect, useState } from "react"; + +/** How long a lost connection stays silent, so a blip that recovers shows nothing. */ +export const CONNECTION_GRACE = 1500; +/** How much longer it reads as reconnecting before it is called offline. */ +export const CONNECTION_STALL = 5000; + +/** + * `none` while connected or inside the grace period, `reconnecting` once the + * loss has outlasted it, and `offline` once it has outlasted the stall too. + */ +export type ConnectionNotice = "none" | "reconnecting" | "offline"; + +/** The notice for a connection that has been lost for `elapsed` milliseconds. */ +export function connectionNotice(elapsed: number | undefined): ConnectionNotice { + if (elapsed === undefined || elapsed < CONNECTION_GRACE) return "none"; + return elapsed < CONNECTION_GRACE + CONNECTION_STALL ? "reconnecting" : "offline"; +} + +/** + * The notice for a connection that is `lost` right now. + * + * Only what is shown waits. Whatever has to act on the real connection, such + * as reopening the document or sending, must keep reading it directly: a + * reconnect inside the grace period never changes this value. + */ +export function useConnectionNotice(lost: boolean): ConnectionNotice { + let [notice, setNotice] = useState("none"); + useEffect(() => { + setNotice("none"); + if (!lost) return; + let timers = [CONNECTION_GRACE, CONNECTION_GRACE + CONNECTION_STALL].map(delay => + setTimeout(() => setNotice(connectionNotice(delay)), delay) + ); + return () => { + for (let timer of timers) clearTimeout(timer); + }; + }, [lost]); + return lost ? notice : "none"; +} diff --git a/packages/editor/src/index.ts b/packages/editor/src/index.ts index 18a601b69..cc418c470 100644 --- a/packages/editor/src/index.ts +++ b/packages/editor/src/index.ts @@ -3,6 +3,8 @@ export type { SidecarCardProps } from "./card"; export { CardMetaStore, useCardMeta } from "./card-meta"; export { collaborationPlugin } from "./collaboration"; export type { CollaborationOptions } from "./collaboration"; +export { CONNECTION_GRACE, CONNECTION_STALL, useConnectionNotice } from "./connection-notice"; +export type { ConnectionNotice } from "./connection-notice"; export { ContentSwapLayer } from "./content-swap"; export type { ContentSwapLayerProps, ContentSwapMotion } from "./content-swap"; export { Count } from "./count"; diff --git a/packages/editor/src/plan-editor.tsx b/packages/editor/src/plan-editor.tsx index 2b4a6e346..888553455 100644 --- a/packages/editor/src/plan-editor.tsx +++ b/packages/editor/src/plan-editor.tsx @@ -18,6 +18,7 @@ import { plugins as dialectPlugins } from "@chopin/dialect"; import { ChangeStore } from "./changes"; import { PlanChanges } from "./changes-chip"; import { collaborationPlugin } from "./collaboration"; +import { useConnectionNotice } from "./connection-notice"; import { PLAN_LEXICAL_THEME } from "./plan-theme"; import { ResearchDraftStore } from "./research-draft"; import { register } from "./widgets"; @@ -104,6 +105,11 @@ export type PlanState = { synced: boolean; /** Why the document was last replaced, if it was. */ reset?: Plan.Reset["reason"]; + /** + * Counts replacements that dropped edits the server never acknowledged, + * so the host can say so once for each, until it is dismissed. + */ + lost?: number; /** Why it could not be opened at all, if it could not. */ failed?: string; }; @@ -151,10 +157,16 @@ export function PlanEditor( // A rotated epoch invalidates the whole local document, so the editor is // rebuilt rather than reconciled — that is what "reset" means. The marks // describe a history that no longer exists, so they go with it. - let onReset = useCallback((reason: Plan.Reset["reason"]) => { + let onReset = useCallback((reason: Plan.Reset["reason"], lost: boolean) => { changes.clear(); questions?.resetDocument(); - setState(prev => ({ ...prev, synced: false, reset: reason, failed: undefined })); + setState(prev => ({ + ...prev, + synced: false, + reset: reason, + failed: undefined, + lost: lost ? (prev.lost ?? 0) + 1 : prev.lost, + })); setGeneration(value => value + 1); }, [changes, questions]); @@ -266,7 +278,9 @@ export function PlanEditor( if (presence) resume(presence); }, [presence, connection]); - let offline = connection !== undefined && connection !== "connected"; + // Locking waits out a blip, and edits made meanwhile wait in the outbox. + let offline = useConnectionNotice(connection !== undefined && connection !== "connected") + !== "none"; let locked = offline || !!busy || !!readOnly || !state.synced; // Empty without a connection, and never used: the editor is not rendered diff --git a/packages/editor/src/provider.test.ts b/packages/editor/src/provider.test.ts index ad7e727ec..05bc5d114 100644 --- a/packages/editor/src/provider.test.ts +++ b/packages/editor/src/provider.test.ts @@ -27,11 +27,13 @@ function emptyUpdate(): string { return btoa(binary); } -function reply(): Plan.Open.Reply { +const EPOCH = "01K0N4TR8K7JGM4R1J7PW4R8YJ"; + +function reply(epoch = EPOCH): Plan.Open.Reply { return { kind: "plan:open", ts: 0, - epoch: "01K0N4TR8K7JGM4R1J7PW4R8YJ", + epoch, seq: 1, update: emptyUpdate(), revision: 1, @@ -53,6 +55,7 @@ function wire(open = true): Transport & { sent: string[]; asked: object[]; up: boolean; + epoch: string; emit: (kind: string, frame: unknown) => void; } { let sent: string[] = []; @@ -63,6 +66,7 @@ function wire(open = true): Transport & { sent, asked, up: open, + epoch: EPOCH, get connected(): boolean { return this.up; }, @@ -82,7 +86,7 @@ function wire(open = true): Transport & { if (!this.up) return Promise.reject(new Error("not connected")); sent.push(kind); asked.push(payload); - if (kind === "plan:open") return Promise.resolve(reply() as T); + if (kind === "plan:open") return Promise.resolve(reply(this.epoch) as T); return Promise.resolve(undefined as T); }, }; @@ -278,6 +282,51 @@ describe("opening the plan", () => { }); }); + /* + * The server rebuilt the document while this client was away, so it never + * heard the reset. Merging the new state into the old document kept edits + * nobody else had, and every later edit was acknowledged but never applied. + */ + it("rebuilds rather than merges when the epoch rotated while it was away", async () => { + let transport = wire(); + let doc = new Y.Doc(); + let resets: Array<[string, boolean]> = []; + let provider = new PlanProvider({ + wire: transport, + doc, + onReset: (reason, lost) => resets.push([reason, lost]), + }); + await provider.connect(); + + transport.up = false; + doc.getText("plan").insert(0, "typed during a blip"); + transport.up = true; + transport.epoch = "01K0N4TR8K7JGM4R1J7PW4R8Z0"; + transport.sent.length = 0; + await provider.resume(); + + expect(resets).toEqual([["replaced", true]]); + // Nothing from the old history is replayed into the new one. + expect(transport.sent).toEqual(["plan:open"]); + expect(provider.synced).toBe(false); + }); + + it("says nothing was lost when a rotated epoch finds the outbox empty", async () => { + let transport = wire(); + let resets: boolean[] = []; + let provider = new PlanProvider({ + wire: transport, + doc: new Y.Doc(), + onReset: (_, lost) => resets.push(lost), + }); + await provider.connect(); + + transport.epoch = "01K0N4TR8K7JGM4R1J7PW4R8Z0"; + await provider.resume(); + + expect(resets).toEqual([false]); + }); + it("does not ask twice while an open is already in flight", async () => { let transport = wire(); let provider = new PlanProvider({ wire: transport, doc: new Y.Doc() }); diff --git a/packages/editor/src/provider.ts b/packages/editor/src/provider.ts index ca951f5d8..b64dfcfb9 100644 --- a/packages/editor/src/provider.ts +++ b/packages/editor/src/provider.ts @@ -43,8 +43,13 @@ function decode(value: string): Uint8Array { export type PlanProviderOptions = { wire: Transport; doc: Y.Doc; - /** Told when the server rotates the epoch and local state must be discarded. */ - onReset?: (reason: Plan.Reset["reason"]) => void; + /** + * Told when the server rotates the epoch and local state must be discarded. + * + * `lost` is true when edits the server never acknowledged went with it, so + * the person can be told rather than finding out later. + */ + onReset?: (reason: Plan.Reset["reason"], lost: boolean) => void; /** * Authoritative snapshot of which prose each decision and comment names. * @@ -347,7 +352,17 @@ export class PlanProvider implements Provider { let reply = await this.#wire.ask("plan:open", { ...resume }); if (!this.#connected || generation !== this.#generation) return; - let rotated = this.#epoch !== undefined && this.#epoch !== reply.epoch; + /* + * The epoch rotated while this client was away, so it never heard the + * reset. Its document and outbox describe a history that no longer + * exists: merging the new state into it keeps edits nobody else has, + * and every later edit is acknowledged but never applies, because it + * builds on them. Rebuild from the server instead, as a reset would. + */ + if (this.#epoch !== undefined && this.#epoch !== reply.epoch) { + this.#discard("replaced"); + return; + } this.#epoch = reply.epoch; // Server state never originates locally, so it must not be echoed back. @@ -357,15 +372,7 @@ export class PlanProvider implements Provider { applyAwarenessUpdate(this.awareness, decode(reply.awareness), this); } - if (rotated) { - this.#outbox.clear(); - this.#outboxBytes = 0; - this.#unsent.clear(); - this.#overdue.clear(); - this.#sends.clear(); - } else { - this.#replay(); - } + this.#replay(); this.#synced = true; this.#emit("status", { status: "connected" }); @@ -555,7 +562,11 @@ export class PlanProvider implements Provider { // The same epoch is the server refusing an oversized update while // keeping the document. Sending it again would be refused again. if (event.epoch === this.#epoch) return this.#park(); + this.#discard(event.reason); + } + #discard(reason: Plan.Reset["reason"]): void { + let lost = this.#outbox.size > 0; this.#outbox.clear(); this.#stopTimers(); this.#generation++; @@ -564,7 +575,7 @@ export class PlanProvider implements Provider { this.#synced = false; this.#emit("sync", false); - this.#options.onReset?.(event.reason); + this.#options.onReset?.(reason, lost); } get synced(): boolean { diff --git a/packages/editor/src/status.test.ts b/packages/editor/src/status.test.ts index 1abf8c521..c59527094 100644 --- a/packages/editor/src/status.test.ts +++ b/packages/editor/src/status.test.ts @@ -36,6 +36,8 @@ describe("describeStatus", () => { expect(status.level).toBe("alert"); expect(status.label).toBe("Offline"); expect(status.reload).toBe(true); + // Reconnecting in place keeps unsent work; a host that can offers it. + expect(status.reconnect).toBe(true); expect(describeStatus({ connection: "connecting", synced: false, stalled: true }).level) .toBe("alert"); }); diff --git a/packages/editor/src/status.tsx b/packages/editor/src/status.tsx index 642eb7249..f196392c3 100644 --- a/packages/editor/src/status.tsx +++ b/packages/editor/src/status.tsx @@ -14,9 +14,14 @@ export type PlanStatusProps = { busy?: boolean; /** Recovers from a terminal state. Reloads the page unless the host supplies one. */ onReload?: () => void; + /** + * Tries the connection again in place. When given, an offline document + * offers this instead of a reload, which would throw away unsent work. + */ + onReconnect?: () => void; }; -export type StatusInput = Omit & { +export type StatusInput = Omit & { /** True once a lost connection has stayed lost long enough to call it offline. */ stalled?: boolean; }; @@ -32,6 +37,8 @@ export type StatusDescription = { label: string; detail?: string; reload?: boolean; + /** A lost connection, which reconnecting in place can recover. */ + reconnect?: boolean; }; /** How long a lost connection stays "Reconnecting" before it reads as offline. */ @@ -62,6 +69,7 @@ export function describeStatus(input: StatusInput): StatusDescription { label: "Offline", detail: "Editing resumes once connected. Reloading may help.", reload: true, + reconnect: true, }; } // The first connection of a fresh page is ordinary loading, not a loss. @@ -116,10 +124,13 @@ function reloadPage() { location.reload(); } -export function PlanStatus({ onReload = reloadPage, ...props }: PlanStatusProps) { +export function PlanStatus({ onReload = reloadPage, onReconnect, ...props }: PlanStatusProps) { let stalled = useStalled(props.connection); let status = describeStatus({ ...props, stalled }); - let { label, level, detail, reload } = status; + let { label, level, reload } = status; + let detail = status.detail; + let reconnect = status.reconnect && onReconnect; + if (reconnect) detail = "Editing resumes once connected."; let detailId = useId(); let previous = useRef(level); let [spoken, setSpoken] = useState(""); @@ -157,10 +168,10 @@ export function PlanStatus({ onReload = reloadPage, ...props }: PlanStatusProps) data-tooltip={detail} data-tooltip-detail="" data-tooltip-verbatim="" - onClick={onReload} + onClick={reconnect || onReload} type="button" > - Reload + {reconnect ? "Reconnect" : "Reload"} )} diff --git a/packages/question/src/react/questionnaire-controller.ts b/packages/question/src/react/questionnaire-controller.ts index d1850743c..2f5c118e0 100644 --- a/packages/question/src/react/questionnaire-controller.ts +++ b/packages/question/src/react/questionnaire-controller.ts @@ -9,6 +9,17 @@ type Model = crdt.Model>; const EMPTY_DRAFTS: Drafts = Object.freeze({}); +/** Said when a request never reached the server, which no retry here can fix. */ +export const OFFLINE = "Not connected. Try again when you're back online."; + +/** True when a request failed for want of a connection rather than being refused. */ +export function unreachable(error: unknown): boolean { + return error instanceof Error + && /^(not connected|connection (lost|restarted)|questionnaire is disconnected)$/.test( + error.message, + ); +} + function normalize(person: { client: string; handle?: string; @@ -264,10 +275,10 @@ export class QuestionnaireController { this.#set({ error: reply.message ?? "Could not submit these answers." }); } }) - .catch(() => { + .catch((error: unknown) => { if (this.#snapshot.closed) return; this.#terminal = false; - this.#set({ error: "Could not submit these answers." }); + this.#set({ error: unreachable(error) ? OFFLINE : "Could not submit these answers." }); }) .finally(() => { if (!this.#terminal) this.#set({ submitting: false }); @@ -292,9 +303,9 @@ export class QuestionnaireController { this.#set({ error: "Could not discard this decision." }); } }) - .catch(() => { + .catch((error: unknown) => { this.#terminal = false; - this.#set({ error: "Could not discard this decision." }); + this.#set({ error: unreachable(error) ? OFFLINE : "Could not discard this decision." }); }) .finally(() => { if (!this.#terminal) this.#set({ submitting: false }); @@ -319,9 +330,9 @@ export class QuestionnaireController { this.#set({ error: "Could not cancel this question." }); } }) - .catch(() => { + .catch((error: unknown) => { this.#terminal = false; - this.#set({ error: "Could not cancel this question." }); + this.#set({ error: unreachable(error) ? OFFLINE : "Could not cancel this question." }); }) .finally(() => { if (!this.#terminal) this.#set({ submitting: false }); diff --git a/packages/question/src/react/unreachable.test.ts b/packages/question/src/react/unreachable.test.ts new file mode 100644 index 000000000..8855d1205 --- /dev/null +++ b/packages/question/src/react/unreachable.test.ts @@ -0,0 +1,16 @@ +import { describe, expect, it } from "bun:test"; + +import { unreachable } from "./questionnaire-controller"; + +describe("unreachable", () => { + it("recognises a request the connection could not carry", () => { + for (let message of ["not connected", "connection lost", "connection restarted"]) { + expect(unreachable(new Error(message))).toBe(true); + } + }); + + it("leaves refusals and other failures to their own copy", () => { + expect(unreachable(new Error("stale"))).toBe(false); + expect(unreachable("not connected")).toBe(false); + }); +}); diff --git a/packages/question/src/react/use-questionnaire-refresh-save.test.ts b/packages/question/src/react/use-questionnaire-refresh-save.test.ts index 5c84972c2..41c6c0147 100644 --- a/packages/question/src/react/use-questionnaire-refresh-save.test.ts +++ b/packages/question/src/react/use-questionnaire-refresh-save.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from "bun:test"; import { addOption as grow, create, decision } from "../index"; import { QuestionnaireController } from "./use-questionnaire"; import type { Transport } from "./use-questionnaire"; +import { OFFLINE } from "./questionnaire-controller"; import { DEFINITION } from "./use-questionnaire.test-fixtures"; // Whole archive 446a9779a937fa5be7cd3eb52fd7f3023d691ed2 callbacks, wrappers only. @@ -128,6 +129,7 @@ describe("QuestionnaireController refresh-save", () => { controller.configure(DEFINITION, false); await Bun.sleep(0); expect(controller.getSnapshot().submitting).toBe(false); - expect(controller.getSnapshot().error).toBe("Could not submit these answers."); + // A dropped connection says so, rather than sounding like a refusal. + expect(controller.getSnapshot().error).toBe(OFFLINE); }); }); diff --git a/scripts/design-contract/exceptions/dynamic-editor.json b/scripts/design-contract/exceptions/dynamic-editor.json index fbea6ece9..a3b6508d5 100644 --- a/scripts/design-contract/exceptions/dynamic-editor.json +++ b/scripts/design-contract/exceptions/dynamic-editor.json @@ -340,7 +340,7 @@ 2 ] ], - "sourceHash": "c939b23c2f50d9a1ac3607af544e93efce6e34be2de43c41ea1cddb4dc62ad49" + "sourceHash": "85e5f52822e9f2c5445e6827f4af8b6e36bca14eadcad82eea617b2db2b0f58a" }, { "file": "packages/editor/src/collaboration.tsx",