Add Comprehensive React Error Boundary System (Global, Route, Component-Level) #121
Conversation
WalkthroughThis PR implements a comprehensive error boundary system for the React frontend. It introduces reusable error boundary components with centralized error logging, moves React type declarations from devDependencies to dependencies, wraps all application routes with error protection, and integrates global error handling at the app initialization level. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Route as Route/Component
participant EB as ErrorBoundary
participant Logger as ErrorLogger
participant LocalStorage as localStorage
participant RemoteAPI as /api/errors
User->>Route: Navigate/Interact
alt Error Occurs
Route->>EB: Throws Error
EB->>EB: getDerivedStateFromError()
EB->>EB: componentDidCatch()
EB->>Logger: logError(error, componentStack)
Logger->>Logger: buildErrorDetails()
Logger->>LocalStorage: storeErrorLocally()
LocalStorage-->>Logger: Save (last 50)
Logger->>RemoteAPI: sendToRemoteLogger()
alt Online
RemoteAPI-->>Logger: Success/Failure
else Offline
Logger->>Logger: Queue error
Note over Logger: Flush on reconnect
end
EB->>EB: Render Fallback UI
EB-->>User: Show Error + Retry Button
User->>EB: Click Retry
EB->>EB: resetErrorBoundary()
EB->>Route: Re-render children
else No Error
Route-->>EB: Render normally
EB-->>User: Display content
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
frontend/src/App.tsx (1)
54-232: Consider reducing nesting with a route configuration pattern.While wrapping each route with
RouteErrorBoundaryprovides good isolation, the current implementation creates significant nesting (3-4 levels deep) and repetition. Consider extracting route definitions into a configuration object and mapping them to reduce boilerplate.Example pattern:
// Route configuration const routes = [ { path: '/', element: Home, protected: false }, { path: '/auth', element: Authentication, protected: false }, { path: '/startDebate', element: StartDebate, protected: true }, // ... other routes ]; // Helper to wrap routes const renderRoute = (config) => ( <Route key={config.path} path={config.path} element={ <RouteErrorBoundary> <config.element /> </RouteErrorBoundary> } /> );This approach would reduce code duplication and improve maintainability while preserving the same error boundary isolation.
frontend/src/components/ErrorBoundary/ErrorFallback.tsx (1)
55-88: Consider adding development error details for consistency.Unlike
DefaultErrorFallback, this component doesn't show error details in development mode. For debugging route-specific errors, consider adding a similar development-only details section.Add after the buttons div (around line 84):
{process.env.NODE_ENV === 'development' && error && ( <details className="mt-4 text-left"> <summary className="cursor-pointer text-xs text-muted-foreground hover:text-foreground flex items-center gap-2"> <Bug className="h-3 w-3" /> Error Details (Development) </summary> <pre className="mt-2 text-xs bg-muted p-2 rounded overflow-auto max-h-32"> {error.message} {error.stack && `\n\n${error.stack}`} </pre> </details> )}frontend/src/components/ErrorBoundary/withErrorBoundary.tsx (1)
43-48: Redundant fallback specification.The
withComponentErrorBoundaryHOC explicitly setsfallback: ComponentErrorFallback, which is already the default value (line 18). This can be simplified.export const withComponentErrorBoundary = <P extends object>( Component: React.ComponentType<P> ) => withErrorBoundary(Component, { - fallback: ComponentErrorFallback, displayName: `withComponentErrorBoundary(${Component.displayName || Component.name})`, });Alternatively, keep it explicit for clarity if that's the team's preference.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (12)
frontend/package.json(1 hunks)frontend/src/App.tsx(2 hunks)frontend/src/components/ErrorBoundary/ErrorBoundary.tsx(1 hunks)frontend/src/components/ErrorBoundary/ErrorFallback.tsx(1 hunks)frontend/src/components/ErrorBoundary/ErrorLogger.ts(1 hunks)frontend/src/components/ErrorBoundary/RouteErrorBoundary.tsx(1 hunks)frontend/src/components/ErrorBoundary/index.ts(1 hunks)frontend/src/components/ErrorBoundary/types.ts(1 hunks)frontend/src/components/ErrorBoundary/withErrorBoundary.tsx(1 hunks)frontend/src/main.tsx(1 hunks)frontend/tsconfig.app.tsbuildinfo(0 hunks)frontend/tsconfig.node.tsbuildinfo(0 hunks)
💤 Files with no reviewable changes (2)
- frontend/tsconfig.node.tsbuildinfo
- frontend/tsconfig.app.tsbuildinfo
🧰 Additional context used
🧬 Code graph analysis (6)
frontend/src/components/ErrorBoundary/ErrorLogger.ts (1)
frontend/src/components/ErrorBoundary/types.ts (1)
ErrorDetails(27-35)
frontend/src/components/ErrorBoundary/RouteErrorBoundary.tsx (3)
frontend/src/components/ErrorBoundary/types.ts (1)
ErrorBoundaryProps(13-19)frontend/src/components/ErrorBoundary/ErrorFallback.tsx (1)
RouteErrorFallback(55-88)frontend/src/components/ErrorBoundary/index.ts (1)
RouteErrorFallback(6-6)
frontend/src/components/ErrorBoundary/withErrorBoundary.tsx (2)
frontend/src/components/ErrorBoundary/types.ts (1)
ErrorBoundaryProps(13-19)frontend/src/components/ErrorBoundary/ErrorFallback.tsx (1)
ComponentErrorFallback(90-122)
frontend/src/components/ErrorBoundary/ErrorFallback.tsx (2)
frontend/src/components/ErrorBoundary/index.ts (3)
DefaultErrorFallback(5-5)RouteErrorFallback(6-6)ComponentErrorFallback(7-7)frontend/src/components/ErrorBoundary/types.ts (1)
ErrorFallbackProps(21-25)
frontend/src/components/ErrorBoundary/ErrorBoundary.tsx (2)
frontend/src/components/ErrorBoundary/types.ts (3)
ErrorBoundaryProps(13-19)ErrorBoundaryState(7-11)ErrorInfo(3-5)frontend/src/components/ErrorBoundary/ErrorFallback.tsx (1)
DefaultErrorFallback(4-53)
frontend/src/App.tsx (2)
frontend/src/components/ErrorBoundary/RouteErrorBoundary.tsx (1)
RouteErrorBoundary(10-20)frontend/src/Pages/ViewDebate.tsx (1)
ViewDebate(31-522)
🔇 Additional comments (20)
frontend/package.json (1)
26-26: Move@types/reactand@types/react-domback todevDependencies.TypeScript type packages (
@types/*) should remain indevDependencies, notdependencies. These packages are used only during development for type checking; TypeScript removes all type information during compilation, and the runtime output contains no type metadata. Bundlers do not include these packages in the production bundle.Unless the error boundary components have a specific production runtime requirement for these types (which is highly unlikely), reverting this change aligns with TypeScript ecosystem best practices.
Can you clarify the rationale for moving these to production dependencies? If there's no specific runtime requirement, they should be reverted to
devDependencies.Also applies to: 28-28
frontend/src/main.tsx (1)
7-16: LGTM! Clean global error boundary integration.The ErrorBoundary wrapping at the root level is correctly positioned inside StrictMode and outside BrowserRouter, ensuring all routing and app errors are caught before the app crashes.
frontend/src/components/ErrorBoundary/RouteErrorBoundary.tsx (1)
10-20: LGTM! Clean wrapper implementation.The component correctly composes
ErrorBoundarywith sensible defaults and proper prop forwarding.frontend/src/components/ErrorBoundary/ErrorBoundary.tsx (6)
10-20: LGTM! Proper initialization.Constructor correctly initializes state and obtains the ErrorLogger singleton instance.
22-27: LGTM! Standard error boundary pattern.
getDerivedStateFromErrorcorrectly updates state to trigger fallback rendering.
29-42: LGTM! Proper error handling.The component correctly logs errors, updates state, and invokes the optional error handler callback.
65-69: LGTM! Proper cleanup.Timeout is correctly cleared on unmount to prevent memory leaks.
71-83: LGTM! Clean reset implementation.The method properly clears pending timeouts and resets error state.
85-100: LGTM! Standard error boundary render pattern.Correctly renders either the fallback UI with appropriate props or the children when no error exists.
frontend/src/components/ErrorBoundary/ErrorFallback.tsx (3)
29-35: Verify the hard navigation is intentional.Line 30 uses
window.location.href = '/'which triggers a full page reload, clearing all React state and losing React Router's client-side navigation. For a global error boundary, this might be intentional to ensure a clean reset after critical errors.If preserving app state is desired, consider using React Router's navigation:
import { useNavigate } from 'react-router-dom'; // Convert to a function component or use a wrapper export function DefaultErrorFallback({ error, resetError }: ErrorFallbackProps) { const navigate = useNavigate(); // ... <button onClick={() => navigate('/')}> Go Home </button> }However, for critical errors, a hard reload may be the safer choice.
38-49: LGTM! Appropriate development-only debugging.Error details are correctly shown only in development mode, providing useful debugging information without exposing sensitive details in production.
90-122: LGTM! Appropriate compact fallback for component errors.The minimal design is well-suited for inline component errors, and the development debug info provides essential information without overwhelming the UI.
frontend/src/components/ErrorBoundary/index.ts (1)
1-14: LGTM! Clean barrel export pattern.The index file is well-structured and provides a clear public API for the ErrorBoundary feature. All exports are properly organized and follow standard barrel export conventions.
frontend/src/components/ErrorBoundary/types.ts (1)
1-35: LGTM! Well-designed type definitions.The type definitions are comprehensive and appropriate for the error boundary system. All interfaces are correctly structured with proper typing.
frontend/src/components/ErrorBoundary/ErrorLogger.ts (4)
3-18: LGTM! Singleton pattern correctly implemented.The singleton pattern is properly implemented with a private constructor and online/offline event listeners. The automatic queue flushing when the network comes back online is a good UX touch.
27-56: LGTM! Comprehensive error logging.The logError method properly handles error details construction, development logging, local storage, and conditional remote logging based on network status.
58-66: LGTM! Proper local storage with size limit.Good use of
slice(-50)to cap the stored errors and prevent unbounded growth. The try-catch ensures graceful degradation if localStorage is unavailable.
99-114: LGTM! Proper error handling for localStorage operations.Both methods include appropriate try-catch blocks with fallback behavior, ensuring the logger remains resilient even if localStorage operations fail.
frontend/src/components/ErrorBoundary/withErrorBoundary.tsx (2)
6-11: LGTM! Appropriate type definitions.The type definitions properly extend ErrorBoundaryProps and accommodate both regular and forwardRef components.
23-26: Type safety concern withas anycast is valid, but alternatives need verification.The
as anycast on line 26 is correctly identified as reducing type safety. However, this HOC differs from other components in the codebase—it wraps a union component type (React.ComponentType<P> | React.ForwardRefExoticComponent<P>) rather than a concrete type. This union constraint complicates typing and is likely why theas anycast exists here uniquely.Before applying the suggested alternatives (e.g.,
React.ComponentPropsWithRef<typeof Component>), verify that they work with the union component type constraint on line 11. The suggested refactorings may not resolve the underlying TypeScript limitation without additional type restructuring.
1 participant
This PR introduces a complete React Error Boundary system to fix the critical issue: “Missing Error Boundaries”.
Right now, any unhandled React error can crash the entire UI — this update fully closes that gap.
What’s Added
1. Global Error Boundary
2. Route-Level Error Boundaries
RouteErrorBoundarycomponent included3. Component-Level Error Boundaries
withErrorBoundary)4. Error Fallback UIs
5. Error Logging System
ErrorLoggerto capture and store errorsWhy This Matters
What I Tested
Files Added / Modified
ErrorBoundary.tsxRouteErrorBoundary.tsxErrorFallback.tsxErrorLogger.tswithErrorBoundary.tsxindex.ts(exports)App.tsx+main.tsxSummary by CodeRabbit
New Features
Bug Fixes