From 68338fb4ae4a798cd4bd95863d11a941770c30e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89mile=20R=C3=A9?= Date: Wed, 15 Jul 2026 11:20:15 +0200 Subject: [PATCH] Address PR review comments on error boundaries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Guard ErrorBoundary against a falsy thrown value: track a separate hasError flag and accept the caught value as unknown, so throwing null, 0, or "" still renders the fallback instead of looping back into the failing subtree. Add role="alert" to InlineError and ErrorState so assistive tech announces errors that appear dynamically after a fetch or query failure. Fix the RecentUpdatesSection doc example, which showed onRetry={reset} with @throwOnFieldError even though reset cannot clear a field error; use a reload and point to the retry table. Signed-off-by: Émile Ré --- contrib/claude/error-handling.md | 38 ++++++++++++------- .../ui/src/v2/ErrorBoundary/ErrorBoundary.tsx | 27 +++++++------ packages/ui/src/v2/ErrorState/ErrorState.tsx | 2 +- .../ui/src/v2/InlineError/InlineError.tsx | 2 +- 4 files changed, 41 insertions(+), 28 deletions(-) diff --git a/contrib/claude/error-handling.md b/contrib/claude/error-handling.md index 5d75bbf06..d4ebfe007 100644 --- a/contrib/claude/error-handling.md +++ b/contrib/claude/error-handling.md @@ -33,34 +33,37 @@ import { Component, type ErrorInfo, type ReactNode } from "react"; export interface ErrorBoundaryProps { children: ReactNode; - // A node, or a render function that receives the caught error + a reset fn. - fallback?: ReactNode | ((error: Error, reset: () => void) => ReactNode); - onError?: (error: Error, info: ErrorInfo) => void; + // A node, or a render function that receives the caught value + a reset fn. + // `unknown` because anything can be thrown, not just an Error. + fallback?: ReactNode | ((error: unknown, reset: () => void) => ReactNode); + onError?: (error: unknown, info: ErrorInfo) => void; } interface ErrorBoundaryState { - error: Error | null; + // Tracked separately from `error` so a falsy thrown value (null, 0, "") still + // renders the fallback instead of looping back into the failing subtree. + hasError: boolean; + error: unknown; } export class ErrorBoundary extends Component { - state: ErrorBoundaryState = { error: null }; + state: ErrorBoundaryState = { hasError: false, error: null }; - static getDerivedStateFromError(error: Error): ErrorBoundaryState { - return { error }; + static getDerivedStateFromError(error: unknown): ErrorBoundaryState { + return { hasError: true, error }; } - componentDidCatch(error: Error, info: ErrorInfo) { + componentDidCatch(error: unknown, info: ErrorInfo) { this.props.onError?.(error, info); } - reset = () => this.setState({ error: null }); + reset = () => this.setState({ hasError: false, error: null }); render() { - const { error } = this.state; - if (error) { + if (this.state.hasError) { const { fallback } = this.props; if (typeof fallback === "function") { - return fallback(error, this.reset); + return fallback(this.state.error, this.reset); } return fallback ?? null; } @@ -188,8 +191,15 @@ export function RecentUpdatesSection({ trustCenterKey }: Props) { const { t } = useTranslation(); return ( ( - + // This section reads a field of the preloaded page query — there is no + // local refetch, so recover with a reload, not the boundary's `reset` + // (see "Retrying: reset vs refetch vs reload" below). + fallback={( + window.location.reload()} + /> )} > diff --git a/packages/ui/src/v2/ErrorBoundary/ErrorBoundary.tsx b/packages/ui/src/v2/ErrorBoundary/ErrorBoundary.tsx index c0dfde27f..e764e70df 100644 --- a/packages/ui/src/v2/ErrorBoundary/ErrorBoundary.tsx +++ b/packages/ui/src/v2/ErrorBoundary/ErrorBoundary.tsx @@ -16,37 +16,40 @@ import { Component, type ErrorInfo, type ReactNode } from "react"; export interface ErrorBoundaryProps { children: ReactNode; - // A node, or a render function that receives the caught error + a reset fn. - fallback?: ReactNode | ((error: Error, reset: () => void) => ReactNode); - onError?: (error: Error, info: ErrorInfo) => void; + // A node, or a render function that receives the caught value + a reset fn. + // The value is `unknown` because anything can be thrown, not just an Error. + fallback?: ReactNode | ((error: unknown, reset: () => void) => ReactNode); + onError?: (error: unknown, info: ErrorInfo) => void; } interface ErrorBoundaryState { - error: Error | null; + // Tracked separately from `error` so a falsy thrown value (null, 0, "") still + // renders the fallback instead of looping back into the failing subtree. + hasError: boolean; + error: unknown; } // The single reusable error boundary primitive (the sanctioned use of a class). // Generic — works at bootstrap, route, section, or component level; only the // placement and the `fallback` differ. See contrib/claude/error-handling.md. export class ErrorBoundary extends Component { - state: ErrorBoundaryState = { error: null }; + state: ErrorBoundaryState = { hasError: false, error: null }; - static getDerivedStateFromError(error: Error): ErrorBoundaryState { - return { error }; + static getDerivedStateFromError(error: unknown): ErrorBoundaryState { + return { hasError: true, error }; } - componentDidCatch(error: Error, info: ErrorInfo) { + componentDidCatch(error: unknown, info: ErrorInfo) { this.props.onError?.(error, info); } - reset = () => this.setState({ error: null }); + reset = () => this.setState({ hasError: false, error: null }); render() { - const { error } = this.state; - if (error) { + if (this.state.hasError) { const { fallback } = this.props; if (typeof fallback === "function") { - return fallback(error, this.reset); + return fallback(this.state.error, this.reset); } return fallback ?? null; } diff --git a/packages/ui/src/v2/ErrorState/ErrorState.tsx b/packages/ui/src/v2/ErrorState/ErrorState.tsx index b8055be00..9c9b123c7 100644 --- a/packages/ui/src/v2/ErrorState/ErrorState.tsx +++ b/packages/ui/src/v2/ErrorState/ErrorState.tsx @@ -39,7 +39,7 @@ export function ErrorState({ code, title, description, actions, fullPage, classN const slots = errorState({ fullPage }); return ( -
+
{code && ( diff --git a/packages/ui/src/v2/InlineError/InlineError.tsx b/packages/ui/src/v2/InlineError/InlineError.tsx index 9cab52030..33b85fea5 100644 --- a/packages/ui/src/v2/InlineError/InlineError.tsx +++ b/packages/ui/src/v2/InlineError/InlineError.tsx @@ -35,7 +35,7 @@ export function InlineError({ layout, message, onRetry, retryLabel = "Retry", cl const slots = inlineError({ layout }); return ( -
+
{message} {onRetry && (