diff --git a/web/e2e/smoke.spec.js b/web/e2e/smoke.spec.js index f481e6c..7ba54ee 100644 --- a/web/e2e/smoke.spec.js +++ b/web/e2e/smoke.spec.js @@ -565,8 +565,13 @@ test('en opgave kan trækkes ned i en tom sektion, og en fejlet indlæsning sige await expect(section.getByText('brænde kaffe')).toBeVisible(); await expect(page.locator('.toast')).toHaveCount(0); - // A load that fails says so, and offers to try again. - await page.route('**/api/v1/projects/*', (r) => r.abort()); + // A load that fails with a server error says so at once and offers to try again. + // A 500 is the server answering, not a dropped connection, so it is not a blip to + // wait out — it is not retried, and the message is immediate. (A dropped + // connection now retries for a few seconds first; that is the self-heal test.) + await page.route('**/api/v1/projects/*', (r) => + r.fulfill({ status: 500, contentType: 'application/json', body: '{"code":"internal_error"}' }) + ); await page.reload(); await expect(page.getByText('Serveren svarede ikke')).toBeVisible(); await expect(page.getByRole('button', { name: 'Prøv igen' })).toBeVisible(); @@ -577,6 +582,52 @@ test('en opgave kan trækkes ned i en tom sektion, og en fejlet indlæsning sige expect(trouble.filter((t) => !t.includes('projects'))).toEqual([]); }); +/** + * A save made during a short drop is not lost to it. + * + * This is the whole of what "offline" means here: not a cache to work in with no + * server, but a save that rides out a few seconds of no connection and goes through + * when it comes back — with a quiet "reconnecting" while it waits, and no error and + * no reverted change once it heals. A drop that outlasts the retry window is a real + * outage and takes the normal error path; that is not this test. + */ +test('en kort netaudsætning under en handling genoprettes af sig selv', async ({ page }) => { + const trouble = watchForTrouble(page); + await page.goto('/'); + + // Wait for the app to finish loading before cutting the line — `goto` resolves at + // the load event, but the data load runs after hydration, and aborting that would + // test a cold start, not a blip mid-use. + const box = page.getByLabel('Ny opgave'); + await expect(box).toBeVisible(); + + // The app is loaded and online. Now the connection drops — every call fails, the + // way it does in a tunnel, rather than one endpoint erroring. + let down = true; + await page.route('**/api/v1/**', (route) => (down ? route.abort() : route.continue())); + + // A task captured during the drop. Its save fails on the network and starts + // retrying rather than giving up. + await box.fill('under et udfald'); + await box.press('Enter'); + + // The quiet pill, once the retry has lasted long enough to be worth showing. + await expect(page.locator('.netstatus')).toBeVisible(); + + // The drop ends. The retry goes through on its own and the pill leaves — no + // error, nothing toasted as failed. + down = false; + await expect(page.locator('.netstatus')).toHaveCount(0, { timeout: 10000 }); + await expect(page.locator('.toast')).toHaveCount(0); + + // And the save landed: an undated quick capture goes to the Inbox, and there it + // is, saved across the drop rather than lost to it. + await page.getByRole('link', { name: /Indbakke/ }).click(); + await expect(page.getByText('under et udfald')).toBeVisible({ timeout: 10000 }); + + expect(trouble).toEqual([]); +}); + /** * A task can be dragged onto another day, and onto another project. * diff --git a/web/src/lib/api.js b/web/src/lib/api.js index c2ef5f2..b2ae722 100644 --- a/web/src/lib/api.js +++ b/web/src/lib/api.js @@ -23,6 +23,57 @@ export class ApiError extends Error { } import { t } from './i18n.svelte.js'; +import { connectivity, sleepOrOnline } from './connectivity.svelte.js'; + +/** + * How long a request keeps retrying a network failure before giving up. + * + * This is the whole of the "offline" feature and its whole limit: it rides out a + * blip of a few seconds so a save is not lost to it, and then — because the app is + * a live view of a shared list, not a local database — it stops and says so rather + * than holding an edit for a network that is not coming back. Fifteen seconds is + * long enough for a handover or a tunnel and short enough that a real outage fails + * while the user is still looking at what they did. + */ +const RETRY_WINDOW_MS = 15000; + +/** Backoff between attempts: 0.5s, 1s, 2s, then 3s, capped. A waiting sleep also + * wakes the instant the OS reports a link again, so most blips end on the first. */ +const backoff = (attempt) => Math.min(500 * 2 ** attempt, 3000); + +/** + * Runs `send` (a fetch), retrying only network failures for a short window. + * + * A network failure is fetch rejecting — no HTTP status ever arrived. An HTTP + * error (4xx/5xx) is the server answering, so it is returned, not retried: retrying + * a 400 just asks the same wrong question again. An abort is the caller changing + * its mind (a superseded search), so it is left to propagate. + * + * Retrying is not free of risk: a create whose request reached the server but whose + * response was lost would be made twice. For a drop of seconds that is rare, and the + * alternative — losing the save every time the network hiccups — is the thing this + * exists to stop. A server-side idempotency key would remove even that; it is a + * larger change noted for later. + */ +async function withRetry(send, noRetry = false) { + const deadline = Date.now() + (noRetry ? 0 : RETRY_WINDOW_MS); + let attempt = 0; + for (;;) { + try { + const res = await send(); + connectivity.reachable(); + return res; + } catch (e) { + if (e?.name === 'AbortError') throw e; + if (Date.now() >= deadline) { + connectivity.lost(); + throw new ApiError(0, 'offline', 'ingen forbindelse'); + } + connectivity.retrying(); + await sleepOrOnline(backoff(attempt++)); + } + } +} /** * The key for each code the API can return. @@ -110,14 +161,10 @@ async function request(method, path, body, options = {}) { init.body = JSON.stringify(body); } - let res; - try { - res = await fetch(`/api/v1${path}`, init); - } catch (e) { - // A network failure is not an HTTP status; give it one the app can handle - // the same way as everything else. - throw new ApiError(0, 'offline', 'ingen forbindelse'); - } + // Retries a network failure for a few seconds so a short drop does not lose the + // call; `withRetry` turns an exhausted window into the same `offline` error the + // app already handles. `options.noRetry` opts a call out — used by nothing yet. + const res = await withRetry(() => fetch(`/api/v1${path}`, init), options.noRetry); if (res.status === 204) return null; @@ -157,12 +204,9 @@ async function upload(path, file, field = 'file') { const form = new FormData(); form.append(field, file); - let res; - try { - res = await fetch(`/api/v1${path}`, { method: 'POST', body: form, credentials: 'same-origin' }); - } catch { - throw new ApiError(0, 'offline', 'ingen forbindelse'); - } + const res = await withRetry(() => + fetch(`/api/v1${path}`, { method: 'POST', body: form, credentials: 'same-origin' }) + ); const text = await res.text(); let payload = null; diff --git a/web/src/lib/connectivity.svelte.js b/web/src/lib/connectivity.svelte.js new file mode 100644 index 0000000..57d380e --- /dev/null +++ b/web/src/lib/connectivity.svelte.js @@ -0,0 +1,70 @@ +/** + * Connectivity, as the app experiences it — not as the browser reports it. + * + * This is for short drops: a few seconds of lost Wi-Fi, a tunnel, a base station + * handing over. It is deliberately NOT offline support. The app is already loaded + * and its data is already in memory, so there is nothing to cache and nothing to + * open from cold; what a blip needs is for the save you just made to survive it, + * not for the whole app to run without a server. + * + * The request layer drives this: it retries a failed request for a short window + * (see api.js) and tells this store while it is trying, so the interface can say + * "reconnecting" rather than reverting the change under the user's hands. If the + * window runs out it is a real outage, and then the normal error path takes over. + * + * `navigator.onLine` is only a hint — it goes false when the machine has no route, + * but it stays true for a dead gateway or a server that is down, so it cannot be + * the source of truth. It is used here only to wake a waiting retry the moment the + * OS sees a link again, which is the one thing it is reliable for. + */ +import { browser } from '$app/environment'; + +class Connectivity { + /** 'online' — every recent request reached the server. + * 'retrying' — a request failed on the network and is being retried. + * 'lost' — a request gave up after the retry window; a real outage. */ + status = $state('online'); + + /** A request reached the server. The network is up, whatever it was doing. */ + reachable() { + this.status = 'online'; + } + + /** A request failed on the network and is about to be tried again. */ + retrying() { + if (this.status !== 'lost') this.status = 'retrying'; + } + + /** A request used up its retry window without reaching the server. */ + lost() { + this.status = 'lost'; + } +} + +export const connectivity = new Connectivity(); + +if (browser) { + // The OS lost its link: say so at once rather than waiting for the next request + // to fail. It does not, on its own, mean the server is unreachable — but with no + // route there is nothing a save can do but wait, and the honest thing is to show + // it waiting. + window.addEventListener('offline', () => connectivity.retrying()); +} + +/** + * Sleeps for `ms`, but wakes early the moment the OS reports a link again — so a + * retry that is biding its time does not sit out the last two seconds of a blip + * that has already ended. + */ +export function sleepOrOnline(ms) { + return new Promise((resolve) => { + let timer; + const done = () => { + clearTimeout(timer); + if (browser) window.removeEventListener('online', done); + resolve(); + }; + timer = setTimeout(done, ms); + if (browser) window.addEventListener('online', done, { once: true }); + }); +} diff --git a/web/src/lib/locales/da.js b/web/src/lib/locales/da.js index 6cb8867..e65a832 100644 --- a/web/src/lib/locales/da.js +++ b/web/src/lib/locales/da.js @@ -130,6 +130,8 @@ export const da = { 'nav.toggleTheme': 'Skift mellem lyst og mørkt tema', 'nav.offline': 'Offline', 'nav.offlineHint': 'Ændringer fra andre vises ikke lige nu', + 'net.reconnecting': 'Genopretter forbindelse…', + 'net.offline': 'Ingen forbindelse', 'nav.doItToday': 'Gør det i dag', // --- the sidebar's own sections ------------------------------------------------ diff --git a/web/src/lib/locales/en.js b/web/src/lib/locales/en.js index df4eed9..3ca1db1 100644 --- a/web/src/lib/locales/en.js +++ b/web/src/lib/locales/en.js @@ -126,6 +126,8 @@ export const en = { 'nav.toggleTheme': 'Switch between light and dark', 'nav.offline': 'Offline', 'nav.offlineHint': 'Changes from other people are not showing right now', + 'net.reconnecting': 'Reconnecting…', + 'net.offline': 'No connection', 'nav.doItToday': 'Do it today', // --- the sidebar's own sections ------------------------------------------------ diff --git a/web/src/routes/+layout.svelte b/web/src/routes/+layout.svelte index 9afa1e0..f498c4f 100644 --- a/web/src/routes/+layout.svelte +++ b/web/src/routes/+layout.svelte @@ -1,6 +1,7 @@