-
Notifications
You must be signed in to change notification settings - Fork 2
Add centralized error boundary pattern for dashboard components #705
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
15003c0
034eef5
19061d9
ff8cb7b
c5304aa
8d0464f
1f7fcd4
5f3fa88
19f8729
82f162d
0a15feb
d1f4a9d
7d1d86e
813531d
7379115
ac85071
2c9a9be
9da1885
f39f230
806aafb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,126 @@ | ||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||
| * @module Shared/ErrorBoundary | ||||||||||||||||||||||||||||||||||||||||||||
| * @description Centralized error boundary pattern for browser-side dashboard components. | ||||||||||||||||||||||||||||||||||||||||||||
| * Prevents individual component failures from breaking the entire page by wrapping | ||||||||||||||||||||||||||||||||||||||||||||
| * render functions with error catching, fallback UI, and optional retry logic. | ||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||
| * @intelligence Intelligence platform resilience layer — each dashboard panel is isolated | ||||||||||||||||||||||||||||||||||||||||||||
| * so a single data-source failure never cascades to the rest of the page. Retry logic | ||||||||||||||||||||||||||||||||||||||||||||
| * maximises successful data acquisition from unstable government APIs. | ||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||
| * @business Platform reliability — isolated component failures improve perceived | ||||||||||||||||||||||||||||||||||||||||||||
| * reliability and reduce support incidents. Automatic retry reduces manual page refreshes | ||||||||||||||||||||||||||||||||||||||||||||
| * and keeps users engaged with available data. | ||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||
| * @marketing Enterprise readiness signal — graceful degradation and structured error | ||||||||||||||||||||||||||||||||||||||||||||
| * handling demonstrate production quality to government and enterprise prospects. | ||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| import { logger } from './logger.js'; | ||||||||||||||||||||||||||||||||||||||||||||
| import { renderErrorFallback, renderLoadingFallback } from './fallback-ui.js'; | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||
| * Localised labels for the loading and error states produced by | ||||||||||||||||||||||||||||||||||||||||||||
| * {@link renderWithFallback}. Both fields are optional; English defaults | ||||||||||||||||||||||||||||||||||||||||||||
| * are used when omitted so the API remains backwards-compatible. | ||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||
| export interface RenderWithFallbackOptions { | ||||||||||||||||||||||||||||||||||||||||||||
| /** ARIA label announced by screen readers while the skeleton is visible. */ | ||||||||||||||||||||||||||||||||||||||||||||
| readonly loadingLabel?: string; | ||||||||||||||||||||||||||||||||||||||||||||
| /** Text shown on the retry button in the error card. */ | ||||||||||||||||||||||||||||||||||||||||||||
| readonly retryLabel?: string; | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||
| * Wrap a synchronous or asynchronous render function with an error boundary. | ||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||
| * - Shows a loading skeleton while an async render is in progress. | ||||||||||||||||||||||||||||||||||||||||||||
| * - On success the container is left with whatever the render function produced. | ||||||||||||||||||||||||||||||||||||||||||||
| * - On failure the container shows an error card with an optional retry button. | ||||||||||||||||||||||||||||||||||||||||||||
| * - Each retry re-runs the full renderFn. | ||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||
| * @param container - Target DOM element that will receive the rendered output. | ||||||||||||||||||||||||||||||||||||||||||||
| * @param renderFn - Function (sync or async) that populates `container`. | ||||||||||||||||||||||||||||||||||||||||||||
| * @param fallbackMessage - Human-readable message shown in the error card. | ||||||||||||||||||||||||||||||||||||||||||||
| * @param options - Optional localised labels for the loading/error states. | ||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||
| export async function renderWithFallback( | ||||||||||||||||||||||||||||||||||||||||||||
| container: HTMLElement, | ||||||||||||||||||||||||||||||||||||||||||||
| renderFn: () => void | Promise<void>, | ||||||||||||||||||||||||||||||||||||||||||||
| fallbackMessage = 'Data temporarily unavailable', | ||||||||||||||||||||||||||||||||||||||||||||
| options: RenderWithFallbackOptions = {}, | ||||||||||||||||||||||||||||||||||||||||||||
| ): Promise<void> { | ||||||||||||||||||||||||||||||||||||||||||||
| // Snapshot original markup so retry attempts can restore pre-existing DOM | ||||||||||||||||||||||||||||||||||||||||||||
| // elements (e.g. <canvas> elements) that renderFn depends on. | ||||||||||||||||||||||||||||||||||||||||||||
| // Note: restore uses innerHTML, so child element references held by callers | ||||||||||||||||||||||||||||||||||||||||||||
| // are not preserved across retries — they will point to recreated nodes. | ||||||||||||||||||||||||||||||||||||||||||||
| const originalHTML = container.innerHTML; | ||||||||||||||||||||||||||||||||||||||||||||
| let inFlight = false; | ||||||||||||||||||||||||||||||||||||||||||||
| let isFirstAttempt = true; | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| const attempt = async (): Promise<void> => { | ||||||||||||||||||||||||||||||||||||||||||||
| if (inFlight) { | ||||||||||||||||||||||||||||||||||||||||||||
| // Prevent overlapping attempts that could cause race conditions. | ||||||||||||||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| inFlight = true; | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| // On retry, restore the original markup so any required child elements | ||||||||||||||||||||||||||||||||||||||||||||
| // (e.g. <canvas> targets) are present for the re-render. The first | ||||||||||||||||||||||||||||||||||||||||||||
| // attempt skips this to preserve existing DOM element references held | ||||||||||||||||||||||||||||||||||||||||||||
| // by the caller. | ||||||||||||||||||||||||||||||||||||||||||||
| if (!isFirstAttempt) { | ||||||||||||||||||||||||||||||||||||||||||||
| container.innerHTML = originalHTML; | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
| isFirstAttempt = false; | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| // Append a dedicated loading overlay so the skeleton stays visible while | ||||||||||||||||||||||||||||||||||||||||||||
| // the async render is in progress without destroying required children. | ||||||||||||||||||||||||||||||||||||||||||||
| const loadingOverlay = document.createElement('div'); | ||||||||||||||||||||||||||||||||||||||||||||
| loadingOverlay.setAttribute('data-error-boundary-loading', 'true'); | ||||||||||||||||||||||||||||||||||||||||||||
| loadingOverlay.setAttribute('aria-busy', 'true'); | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
| loadingOverlay.setAttribute('aria-busy', 'true'); | |
| loadingOverlay.setAttribute('aria-busy', 'true'); | |
| loadingOverlay.className = 'error-boundary-loading-overlay'; | |
| // Ensure the container provides a positioning context for the overlay. | |
| const currentPosition = getComputedStyle(container).position; | |
| if (currentPosition === '' || currentPosition === 'static') { | |
| container.style.position = 'relative'; | |
| } | |
| // Style the overlay to cover the container without affecting layout. | |
| loadingOverlay.style.position = 'absolute'; | |
| loadingOverlay.style.top = '0'; | |
| loadingOverlay.style.right = '0'; | |
| loadingOverlay.style.bottom = '0'; | |
| loadingOverlay.style.left = '0'; | |
| loadingOverlay.style.display = 'flex'; | |
| loadingOverlay.style.alignItems = 'center'; | |
| loadingOverlay.style.justifyContent = 'center'; | |
| loadingOverlay.style.zIndex = '1'; | |
| loadingOverlay.style.pointerEvents = 'none'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 2c9a9be. The overlay now sets position: absolute (inset 0) on itself and ensures the container has position: relative when it's static, so the skeleton covers the content area without shifting layout. pointerEvents: none and zIndex: 1 are also applied. The element also gets the class error-boundary-loading-overlay for styling hooks.
Copilot
AI
Mar 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
renderWithFallback mutates container.style.position (sets it to relative when computed position is static) but never restores the previous inline value. This can unintentionally change layout/stacking context for the container after the render completes, even though the loading overlay is removed. Consider capturing the prior inline style.position (and whether you changed it) and restoring it in the finally block (or toggling a CSS class instead of setting inline styles).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in f39f230. priorInlinePosition = container.style.position is captured before the mutation and container.style.position = priorInlinePosition is called in the finally block after the overlay is removed, so the container's stacking context is always restored to whatever it was before renderWithFallback ran.
Copilot
AI
Mar 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
renderWithFallback() always calls renderLoadingFallback(container), which clears the container’s existing DOM. Most dashboards rely on pre-existing markup (e.g. <canvas id="partyEffectivenessChart"> in index*.html) and render functions query by ID rather than recreating elements; if callers pass a container that wraps those elements, the skeleton will delete them and retries/successful renders may silently do nothing. Consider changing the contract so loading/error states overlay without destroying required children, or snapshot/restore the container’s original markup on retry.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 1f7fcd4. renderWithFallback now snapshots container.innerHTML at call time and restores it (container.innerHTML = originalHTML) immediately before invoking renderFn(), so any pre-existing DOM elements (e.g. <canvas> targets) are always present when the render function runs — on the initial attempt and on every retry. Two new tests verify this behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
renderErrorFallback(dashboardSection, ...)clears and replaces the entire#party-dashboardsection. On retry,init()expects existing chart DOM (e.g.,#partyEffectivenessChart,#partyComparisonChart, etc.), but those canvases were removed by the fallback, so the dashboard cannot recover without a page reload. Render the error UI into a dedicated placeholder inside the section (or preserve/restore the original section markup before retry) so a retry can actually re-render charts.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 1f7fcd4. The catch block now creates a
div(errContainer), appends it todashboardSection, and callsrenderErrorFallback(errContainer, …). This leaves the canvas elements untouched. The retry callback removeserrContainerbefore re-callinginit(), so all expected chart targets are present for the retry render.