From 43173e2b333024595504214b2b34cde6b48683c1 Mon Sep 17 00:00:00 2001 From: Maurice Date: Sun, 31 May 2026 17:15:53 +0200 Subject: [PATCH] Surface silent store failures to the user and validate API responses in dev Reservation toggle, todo/packing toggle and budget reorder were swallowing API errors after rolling back, so the user saw the change silently snap back with no explanation. Route those failures through the existing toast channel (new store/notify.ts bridges to window.__addToast, the same channel SystemNoticeBanner uses); the reservation toggle re-throws so ReservationsPanel's own translated toast finally fires. Also wire the existing parseInDev/checkInDev response validation into the maps and notification-test endpoints to catch contract drift in dev. --- client/src/api/client.ts | 38 ++++++++++++++----- client/src/store/notify.ts | 17 +++++++++ client/src/store/slices/budgetSlice.ts | 12 ++++-- client/src/store/slices/packingSlice.ts | 6 ++- client/src/store/slices/reservationsSlice.ts | 5 ++- client/src/store/slices/todoSlice.ts | 6 ++- .../unit/slices/reservationsSlice.test.ts | 7 ++-- 7 files changed, 73 insertions(+), 18 deletions(-) create mode 100644 client/src/store/notify.ts diff --git a/client/src/api/client.ts b/client/src/api/client.ts index 2d8b6ae3..d66c2503 100644 --- a/client/src/api/client.ts +++ b/client/src/api/client.ts @@ -4,6 +4,9 @@ import { weatherResultSchema, type WeatherResult, inAppListResultSchema, type InAppListResult, unreadCountResultSchema, type UnreadCountResult, + channelTestResultSchema, + mapsSearchResultSchema, mapsAutocompleteResultSchema, mapsPlaceDetailsResultSchema, + mapsPlacePhotoResultSchema, mapsReverseResultSchema, mapsResolveUrlResultSchema, type NotificationRespondRequest, type SettingUpsertRequest, type SettingsBulkRequest, type JourneyCreateRequest, type JourneyAddTripRequest, @@ -57,6 +60,23 @@ export function parseInDev(schema: S, data: unknown, lab } return data as z.infer } + +/** + * Same dev-only drift check as parseInDev, but passes the payload straight + * through with its original inferred type instead of the schema type. Use this + * for endpoints whose existing consumers rely on the loose `r.data` type — it + * adds the development contract-drift warning without retyping the public + * surface (so it can never break a consumer that worked before). + */ +function checkInDev(schema: z.ZodTypeAny, data: T, label: string): T { + if (API_DEV) { + const result = schema.safeParse(data) + if (!result.success) { + console.warn(`[api] ${label}: response did not match the @trek/shared schema`, result.error.issues) + } + } + return data +} const RATE_LIMIT_MESSAGES: Record = { en: 'Too many attempts. Please try again later.', de: 'Zu viele Versuche. Bitte versuchen Sie es später erneut.', @@ -507,13 +527,13 @@ export const journeyApi = { } export const mapsApi = { - search: (query: string, lang?: string) => apiClient.post(`/maps/search?lang=${lang || 'en'}`, { query }).then(r => r.data), + search: (query: string, lang?: string) => apiClient.post(`/maps/search?lang=${lang || 'en'}`, { query }).then(r => checkInDev(mapsSearchResultSchema, r.data, 'maps.search')), autocomplete: (input: string, lang?: string, locationBias?: { low: { lat: number; lng: number }; high: { lat: number; lng: number } }, signal?: AbortSignal) => - apiClient.post('/maps/autocomplete', { input, lang, locationBias }, { signal }).then(r => r.data), - details: (placeId: string, lang?: string) => apiClient.get(`/maps/details/${encodeURIComponent(placeId)}`, { params: { lang } }).then(r => r.data), - placePhoto: (placeId: string, lat?: number, lng?: number, name?: string) => apiClient.get(`/maps/place-photo/${encodeURIComponent(placeId)}`, { params: { lat, lng, name } }).then(r => r.data), - reverse: (lat: number, lng: number, lang?: string) => apiClient.get('/maps/reverse', { params: { lat, lng, lang } }).then(r => r.data), - resolveUrl: (url: string) => apiClient.post('/maps/resolve-url', { url }).then(r => r.data), + apiClient.post('/maps/autocomplete', { input, lang, locationBias }, { signal }).then(r => checkInDev(mapsAutocompleteResultSchema, r.data, 'maps.autocomplete')), + details: (placeId: string, lang?: string) => apiClient.get(`/maps/details/${encodeURIComponent(placeId)}`, { params: { lang } }).then(r => checkInDev(mapsPlaceDetailsResultSchema, r.data, 'maps.details')), + placePhoto: (placeId: string, lat?: number, lng?: number, name?: string) => apiClient.get(`/maps/place-photo/${encodeURIComponent(placeId)}`, { params: { lat, lng, name } }).then(r => checkInDev(mapsPlacePhotoResultSchema, r.data, 'maps.placePhoto')), + reverse: (lat: number, lng: number, lang?: string) => apiClient.get('/maps/reverse', { params: { lat, lng, lang } }).then(r => checkInDev(mapsReverseResultSchema, r.data, 'maps.reverse')), + resolveUrl: (url: string) => apiClient.post('/maps/resolve-url', { url }).then(r => checkInDev(mapsResolveUrlResultSchema, r.data, 'maps.resolveUrl')), } export const airportsApi = { @@ -651,9 +671,9 @@ export const shareApi = { export const notificationsApi = { getPreferences: () => apiClient.get('/notifications/preferences').then(r => r.data), updatePreferences: (prefs: Record>) => apiClient.put('/notifications/preferences', prefs).then(r => r.data), - testSmtp: (email?: string) => apiClient.post('/notifications/test-smtp', { email }).then(r => r.data), - testWebhook: (url?: string) => apiClient.post('/notifications/test-webhook', { url }).then(r => r.data), - testNtfy: (payload: { topic?: string; server?: string | null; token?: string | null }) => apiClient.post('/notifications/test-ntfy', payload).then(r => r.data), + testSmtp: (email?: string) => apiClient.post('/notifications/test-smtp', { email }).then(r => checkInDev(channelTestResultSchema, r.data, 'notifications.testSmtp')), + testWebhook: (url?: string) => apiClient.post('/notifications/test-webhook', { url }).then(r => checkInDev(channelTestResultSchema, r.data, 'notifications.testWebhook')), + testNtfy: (payload: { topic?: string; server?: string | null; token?: string | null }) => apiClient.post('/notifications/test-ntfy', payload).then(r => checkInDev(channelTestResultSchema, r.data, 'notifications.testNtfy')), } export const inAppNotificationsApi = { diff --git a/client/src/store/notify.ts b/client/src/store/notify.ts new file mode 100644 index 00000000..82938049 --- /dev/null +++ b/client/src/store/notify.ts @@ -0,0 +1,17 @@ +// Bridge for surfacing user-facing toasts from non-component code (Zustand +// slices, store actions) where the `useToast` hook isn't available. Mirrors the +// global `window.__addToast` channel that ToastContainer registers and that +// SystemNoticeBanner already uses for the same reason. + +type NotifyType = 'success' | 'error' | 'warning' | 'info' + +/** + * Show a toast from outside the React tree. No-ops gracefully if the + * ToastContainer hasn't registered its handler yet (e.g. very early boot), + * so callers never have to guard for it. + */ +export function notify(message: string, type: NotifyType = 'info', duration?: number): void { + if (typeof window !== 'undefined' && typeof window.__addToast === 'function') { + window.__addToast(message, type, duration) + } +} diff --git a/client/src/store/slices/budgetSlice.ts b/client/src/store/slices/budgetSlice.ts index b57c4fb9..40dd2189 100644 --- a/client/src/store/slices/budgetSlice.ts +++ b/client/src/store/slices/budgetSlice.ts @@ -5,6 +5,7 @@ import type { TripStoreState } from '../tripStore' import type { BudgetItem, BudgetItemMember } from '../../types' import type { BudgetCreateItemRequest, BudgetUpdateItemRequest } from '@trek/shared' import { getApiErrorMessage } from '../../types' +import { notify } from '../notify' type SetState = StoreApi['setState'] type GetState = StoreApi['getState'] @@ -104,10 +105,12 @@ export const createBudgetSlice = (set: SetState, get: GetState): BudgetSlice => }) try { await budgetApi.reorderItems(tripId, orderedIds) - } catch { - // Reload on failure + } catch (err: unknown) { + // Reload on failure to restore the server's ordering, and tell the user + // their reorder didn't stick (the caller fires this without awaiting). const data = await budgetApi.list(tripId) set({ budgetItems: data.items }) + notify(getApiErrorMessage(err, 'Error reordering budget items'), 'error') } }, @@ -132,9 +135,12 @@ export const createBudgetSlice = (set: SetState, get: GetState): BudgetSlice => }) try { await budgetApi.reorderCategories(tripId, orderedCategories) - } catch { + } catch (err: unknown) { + // Reload on failure to restore the server's ordering, and tell the user + // their reorder didn't stick (the caller fires this without awaiting). const data = await budgetApi.list(tripId) set({ budgetItems: data.items }) + notify(getApiErrorMessage(err, 'Error reordering budget items'), 'error') } }, }) diff --git a/client/src/store/slices/packingSlice.ts b/client/src/store/slices/packingSlice.ts index ab3f373b..2bd10957 100644 --- a/client/src/store/slices/packingSlice.ts +++ b/client/src/store/slices/packingSlice.ts @@ -3,6 +3,7 @@ import type { StoreApi } from 'zustand' import type { TripStoreState } from '../tripStore' import type { PackingItem } from '../../types' import { getApiErrorMessage } from '../../types' +import { notify } from '../notify' type SetState = StoreApi['setState'] type GetState = StoreApi['getState'] @@ -56,12 +57,15 @@ export const createPackingSlice = (set: SetState, get: GetState): PackingSlice = })) try { await packingRepo.update(tripId, id, { checked }) - } catch { + } catch (err: unknown) { + // The caller fires this optimistically and doesn't await, so rolling back + // silently would just flip the checkbox with no explanation. Surface it. set(state => ({ packingItems: state.packingItems.map(item => item.id === id ? { ...item, checked: checked ? 0 : 1 } : item ) })) + notify(getApiErrorMessage(err, 'Error updating item'), 'error') } }, }) diff --git a/client/src/store/slices/reservationsSlice.ts b/client/src/store/slices/reservationsSlice.ts index c020a593..96414c9a 100644 --- a/client/src/store/slices/reservationsSlice.ts +++ b/client/src/store/slices/reservationsSlice.ts @@ -58,8 +58,11 @@ export const createReservationsSlice = (set: SetState, get: GetState): Reservati })) try { await reservationsApi.update(tripId, id, { status: newStatus }) - } catch { + } catch (err: unknown) { + // Roll back the optimistic toggle and surface the failure so the caller's + // catch can notify the user — without it the status silently snaps back. set({ reservations: prev }) + throw new Error(getApiErrorMessage(err, 'Error updating reservation')) } }, diff --git a/client/src/store/slices/todoSlice.ts b/client/src/store/slices/todoSlice.ts index 58070a85..ce545fa5 100644 --- a/client/src/store/slices/todoSlice.ts +++ b/client/src/store/slices/todoSlice.ts @@ -3,6 +3,7 @@ import type { StoreApi } from 'zustand' import type { TripStoreState } from '../tripStore' import type { TodoItem } from '../../types' import { getApiErrorMessage } from '../../types' +import { notify } from '../notify' type SetState = StoreApi['setState'] type GetState = StoreApi['getState'] @@ -56,12 +57,15 @@ export const createTodoSlice = (set: SetState, get: GetState): TodoSlice => ({ })) try { await todoApi.update(tripId, id, { checked }) - } catch { + } catch (err: unknown) { + // The caller fires this optimistically and doesn't await, so rolling back + // silently would just flip the checkbox with no explanation. Surface it. set(state => ({ todoItems: state.todoItems.map(item => item.id === id ? { ...item, checked: checked ? 0 : 1 } : item ) })) + notify(getApiErrorMessage(err, 'Error updating todo'), 'error') } }, }) diff --git a/client/tests/unit/slices/reservationsSlice.test.ts b/client/tests/unit/slices/reservationsSlice.test.ts index eea9810d..a95b91b7 100644 --- a/client/tests/unit/slices/reservationsSlice.test.ts +++ b/client/tests/unit/slices/reservationsSlice.test.ts @@ -123,7 +123,7 @@ describe('reservationsSlice', () => { expect(useTripStore.getState().reservations[0].status).toBe('confirmed'); }); - it('FE-RESERV-007: toggleReservationStatus rolls back on API failure (silent)', async () => { + it('FE-RESERV-007: toggleReservationStatus rolls back and surfaces the error on API failure', async () => { const reservation = buildReservation({ id: 10, trip_id: 1, status: 'confirmed' }); seedStore(useTripStore, { reservations: [reservation] }); @@ -133,8 +133,9 @@ describe('reservationsSlice', () => { ), ); - // Does NOT throw (silent rollback) - await useTripStore.getState().toggleReservationStatus(1, 10); + // Rolls back the optimistic toggle AND rejects, so the caller's catch can + // show a toast (previously the failure was swallowed and the toast never fired). + await expect(useTripStore.getState().toggleReservationStatus(1, 10)).rejects.toThrow(); expect(useTripStore.getState().reservations[0].status).toBe('confirmed'); });