diff --git a/client/src/repo/accommodationRepo.ts b/client/src/repo/accommodationRepo.ts index 75e8c345..93cb0550 100644 --- a/client/src/repo/accommodationRepo.ts +++ b/client/src/repo/accommodationRepo.ts @@ -1,16 +1,20 @@ import { accommodationsApi } from '../api/client' import { offlineDb, upsertAccommodations } from '../db/offlineDb' +import { onlineThenCache } from './withOfflineFallback' import type { Accommodation } from '../types' export const accommodationRepo = { async list(tripId: number | string): Promise<{ accommodations: Accommodation[] }> { - if (!navigator.onLine) { - const accommodations = await offlineDb.accommodations - .where('trip_id').equals(Number(tripId)).toArray() - return { accommodations } - } - const result = await accommodationsApi.list(tripId) - upsertAccommodations(result.accommodations || []).catch(() => {}) - return result + return onlineThenCache( + async () => { + const result = await accommodationsApi.list(tripId) + upsertAccommodations(result.accommodations || []).catch(() => {}) + return result + }, + async () => ({ + accommodations: await offlineDb.accommodations + .where('trip_id').equals(Number(tripId)).toArray(), + }), + ) }, } diff --git a/client/src/repo/budgetRepo.ts b/client/src/repo/budgetRepo.ts index 3ea50a7b..c2f0fa31 100644 --- a/client/src/repo/budgetRepo.ts +++ b/client/src/repo/budgetRepo.ts @@ -1,18 +1,20 @@ import { budgetApi } from '../api/client' import { offlineDb, upsertBudgetItems } from '../db/offlineDb' +import { onlineThenCache } from './withOfflineFallback' import type { BudgetItem } from '../types' export const budgetRepo = { async list(tripId: number | string): Promise<{ items: BudgetItem[] }> { - if (!navigator.onLine) { - const cached = await offlineDb.budgetItems - .where('trip_id') - .equals(Number(tripId)) - .toArray() - return { items: cached } - } - const result = await budgetApi.list(tripId) - upsertBudgetItems(result.items) - return result + return onlineThenCache( + async () => { + const result = await budgetApi.list(tripId) + upsertBudgetItems(result.items) + return result + }, + async () => ({ + items: await offlineDb.budgetItems + .where('trip_id').equals(Number(tripId)).toArray(), + }), + ) }, } diff --git a/client/src/repo/dayRepo.ts b/client/src/repo/dayRepo.ts index de105748..0f6eaffb 100644 --- a/client/src/repo/dayRepo.ts +++ b/client/src/repo/dayRepo.ts @@ -1,18 +1,22 @@ import { daysApi } from '../api/client' import { offlineDb, upsertDays } from '../db/offlineDb' +import { onlineThenCache } from './withOfflineFallback' import type { Day } from '../types' export const dayRepo = { async list(tripId: number | string): Promise<{ days: Day[] }> { - if (!navigator.onLine) { - const cached = await offlineDb.days - .where('trip_id') - .equals(Number(tripId)) - .sortBy('day_number' as keyof Day) - return { days: cached as Day[] } - } - const result = await daysApi.list(tripId) - upsertDays(result.days) - return result + return onlineThenCache( + async () => { + const result = await daysApi.list(tripId) + upsertDays(result.days) + return result + }, + async () => ({ + days: (await offlineDb.days + .where('trip_id') + .equals(Number(tripId)) + .sortBy('day_number' as keyof Day)) as Day[], + }), + ) }, } diff --git a/client/src/repo/fileRepo.ts b/client/src/repo/fileRepo.ts index db96bad8..8c75c863 100644 --- a/client/src/repo/fileRepo.ts +++ b/client/src/repo/fileRepo.ts @@ -1,18 +1,20 @@ import { filesApi } from '../api/client' import { offlineDb, upsertTripFiles } from '../db/offlineDb' +import { onlineThenCache } from './withOfflineFallback' import type { TripFile } from '../types' export const fileRepo = { async list(tripId: number | string): Promise<{ files: TripFile[] }> { - if (!navigator.onLine) { - const cached = await offlineDb.tripFiles - .where('trip_id') - .equals(Number(tripId)) - .toArray() - return { files: cached } - } - const result = await filesApi.list(tripId) - upsertTripFiles(result.files) - return result + return onlineThenCache( + async () => { + const result = await filesApi.list(tripId) + upsertTripFiles(result.files) + return result + }, + async () => ({ + files: await offlineDb.tripFiles + .where('trip_id').equals(Number(tripId)).toArray(), + }), + ) }, } diff --git a/client/src/repo/packingRepo.ts b/client/src/repo/packingRepo.ts index 955f7dae..36adc2fd 100644 --- a/client/src/repo/packingRepo.ts +++ b/client/src/repo/packingRepo.ts @@ -1,20 +1,22 @@ import { packingApi } from '../api/client' import { offlineDb, upsertPackingItems } from '../db/offlineDb' import { mutationQueue, generateUUID, nextTempId } from '../sync/mutationQueue' +import { onlineThenCache } from './withOfflineFallback' import type { PackingItem } from '../types' export const packingRepo = { async list(tripId: number | string): Promise<{ items: PackingItem[] }> { - if (!navigator.onLine) { - const cached = await offlineDb.packingItems - .where('trip_id') - .equals(Number(tripId)) - .toArray() - return { items: cached } - } - const result = await packingApi.list(tripId) - upsertPackingItems(result.items) - return result + return onlineThenCache( + async () => { + const result = await packingApi.list(tripId) + upsertPackingItems(result.items) + return result + }, + async () => ({ + items: await offlineDb.packingItems + .where('trip_id').equals(Number(tripId)).toArray(), + }), + ) }, async create(tripId: number | string, data: Record & { name: string }): Promise<{ item: PackingItem }> { diff --git a/client/src/repo/placeRepo.ts b/client/src/repo/placeRepo.ts index a81088a6..752f0ad4 100644 --- a/client/src/repo/placeRepo.ts +++ b/client/src/repo/placeRepo.ts @@ -1,20 +1,22 @@ import { placesApi } from '../api/client' import { offlineDb, upsertPlaces } from '../db/offlineDb' import { mutationQueue, generateUUID, nextTempId } from '../sync/mutationQueue' +import { onlineThenCache } from './withOfflineFallback' import type { Place } from '../types' export const placeRepo = { async list(tripId: number | string, params?: Record): Promise<{ places: Place[] }> { - if (!navigator.onLine) { - const cached = await offlineDb.places - .where('trip_id') - .equals(Number(tripId)) - .toArray() - return { places: cached } - } - const result = await placesApi.list(tripId, params) - upsertPlaces(result.places) - return result + return onlineThenCache( + async () => { + const result = await placesApi.list(tripId, params) + upsertPlaces(result.places) + return result + }, + async () => ({ + places: await offlineDb.places + .where('trip_id').equals(Number(tripId)).toArray(), + }), + ) }, async create(tripId: number | string, data: Record & { name: string }): Promise<{ place: Place }> { diff --git a/client/src/repo/reservationRepo.ts b/client/src/repo/reservationRepo.ts index 575b8075..0394adb1 100644 --- a/client/src/repo/reservationRepo.ts +++ b/client/src/repo/reservationRepo.ts @@ -1,18 +1,20 @@ import { reservationsApi } from '../api/client' import { offlineDb, upsertReservations } from '../db/offlineDb' +import { onlineThenCache } from './withOfflineFallback' import type { Reservation } from '../types' export const reservationRepo = { async list(tripId: number | string): Promise<{ reservations: Reservation[] }> { - if (!navigator.onLine) { - const cached = await offlineDb.reservations - .where('trip_id') - .equals(Number(tripId)) - .toArray() - return { reservations: cached } - } - const result = await reservationsApi.list(tripId) - upsertReservations(result.reservations) - return result + return onlineThenCache( + async () => { + const result = await reservationsApi.list(tripId) + upsertReservations(result.reservations) + return result + }, + async () => ({ + reservations: await offlineDb.reservations + .where('trip_id').equals(Number(tripId)).toArray(), + }), + ) }, } diff --git a/client/src/repo/todoRepo.ts b/client/src/repo/todoRepo.ts index e284b23a..60b47aff 100644 --- a/client/src/repo/todoRepo.ts +++ b/client/src/repo/todoRepo.ts @@ -1,18 +1,20 @@ import { todoApi } from '../api/client' import { offlineDb, upsertTodoItems } from '../db/offlineDb' +import { onlineThenCache } from './withOfflineFallback' import type { TodoItem } from '../types' export const todoRepo = { async list(tripId: number | string): Promise<{ items: TodoItem[] }> { - if (!navigator.onLine) { - const cached = await offlineDb.todoItems - .where('trip_id') - .equals(Number(tripId)) - .toArray() - return { items: cached } - } - const result = await todoApi.list(tripId) - upsertTodoItems(result.items) - return result + return onlineThenCache( + async () => { + const result = await todoApi.list(tripId) + upsertTodoItems(result.items) + return result + }, + async () => ({ + items: await offlineDb.todoItems + .where('trip_id').equals(Number(tripId)).toArray(), + }), + ) }, } diff --git a/client/src/repo/tripRepo.ts b/client/src/repo/tripRepo.ts index 082e346a..e3337dd0 100644 --- a/client/src/repo/tripRepo.ts +++ b/client/src/repo/tripRepo.ts @@ -1,33 +1,42 @@ import { tripsApi } from '../api/client' import { offlineDb, upsertTrip } from '../db/offlineDb' +import { onlineThenCache } from './withOfflineFallback' import type { Trip } from '../types' export const tripRepo = { async list(): Promise<{ trips: Trip[]; archivedTrips: Trip[] }> { - if (!navigator.onLine) { - const all = await offlineDb.trips.toArray() - return { - trips: all.filter(t => !t.is_archived), - archivedTrips: all.filter(t => t.is_archived), - } - } - const [active, archived] = await Promise.all([ - tripsApi.list(), - tripsApi.list({ archived: 1 }), - ]) - active.trips.forEach(t => upsertTrip(t)) - archived.trips.forEach(t => upsertTrip(t)) - return { trips: active.trips, archivedTrips: archived.trips } + return onlineThenCache( + async () => { + const [active, archived] = await Promise.all([ + tripsApi.list(), + tripsApi.list({ archived: 1 }), + ]) + active.trips.forEach(t => upsertTrip(t)) + archived.trips.forEach(t => upsertTrip(t)) + return { trips: active.trips, archivedTrips: archived.trips } + }, + async () => { + const all = await offlineDb.trips.toArray() + return { + trips: all.filter(t => !t.is_archived), + archivedTrips: all.filter(t => t.is_archived), + } + }, + ) }, async get(tripId: number | string): Promise<{ trip: Trip }> { - if (!navigator.onLine) { - const cached = await offlineDb.trips.get(Number(tripId)) - if (cached) return { trip: cached } - throw new Error('No cached trip data available offline') - } - const result = await tripsApi.get(tripId) - upsertTrip(result.trip) - return result + return onlineThenCache( + async () => { + const result = await tripsApi.get(tripId) + upsertTrip(result.trip) + return result + }, + async () => { + const cached = await offlineDb.trips.get(Number(tripId)) + if (cached) return { trip: cached } + throw new Error('No cached trip data available offline') + }, + ) }, } diff --git a/client/src/repo/withOfflineFallback.ts b/client/src/repo/withOfflineFallback.ts new file mode 100644 index 00000000..fba7a063 --- /dev/null +++ b/client/src/repo/withOfflineFallback.ts @@ -0,0 +1,48 @@ +/** + * True when an error means the request never reached the server — a network-level + * failure (offline, captive portal, proxy auth wall, dropped connection, CORS). + * Axios sets `response` only when the server actually replied; its absence (on an + * Axios error) means we never got one. A real HTTP error (4xx/5xx) HAS a response + * and must NOT be treated as a network failure — the server spoke, so the caller + * needs to see it. Non-Axios errors are surfaced too. + */ +function isNetworkError(err: unknown): boolean { + const e = err as { isAxiosError?: boolean; response?: unknown } | null + return !!e && e.isAxiosError === true && e.response == null +} + +/** + * Read-through cache pattern shared by every repo's read methods. + * + * Reads degrade to the local Dexie cache in two situations: + * 1. The browser reports it is offline (`navigator.onLine` false) — skip the + * doomed request entirely. + * 2. The browser *thinks* it is online but the request fails at the network + * level — a lying `navigator.onLine` on a captive portal, a dropped + * connection (H2). Rather than surfacing that (which blanks the trip even + * though a good cached copy exists), we fall back to the cache. + * + * We intentionally gate only on `navigator.onLine`, NOT the connectivity probe: + * the probe is a coarse global flag, and a single failed health check would + * otherwise force every read to the (possibly empty) cache even when the request + * itself would succeed. The network-error catch below covers the captive-portal + * case the probe was meant to. + * + * A genuine HTTP error (404/403/500 — the server responded) is NOT swallowed: it + * is rethrown so callers can set error state, navigate away, etc. + * + * Writes must NOT use this — they go through the mutation queue so failures are + * surfaced and retried, not silently swallowed. + */ +export async function onlineThenCache( + onlineFn: () => Promise, + cacheFn: () => Promise, +): Promise { + if (!navigator.onLine) return cacheFn() + try { + return await onlineFn() + } catch (err) { + if (isNetworkError(err)) return cacheFn() + throw err + } +} diff --git a/client/tests/unit/repo/placeRepo.test.ts b/client/tests/unit/repo/placeRepo.test.ts index 45387841..4724ca56 100644 --- a/client/tests/unit/repo/placeRepo.test.ts +++ b/client/tests/unit/repo/placeRepo.test.ts @@ -64,6 +64,20 @@ describe('placeRepo.list', () => { const result = await placeRepo.list(99); expect(result.places).toHaveLength(0); }); + + it('online but request fails — falls back to Dexie cache (captive portal)', async () => { + // navigator.onLine lies "true" on a captive portal; the request throws. + const place = buildPlace({ trip_id: 1 }); + await offlineDb.places.put(place); + + server.use( + http.get('/api/trips/1/places', () => HttpResponse.error()), + ); + + const result = await placeRepo.list(1); + expect(result.places).toHaveLength(1); + expect(result.places[0].id).toBe(place.id); + }); }); describe('placeRepo.create', () => { diff --git a/client/tests/unit/repo/withOfflineFallback.test.ts b/client/tests/unit/repo/withOfflineFallback.test.ts new file mode 100644 index 00000000..6a679df2 --- /dev/null +++ b/client/tests/unit/repo/withOfflineFallback.test.ts @@ -0,0 +1,76 @@ +/** + * onlineThenCache — the read-through fallback shared by every repo (H2). + * + * Branches: + * - navigator offline → cache only (skip the request) + * - online but the request fails at the network level → fall back to cache + * - online but the server returns an HTTP error → rethrow (don't mask) + * - online and the request succeeds → return it, skip cache + */ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { onlineThenCache } from '../../../src/repo/withOfflineFallback'; + +beforeEach(() => { + Object.defineProperty(navigator, 'onLine', { value: true, writable: true, configurable: true }); +}); + +afterEach(() => { + vi.restoreAllMocks(); +}); + +describe('onlineThenCache', () => { + it('returns the online result when online', async () => { + const online = vi.fn().mockResolvedValue('online'); + const cache = vi.fn().mockResolvedValue('cache'); + + expect(await onlineThenCache(online, cache)).toBe('online'); + expect(online).toHaveBeenCalledOnce(); + expect(cache).not.toHaveBeenCalled(); + }); + + it('reads the cache without calling online when navigator is offline', async () => { + Object.defineProperty(navigator, 'onLine', { value: false }); + const online = vi.fn().mockResolvedValue('online'); + const cache = vi.fn().mockResolvedValue('cache'); + + expect(await onlineThenCache(online, cache)).toBe('cache'); + expect(online).not.toHaveBeenCalled(); + }); + + it('falls back to the cache on a network-level failure (no HTTP response)', async () => { + // Axios network error: the request never reached the server (captive portal). + const netErr = Object.assign(new Error('Network Error'), { isAxiosError: true, response: undefined }); + const online = vi.fn().mockRejectedValue(netErr); + const cache = vi.fn().mockResolvedValue('cache'); + + expect(await onlineThenCache(online, cache)).toBe('cache'); + expect(online).toHaveBeenCalledOnce(); + expect(cache).toHaveBeenCalledOnce(); + }); + + it('rethrows a genuine HTTP error (server responded) instead of masking it', async () => { + // 404/403/500 mean the server replied — callers must see it, not a stale cache. + const httpErr = Object.assign(new Error('Not Found'), { isAxiosError: true, response: { status: 404 } }); + const online = vi.fn().mockRejectedValue(httpErr); + const cache = vi.fn().mockResolvedValue('cache'); + + await expect(onlineThenCache(online, cache)).rejects.toThrow('Not Found'); + expect(cache).not.toHaveBeenCalled(); + }); + + it('rethrows a non-Axios error rather than swallowing it', async () => { + const online = vi.fn().mockRejectedValue(new Error('bug')); + const cache = vi.fn().mockResolvedValue('cache'); + + await expect(onlineThenCache(online, cache)).rejects.toThrow('bug'); + expect(cache).not.toHaveBeenCalled(); + }); + + it('propagates a cache error (e.g. nothing cached) when online also failed', async () => { + Object.defineProperty(navigator, 'onLine', { value: false }); + const online = vi.fn().mockResolvedValue('online'); + const cache = vi.fn().mockRejectedValue(new Error('No cached data')); + + await expect(onlineThenCache(online, cache)).rejects.toThrow('No cached data'); + }); +});