From 39b6d9f786ec42a496c89f8d6d70b54bf0798eb7 Mon Sep 17 00:00:00 2001 From: Maurice <61554723+mauriceboe@users.noreply.github.com> Date: Thu, 6 Aug 2026 23:42:57 +0200 Subject: [PATCH] feat(client): catch render errors instead of blanking the page (#1805) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(client): catch render errors instead of blanking the page There were no error boundaries anywhere, so one throw during render unmounted the whole tree and left the user staring at a white page with no way back. ErrorBoundary holds the state in a class — getDerivedStateFromError has no hook equivalent — and renders a function component for the visible half, so the panel can translate normally. TranslationContext isn't exported, so contextType was not an option anyway. Two things it does beyond showing a message. A chunk that 404s after a deploy gets one reload rather than a retry button: React.lazy caches the rejected promise, so retrying re-throws immediately. And the reload happens once per session, or a genuinely missing chunk would spin the tab. The global handlers in main.tsx cover what no boundary reaches — event handlers and async code. They filter axios errors: the interceptor already redirects on 401/403 and there are 600+ toast.error call sites, so reporting again would mean double toasts and, for the redirects, loops. * feat(client): isolate routes, widgets and the shell chrome The route boundary goes on the single line where all 16 protected routes render their content, inside MobileShell rather than around it: the navigation survives, so a broken page is something the user can walk away from instead of a dead end. The mobile --m-* tokens live on the shell's .m-root too, so a fallback outside it would render invisible. key={location.pathname}, not resetKeys — ProtectedRoute is the same component at the same position for every protected route, so otherwise a failure could follow the user across a navigation. The public routes render outside ProtectedRoute and were therefore uncovered, including /login (where people land when everything else failed) and the two anonymous share pages. PublicRoute wraps those seven. The five global widgets and the shell chrome get fallback={null} each: a broken widget should disappear quietly, and if BottomNav dies the user loses the very thing they need to leave the route. * feat(client): isolate the GL maps and the plugin frame Both map wrappers put the boundary outside their Suspense, not inside: Suspense handles a pending promise, a rejected one throws straight past it. Falling back to the Leaflet renderer keeps a usable map rather than showing an error card — the same downgrade the Suspense fallback already does while loading. PluginFrame is third-party code rendering in our tree; a throw there should cost the plugin, not the page. resetKeys={[pluginId]} so switching plugins clears it. The locale loader had no .catch (TranslationContext.tsx). After a deploy the chunk can be gone, which produced an unhandled rejection and left the UI silently on the previous language. It now keeps the strings it has and says so in the console. --- client/src/App.tsx | 57 ++++- .../src/components/Journey/JourneyMapAuto.tsx | 14 +- client/src/components/Map/MapViewAuto.tsx | 12 +- client/src/components/Plugins/PluginFrame.tsx | 58 ++--- .../components/shared/ErrorBoundary.test.tsx | 216 ++++++++++++++++++ .../src/components/shared/ErrorBoundary.tsx | 212 +++++++++++++++++ client/src/i18n/TranslationContext.tsx | 4 + client/src/main.tsx | 15 +- client/src/mobile/MobileShell.tsx | 7 +- client/src/utils/chunkReload.test.ts | 73 ++++++ client/src/utils/chunkReload.ts | 56 +++++ client/src/utils/globalErrorHandlers.test.ts | 78 +++++++ client/src/utils/globalErrorHandlers.ts | 58 +++++ shared/src/i18n/ar/common.ts | 9 + shared/src/i18n/br/common.ts | 9 + shared/src/i18n/ca/common.ts | 9 + shared/src/i18n/cs/common.ts | 9 + shared/src/i18n/de/common.ts | 9 + shared/src/i18n/en/common.ts | 9 + shared/src/i18n/es/common.ts | 9 + shared/src/i18n/fr/common.ts | 9 + shared/src/i18n/gr/common.ts | 9 + shared/src/i18n/hu/common.ts | 9 + shared/src/i18n/id/common.ts | 9 + shared/src/i18n/it/common.ts | 9 + shared/src/i18n/ja/common.ts | 9 + shared/src/i18n/ko/common.ts | 9 + shared/src/i18n/nl/common.ts | 9 + shared/src/i18n/pl/common.ts | 9 + shared/src/i18n/ru/common.ts | 9 + shared/src/i18n/sv/common.ts | 9 + shared/src/i18n/tr/common.ts | 9 + shared/src/i18n/uk/common.ts | 9 + shared/src/i18n/vi/common.ts | 9 + shared/src/i18n/zh-TW/common.ts | 9 + shared/src/i18n/zh/common.ts | 9 + 36 files changed, 1017 insertions(+), 50 deletions(-) create mode 100644 client/src/components/shared/ErrorBoundary.test.tsx create mode 100644 client/src/components/shared/ErrorBoundary.tsx create mode 100644 client/src/utils/chunkReload.test.ts create mode 100644 client/src/utils/chunkReload.ts create mode 100644 client/src/utils/globalErrorHandlers.test.ts create mode 100644 client/src/utils/globalErrorHandlers.ts diff --git a/client/src/App.tsx b/client/src/App.tsx index 4e01bb20c..e415eef28 100644 --- a/client/src/App.tsx +++ b/client/src/App.tsx @@ -30,6 +30,7 @@ import SaveToCollectionModal from './components/Collections/SaveToCollectionModa import MSaveToCollectionSheet from './components/Collections/MSaveToCollectionSheet' import BackgroundTasksWidget from './components/BackgroundTasks/BackgroundTasksWidget' import MobileShell from './mobile/MobileShell' +import ErrorBoundary from './components/shared/ErrorBoundary' import { useIsPhone } from './mobile/useIsPhone' import { TranslationProvider, useTranslation } from './i18n' import { authApi } from './api/client' @@ -94,7 +95,39 @@ function ProtectedRoute({ children, adminRequired = false, addonId }: ProtectedR // sheets, toasts); from 768px up the legacy wrapper stays untouched. The // shell branches internally so pages keep their state when the viewport // crosses the breakpoint. - return {children} + // The boundary sits inside the shell so a broken page keeps the navigation the + // user needs to leave it — outside, the route would be a dead end, and the + // mobile --m-* tokens live on the shell's .m-root anyway. + // + // key, not resetKeys: ProtectedRoute is the same component at the same position + // for all 16 protected routes, so without it React could keep the failed + // instance across a navigation and the error would follow the user around. + return ( + + + {children} + + + ) +} + +/** + * The public routes render outside ProtectedRoute, so the route boundary above + * never sees them — including /login, where someone lands when everything else + * failed, and the two anonymous share pages. + */ +function PublicRoute({ children }: { children: React.ReactNode }) { + const location = useLocation() + return ( + + {children} + + ) } function RootRedirect() { @@ -210,21 +243,21 @@ export default function App() { return ( - {!isAuthPage && } - - {!isAuthPage && } + {!isAuthPage && } + + {!isAuthPage && } {!isAuthPage && (isPhone ? : )} - + } /> - } /> - } /> - } /> - } /> - } /> - } /> + } /> + } /> + } /> + } /> + } /> + } /> {/* OAuth 2.1 consent page — intentionally outside ProtectedRoute */} - } /> + } /> (function JourneyM if (useGL) { return ( - - {/* eslint-disable-next-line @typescript-eslint/no-explicit-any */} - - + // See MapViewAuto: the boundary has to sit outside the Suspense to catch a + // chunk that never arrives. + // eslint-disable-next-line @typescript-eslint/no-explicit-any + }> + + {/* eslint-disable-next-line @typescript-eslint/no-explicit-any */} + + + ) } // eslint-disable-next-line @typescript-eslint/no-explicit-any diff --git a/client/src/components/Map/MapViewAuto.tsx b/client/src/components/Map/MapViewAuto.tsx index e6fdc05c4..fba1d69c2 100644 --- a/client/src/components/Map/MapViewAuto.tsx +++ b/client/src/components/Map/MapViewAuto.tsx @@ -1,6 +1,7 @@ import { lazy, Suspense } from 'react' import { useSettingsStore } from '../../store/settingsStore' import { MapView } from './MapView' +import ErrorBoundary from '../shared/ErrorBoundary' // MapLibre/Mapbox pull in a ~230 KB (gzip) GL engine. Lazy-load the GL renderer so // Leaflet-only installs never download it — it ships only once a GL provider is picked. @@ -27,9 +28,14 @@ export function MapViewAuto(props: any) { // Render the previous Leaflet map as the fallback so there's no blank flash // while the GL chunk loads on first use. return ( - }> - - + // Outside the Suspense on purpose: Suspense handles the pending promise, + // a rejected one (chunk gone after a deploy) throws past it. Falling back + // to Leaflet keeps a usable map instead of an error card. + }> + }> + + + ) } return diff --git a/client/src/components/Plugins/PluginFrame.tsx b/client/src/components/Plugins/PluginFrame.tsx index 56a62a5e0..45df856ed 100644 --- a/client/src/components/Plugins/PluginFrame.tsx +++ b/client/src/components/Plugins/PluginFrame.tsx @@ -1,6 +1,7 @@ import { useCallback, useEffect, useRef, useState } from 'react' import { useNavigate } from 'react-router' import { useTranslation } from '../../i18n' +import ErrorBoundary from '../shared/ErrorBoundary' import { useAuthStore } from '../../store/authStore' import { useSettingsStore } from '../../store/settingsStore' import { useTripStore } from '../../store/tripStore' @@ -582,32 +583,35 @@ export default function PluginFrame({ pluginId, tripId = null, placeId = null, d const confirmTitle = confirmReq?.title ? `${pluginLabel} — ${confirmReq.title}` : pluginLabel return ( - <> -