mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-06-19 13:21:46 +00:00
fix(repo): fall back to Dexie when a network read fails (H2) (#1179)
Repos gated reads on raw navigator.onLine and the online branch had no try/catch, so a captive portal or connected-but-no-internet (navigator.onLine lying "true") threw a network error instead of serving the good cached copy — blanking the trip even though Dexie held it. - new onlineThenCache(onlineFn, cacheFn) helper: reads the cache when offline, and on a network-level failure (Axios error with no HTTP response). A genuine HTTP error (4xx/5xx — the server responded) is rethrown so callers still set error state / navigate, not masked by a stale cache. - gates only on navigator.onLine, NOT the connectivity probe: the probe is a coarse global flag and one failed health check would otherwise divert every read to the (possibly empty) cache even when the request would succeed. - every repo list/get read path routed through it (reads only — writes still go through the mutation queue so failures surface) - tests: captive-portal fallback, HTTP-error rethrow, non-Axios rethrow
This commit is contained in:
@@ -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(),
|
||||
}),
|
||||
)
|
||||
},
|
||||
}
|
||||
|
||||
@@ -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(),
|
||||
}),
|
||||
)
|
||||
},
|
||||
}
|
||||
|
||||
+14
-10
@@ -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[],
|
||||
}),
|
||||
)
|
||||
},
|
||||
}
|
||||
|
||||
+12
-10
@@ -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(),
|
||||
}),
|
||||
)
|
||||
},
|
||||
}
|
||||
|
||||
@@ -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<string, unknown> & { name: string }): Promise<{ item: PackingItem }> {
|
||||
|
||||
@@ -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<string, unknown>): 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<string, unknown> & { name: string }): Promise<{ place: Place }> {
|
||||
|
||||
@@ -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(),
|
||||
}),
|
||||
)
|
||||
},
|
||||
}
|
||||
|
||||
+12
-10
@@ -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(),
|
||||
}),
|
||||
)
|
||||
},
|
||||
}
|
||||
|
||||
+31
-22
@@ -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')
|
||||
},
|
||||
)
|
||||
},
|
||||
}
|
||||
|
||||
@@ -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<T>(
|
||||
onlineFn: () => Promise<T>,
|
||||
cacheFn: () => Promise<T>,
|
||||
): Promise<T> {
|
||||
if (!navigator.onLine) return cacheFn()
|
||||
try {
|
||||
return await onlineFn()
|
||||
} catch (err) {
|
||||
if (isNetworkError(err)) return cacheFn()
|
||||
throw err
|
||||
}
|
||||
}
|
||||
@@ -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', () => {
|
||||
|
||||
@@ -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');
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user