diff --git a/web/src/api/client.ts b/web/src/api/client.ts index 1fc884d..754edaf 100644 --- a/web/src/api/client.ts +++ b/web/src/api/client.ts @@ -41,6 +41,9 @@ export type ApiRequest = { method?: 'GET' | 'POST' | 'PUT' | 'PATCH' | 'DELETE' // Sent as JSON. A GET must not have one. body?: unknown + // Aborts the request; fetch then rejects with the signal's reason. No + // request has a deadline unless its caller aborts it. + signal?: AbortSignal } // ApiRead is a GET's decoded response body, and whether the service worker @@ -78,7 +81,7 @@ export function createApiClient(location: ApiLocation, network: NetworkStatus, s let forbidden = false const listeners = new Set<() => void>() - async function send(method: string, path: string, body: unknown): Promise { + async function send(method: string, path: string, body: unknown, signal: AbortSignal | undefined): Promise { const headers: Record = { Accept: 'application/json' } if (body !== undefined) headers['Content-Type'] = 'application/json' if (method !== 'GET') { @@ -97,7 +100,7 @@ export function createApiClient(location: ApiLocation, network: NetworkStatus, s } let response: Response try { - response = await fetch(path, { method, headers, credentials: 'same-origin', body: json }) + response = await fetch(path, { method, headers, credentials: 'same-origin', body: json, signal }) } catch (error) { // fetch rejects with a TypeError when the network fails; an abort is a // DOMException and says nothing about the network. @@ -138,14 +141,14 @@ export function createApiClient(location: ApiLocation, network: NetworkStatus, s if (method === 'GET' && request.body !== undefined) { throw new Error(`GET ${path}: a GET request cannot carry a body`) } - let response = await send(method, url, request.body) + let response = await send(method, url, request.body, request.signal) if (response.status === 403) { const error = await parseForbidden(method, path, response) // Only a non-GET carries the token, so only it can be refused for it. if (error.code !== 'csrf' || method === 'GET') throw error // The session's token changed: refresh it once and retry once. await refreshMe() - response = await send(method, url, request.body) + response = await send(method, url, request.body, request.signal) if (response.status === 403) throw await parseForbidden(method, path, response) } return { body: await decode(method, path, response), cached: response.headers.has(cacheFallbackHeader) } diff --git a/web/src/shopping/api.ts b/web/src/shopping/api.ts index e9b5430..fb3fbdb 100644 --- a/web/src/shopping/api.ts +++ b/web/src/shopping/api.ts @@ -7,14 +7,26 @@ import { type MutationObserverOptions, type QueryClient, } from '@tanstack/react-query' -import { useEffect, useReducer, useRef, useSyncExternalStore } from 'react' +import { useEffect, useReducer, useSyncExternalStore } from 'react' import { ApiError, useApi, type ApiClient } from '../api/index.ts' import { isRecord } from '../api/errors.ts' import { meQueryKey } from '../AuthBootstrap.tsx' import { oneOf } from '../dinners/api.ts' import type { NetworkStatus } from '../pwa/networkStatus.ts' import { HOUSEHOLD_SETTINGS_KEY, type HouseholdSettings } from '../settings/api.ts' -import { dequeue, enqueue, keep, queued, SAVED_KEY, saveRun, savedRun, saveSchedule } from './offline.ts' +import { + dequeue, + enqueue, + keep, + markWillSync, + queued, + SAVED_KEY, + saveRun, + savedRun, + saveSchedule, + synced, + willSync, +} from './offline.ts' export const SHOPPING_PATH = '/api/v1/shopping' @@ -215,19 +227,45 @@ export type Toggles = { toggle: (toggle: Toggle) => void } -// reachable resolves once network says the server can be reached: at once -// when it says so now. -function reachable(network: NetworkStatus): Promise { +// TOGGLE_DEADLINE is how long, in milliseconds, one try of a toggle — its +// GET /api/v1/me and its PATCH — may take before it counts as the server not +// answering. Only toggles have a deadline: the API client sets none. +export const TOGGLE_DEADLINE = 10_000 + +// RETRY_FIRST and RETRY_CEILING bound, in milliseconds, how long a toggle +// waits while the server cannot be reached before it is tried again anyway: +// the first wait, doubled after each failed try up to the ceiling. +export const RETRY_FIRST = 1_000 +export const RETRY_CEILING = 30_000 + +// reachable resolves once network says the server can be reached — at once +// when it says so now — or after delay milliseconds, whichever is first. The +// server coming back says nothing by itself, and the browser sends no online +// event when it never thought it was offline, so the toggle is tried again +// after delay whatever network says. +function reachable(network: NetworkStatus, delay: number): Promise { if (!network.offline()) return Promise.resolve() return new Promise((resolve) => { - const stop = network.subscribe(() => { - if (network.offline()) return + const done = () => { + clearTimeout(timer) stop() resolve() + } + const timer = setTimeout(done, delay) + const stop = network.subscribe(() => { + if (!network.offline()) done() }) }) } +// beforeDeadline is promise, rejected with signal's reason once it aborts. +function beforeDeadline(promise: Promise, signal: AbortSignal): Promise { + return new Promise((resolve, reject) => { + signal.addEventListener('abort', () => reject(signal.reason as Error), { once: true }) + promise.then(resolve, reject) + }) +} + // toggleOptions is the mutation every toggle of the week's Supply run is, // made on the Supply run screen (useToggles) or replayed from the queue saved // on this device (useSupplyRunSync). @@ -237,10 +275,16 @@ function reachable(network: NetworkStatus): Promise { // include every toggle before it and none after, and a replayed toggle never // overlaps another replayed or made here. While the server cannot be // reached — the device is offline, or the toggle's PATCH failed with a -// network error (fetch rejected with a TypeError) — a toggle waits, pending, -// and is sent once it can be: those after it wait behind it. Any answer from the server settles -// it: a refusal (a 400, 404, 409 or any other error response) or a response -// that is not the item as toggled fails it, and it is not sent again. Before +// network error (fetch rejected with a TypeError), or a try ran past its +// deadline (TOGGLE_DEADLINE) — a toggle waits, pending, and is tried again +// once network says the server can be reached, or after a wait that doubles +// from RETRY_FIRST up to RETRY_CEILING, whichever is first: those after it +// wait behind it, in order. A try past its deadline is aborted and marks the +// network failed, as a network error does, so the item says it will sync; the +// server may have applied the aborted PATCH, and sending it again is safe for +// the reason a replay is (see useSupplyRunSync). Any answer from the server +// settles it: a refusal (a 400, 404, 409 or any other error response) or a +// response that is not the item as toggled fails it, and it is not sent again. Before // a toggle is sent, GET /api/v1/me has answered, so it carries the session's // CSRF token even when the app opened offline. // @@ -259,20 +303,30 @@ function toggleOptions( networkMode: 'always', retry: false, gcTime: Infinity, - mutationFn: async ({ id, needed }: KeyedToggle): Promise => { + mutationFn: async ({ key, id, needed }: KeyedToggle): Promise => { const path = `${SHOPPING_PATH}/items/${encodeURIComponent(id)}` const where = `PATCH ${path}` let body: unknown - for (;;) { - await reachable(api.network) + for (let failures = 0; ; failures++) { + if (api.network.offline()) markWillSync(key) + await reachable(api.network, Math.min(RETRY_FIRST * 2 ** failures, RETRY_CEILING)) + const deadline = new AbortController() + const timer = setTimeout(() => deadline.abort(new DOMException(`${where}: no answer in time`, 'TimeoutError')), TOGGLE_DEADLINE) try { - await queryClient.ensureQueryData({ queryKey: meQueryKey, queryFn: api.loadMe, networkMode: 'always' }) - body = await api.apiFetch(path, { method: 'PATCH', body: { needed } }) + const me = queryClient.ensureQueryData({ queryKey: meQueryKey, queryFn: api.loadMe, networkMode: 'always' }) + await beforeDeadline(me, deadline.signal) + body = await api.apiFetch(path, { method: 'PATCH', body: { needed }, signal: deadline.signal }) break } catch (error) { + if (deadline.signal.aborted) { + api.network.requestFailed() + continue + } // The API client marks the network offline as fetch rejects; a // TypeError it did not mark is not a network failure. if (!(error instanceof TypeError) || !api.network.offline()) throw error + } finally { + clearTimeout(timer) } } if (!isRecord(body) || body.id !== id || body.needed !== needed) throw new Error(`${where}: response is not item ${id} as toggled`) @@ -307,6 +361,7 @@ function toggleOptions( if ((earlier.state.variables as KeyedToggle).id === id) cache.remove(earlier) } keep(() => dequeue(settled)) + synced(settled) }, } } @@ -319,7 +374,8 @@ function toggleOptions( // once; a refused toggle just stops being pending, which puts both back. // An item whose pending toggle was made, or waited, while the server could // not be reached will sync: it says so until that toggle settles, even once -// the server can be reached again. +// the server can be reached again, and even when the screen is left and +// opened again meanwhile (markWillSync keeps it with the page, not here). // // The Supply run and the toggles are read straight from the query and // mutation caches as each render begins, never from the caches' batched @@ -345,9 +401,6 @@ export function useToggles(weekStart: string): Toggles { useMutationState({ filters: { mutationKey } }) const offline = useSyncExternalStore(api.network.subscribe, api.network.offline) const [, renderAgain] = useReducer((n: number) => n + 1, 0) - // The keys of the pending toggles seen while the server could not be - // reached: every change of the network status renders again. - const waited = useRef>(new Set()) const run = queryClient.getQueryData(key) if (!run) throw new Error(`The week of ${weekStart} has no Supply run read to toggle items of.`) const toggles = queryClient @@ -355,14 +408,16 @@ export function useToggles(weekStart: string): Toggles { .findAll({ mutationKey }) .map((mutation) => ({ status: mutation.state.status, toggle: mutation.state.variables as KeyedToggle, error: mutation.state.error })) const pending = toggles.filter((t) => t.status === 'pending').map((t) => t.toggle) - waited.current = new Set(pending.filter((t) => offline || waited.current.has(t.key)).map((t) => t.key)) + // Every change of the network status renders again, so each pending + // toggle seen while the server cannot be reached is marked. + if (offline) pending.forEach((t) => markWillSync(t.key)) const failed = toggles .filter((t, i) => t.status === 'error' && !toggles.slice(i + 1).some((later) => later.toggle.id === t.toggle.id)) .at(-1) const { mutate } = useMutation(toggleOptions(weekStart, api, queryClient)) return { shown: shown(run, pending), - willSync: new Set(pending.filter((t) => waited.current.has(t.key)).map((t) => t.id)), + willSync: new Set(pending.filter((t) => willSync(t.key)).map((t) => t.id)), failed: failed === undefined ? null : { toggle: { id: failed.toggle.id, needed: failed.toggle.needed }, error: failed.error! }, toggle: ({ id, needed }) => { const queuedToggle = { key: crypto.randomUUID(), weekStart, id, needed } diff --git a/web/src/shopping/offline.ts b/web/src/shopping/offline.ts index 6b1c323..055fb00 100644 --- a/web/src/shopping/offline.ts +++ b/web/src/shopping/offline.ts @@ -106,6 +106,25 @@ export function dequeue(key: string) { write((saved) => ({ ...saved, queue: saved.queue.filter((toggle) => toggle.key !== key) })) } +// The keys of the toggles that will sync: made, or waiting, while the server +// could not be reached, and not settled yet. They live here, with the page, +// as the toggles in the mutation cache do: the Supply run screen is left and +// opened again while a toggle is still pending, and starts with nothing of +// its own. +const syncing = new Set() + +export function markWillSync(key: string) { + syncing.add(key) +} + +export function willSync(key: string): boolean { + return syncing.has(key) +} + +export function synced(key: string) { + syncing.delete(key) +} + // The latest failure to read or write the saved Supply run, which the Supply // run screen shows: a toggle that cannot be kept may be lost on a reload. let problem: Error | null = null diff --git a/web/src/shopping/offlineQueue.test.tsx b/web/src/shopping/offlineQueue.test.tsx index e711612..e6f0f0c 100644 --- a/web/src/shopping/offlineQueue.test.tsx +++ b/web/src/shopping/offlineQueue.test.tsx @@ -7,7 +7,7 @@ import { createNetworkStatus, type NetworkWindow } from '../pwa/networkStatus.ts import { HOUSEHOLD_SETTINGS } from '../test/ApiProviders.tsx' import { errorResponse, fakeLocation, jsonResponse, ME } from '../test/fetchStub.ts' import { mockViewport } from '../test/matchMedia.ts' -import { SupplyRunSync } from './api.ts' +import { RETRY_FIRST, SupplyRunSync, TOGGLE_DEADLINE } from './api.ts' import { SAVED_KEY } from './offline.ts' import { SupplyRunPage } from './SupplyRunPage.tsx' @@ -68,8 +68,8 @@ function goOnline() { // stubServer is the server's Supply runs of this week and next: a PATCH // marks its item and answers with its week's counts, unless refuse answers // it instead; when hold is given, the item is marked at once but the answer -// waits for the promise hold returns. This week's counts as archived once -// archive is called. +// waits for the promise hold returns, or until the request is aborted. This +// week's counts as archived once archive is called. function stubServer(refuse: (id: string) => Response | undefined = () => undefined, hold?: () => Promise) { const needed = new Map(Object.values(WEEKS).flatMap((items) => items.map((item) => [item.id, item.needed] as const))) let status = 'current' @@ -111,8 +111,14 @@ function stubServer(refuse: (id: string) => Response | undefined = () => undefin const answer = jsonResponse(200, { id, needed: body.needed, ...counts(week) }) inFlight++ mostInFlight = Math.max(mostInFlight, inFlight) - await hold?.() - inFlight-- + try { + await new Promise((resolve, reject) => { + init?.signal?.addEventListener('abort', () => reject(init.signal!.reason as Error)) + void (hold?.() ?? Promise.resolve()).then(resolve) + }) + } finally { + inFlight-- + } return answer }) vi.stubGlobal('fetch', fetchMock) @@ -151,20 +157,35 @@ function queue(): string[] { // openPage opens the Supply run screen in a new window, as the app does: // with SupplyRunSync, and with /me loaded unless the server cannot be -// reached. +// reached. leave goes to another screen of the same page, and reopen comes +// back to the Supply run screen. async function openPage() { const api = createApiClient(fakeLocation('/plan/shopping'), createNetworkStatus(openWindow()), createVerifiedSession(localStorage)) if (onLine && reachable) await api.loadMe() - return render( + const queryClient = createQueryClient() + const app = (onScreen: boolean) => ( - + - + {onScreen && } - , + ) + const view = render(app(true)) + return { ...view, leave: () => view.rerender(app(false)), reopen: () => view.rerender(app(true)) } +} + +// reload ends the page open and its timers, as a reload does: a toggle it +// was waiting to try again is never tried, by it, again. +function reload(page: { unmount: () => void }) { + page.unmount() + vi.clearAllTimers() +} + +async function advance(ms: number) { + await act(() => vi.advanceTimersByTimeAsync(ms)) } function checkbox(name: string) { @@ -179,7 +200,9 @@ beforeEach(() => { onLine = true reachable = true mockViewport(390) - vi.useFakeTimers({ toFake: ['Date'] }) + // setTimeout moves with real time, and a test can jump past a deadline or + // a wait (advance). + vi.useFakeTimers({ toFake: ['Date', 'setTimeout', 'clearTimeout'], shouldAdvanceTime: true }) vi.setSystemTime(new Date('2026-10-07T16:00:00Z')) }) @@ -259,7 +282,7 @@ describe('offline toggle queue', () => { expect(queue()).toEqual(['i3 true', 'i1 false']) // Reload while offline: the settings, /me and the list cannot be read. - first.unmount() + reload(first) await openPage() expect(await screen.findByText('2 / 3')).toBeInTheDocument() expect(checkbox('Taco seasoning')).toBeChecked() @@ -360,7 +383,7 @@ describe('offline toggle queue, replay', () => { // The server applied it; the page reloads before its answer arrives. await waitFor(() => expect(answers.waiting()).toBe(1)) expect(queue()).toEqual(['i3 true']) - first.unmount() + reload(first) act(() => answers.release()) await openPage() @@ -394,6 +417,79 @@ describe('offline toggle queue, replay', () => { expect(screen.getByText('1 / 3')).toBeInTheDocument() }) + it('aborts a PATCH that never answers at its deadline, keeps it pending and tries it again before the toggles after it', async () => { + let tries = 0 + // The first PATCH never answers; every later one answers at once. + const server = stubServer(undefined, () => (++tries === 1 ? new Promise(() => {}) : Promise.resolve())) + await openPage() + expect(await screen.findByText('2 / 3')).toBeInTheDocument() + + fireEvent.click(checkbox('Taco seasoning')) + await waitFor(() => expect(patches(server.fetchMock)).toHaveLength(1)) + fireEvent.click(checkbox('Ground beef')) + expect(queue()).toEqual(['i3 true', 'i1 false']) + + // Past the deadline the hung PATCH is aborted; both still wait, in order. + await advance(TOGGLE_DEADLINE) + expect(await within(row('Taco seasoning')).findByText('will sync')).toBeInTheDocument() + expect(within(row('Ground beef')).getByText('will sync')).toBeInTheDocument() + expect(patches(server.fetchMock)).toHaveLength(1) + expect(queue()).toEqual(['i3 true', 'i1 false']) + + await advance(RETRY_FIRST) + await waitFor(() => expect(queue()).toEqual([])) + expect(patches(server.fetchMock)).toEqual([ + `${ITEM('i3')} {"needed":true}`, + `${ITEM('i3')} {"needed":true}`, + `${ITEM('i1')} {"needed":false}`, + ]) + expect(server.mostInFlight()).toBe(1) + await waitFor(() => expect(screen.queryByText('will sync')).not.toBeInTheDocument()) + expect(screen.getByText('2 / 3')).toBeInTheDocument() + expect(screen.queryByRole('alert')).not.toBeInTheDocument() + }) + + it('tries a toggle again once the server is back, with no online event from the browser', async () => { + const { fetchMock } = stubServer() + await openPage() + expect(await screen.findByText('2 / 3')).toBeInTheDocument() + + // The browser says online throughout; the server cannot be reached, then can. + reachable = false + fireEvent.click(checkbox('Corn tortillas')) + expect(await within(row('Corn tortillas')).findByText('will sync')).toBeInTheDocument() + reachable = true + + await advance(RETRY_FIRST) + await waitFor(() => expect(queue()).toEqual([])) + // The first try failed at GET /api/v1/me, before its PATCH. + expect(patches(fetchMock)).toEqual([`${ITEM('i2')} {"needed":false}`]) + await waitFor(() => expect(screen.queryByText('will sync')).not.toBeInTheDocument()) + expect(screen.getByText('1 / 3')).toBeInTheDocument() + }) + + it('keeps "will sync" when the Supply run screen is left and opened again while the replay is pending', async () => { + const answers = held() + stubServer(undefined, answers.hold) + const page = await openPage() + expect(await screen.findByText('2 / 3')).toBeInTheDocument() + goOffline() + fireEvent.click(checkbox('Ground beef')) + expect(within(row('Ground beef')).getByText('will sync')).toBeInTheDocument() + + page.leave() + goOnline() + await waitFor(() => expect(answers.waiting()).toBe(1)) + page.reopen() + expect(await within(row('Ground beef')).findByText('will sync')).toBeInTheDocument() + expect(queue()).toEqual(['i1 false']) + + act(() => answers.release()) + await waitFor(() => expect(screen.queryByText('will sync')).not.toBeInTheDocument()) + expect(queue()).toEqual([]) + expect(screen.getByText('1 / 3')).toBeInTheDocument() + }) + it('says the saved list is unreadable when it is corrupt offline, without rendering again and again', async () => { stubServer() onLine = false