EDM-5267: Mitigate errors when UI service restarts - #770
Conversation
Made-with: Cursor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Enterprise Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review. WalkthroughThe standalone router now recreates lazy route components and remounts route error boundaries when the location changes. The error alert also provides a translated action that reloads the browser page. ChangesNavigation Error Recovery
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This change adds recovery behavior for stale UI assets and navigation after a service restart; no actionable merge-blocking risk remains based on the supplied evidence. Suggested labels: 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
asmasarw
left a comment
There was a problem hiding this comment.
🔴 resettableLazy.tsx re-creates the lazy component whenever location.key changes:
In react-router v6, location.key is regenerated on every history transition (push and replace), not only when moving between pages.
Consequences confirmed:
- Detail pages remount on every tab switch. DeviceDetailsPage (and Fleet/Repository/ImageBuild/AuthProvider details) render internal under :id/*. Switching tabs changes location.key → the whole page remounts (refetch + lost tab state + spinner).
- List pages remount when a URL-synced filter is applied. DevicesPage commits filters via setSearchParams (push) → new location.key → remount: search input loses focus mid-typing, pagination resets, full refetch. This also undercuts the pagination work in #664.
(Navigations run in a React transition, so the spinner may not always visibly flash — but the remount, state loss, and refetch still happen.)
Suggested fixes (any one)
-
Simplest: drop the per-navigation re-import and rely on the ErrorBoundary "Reload page" button for recovery — a full reload already pulls fresh assets.
-
Retry-on-error: wrap the dynamic import() to catch a chunk-load failure and re-import (optionally cache-busting), so a fresh lazy is created only when a load actually fails:
const load = () => importer().catch(() => importer());
const Component = React.useMemo(() => React.lazy(load), []);- If reset-on-nav is truly desired, key it on the matched route/module (something stable per page), not location.key.
Ocassionally, the UI could show a "failed to load chunk" error instead of the selected page.
This seemed to happen when the UI service is restarted, as the loaded UI does not have the correct bundled assets.
Fixes:
location.keyso that when navigating to another tab after the UI has fully restarted (or back to the same tab), the page is shown instead of the stale error.Affected areas
apps/standalone/: Re-imports route components when the route changes. KeysErrorBoundarywithlocation.keyto clear stale errors after navigation or a UI service restart.libs/ui-components/: AddslazyResetterand a “Reload page” action for chunk-loading errors.Impact
libs/types/,libs/i18n/,libs/cypress/,apps/ocp-plugin/,proxy/,packaging/, or CI configuration.