feat: added error boundary #4689
Conversation
sarthakNITT
requested review from
Mayaleeeee,
akshatnema,
anshgoyalevil,
asyncapi-bot-eve,
derberg,
princerajpoot20 and
sambhavgupta0705
as code owners
WalkthroughAdds a React ErrorBoundary component and wraps the entire app root in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related issues
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
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 |
✅ Deploy Preview for asyncapi-website ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4689 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 22 22
Lines 799 799
Branches 146 146
=========================================
Hits 799 799 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-4689--asyncapi-website.netlify.app/ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
pages/_app.tsx (1)
11-46: Global ErrorBoundary wrapper is solid; consider provider placement for future needsWrapping the whole tree with
ErrorBoundaryachieves the desired protection against white‑screen crashes while preserving existing layout and providers. If you later want the fallback UI to consumeAppContext(e.g., theme, path, additional UX state), consider moving the boundary inside the provider:<AppContext.Provider value={{ path: router.asPath }}> <ErrorBoundary> {/* existing tree */} </ErrorBoundary> </AppContext.Provider>No change is strictly required now; this is just a future‑proofing tweak.
components/ErrorBoundary.tsx (1)
1-49: ErrorBoundary behavior looks correct; tighten typings and logging for polishThe boundary implementation follows the usual React pattern and should work as intended. A few minor refinements you might consider:
- Give props a concrete type instead of
any, aligned with the class generics:-export default class ErrorBoundary extends React.Component<React.PropsWithChildren, ErrorBoundaryState> { - constructor(props: any) { +type ErrorBoundaryProps = React.PropsWithChildren<unknown>; + +export default class ErrorBoundary extends React.Component<ErrorBoundaryProps, ErrorBoundaryState> { + constructor(props: ErrorBoundaryProps) { super(props); this.state = { hasError: false }; }
- Make the lifecycle signatures explicit and typed, using unused‑param naming to satisfy linting:
- static getDerivedStateFromError() { + static getDerivedStateFromError(_error: Error): ErrorBoundaryState { return { hasError: true }; } - componentDidCatch(error: any, info: any) { + componentDidCatch(error: Error, info: React.ErrorInfo) { // eslint-disable-next-line no-console console.error('ErrorBoundary caught:', error, info); }
- If you already have a central logging/monitoring utility (e.g., similar to how build runners use a shared
logger.error), consider delegating to that here in addition toconsole.errorso frontend runtime errors are captured consistently. Based on learnings, aligning with existing error-handling patterns will make ops/debugging easier across the stack.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/ErrorBoundary.tsx(1 hunks)pages/_app.tsx(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: sagarkori143
Repo: asyncapi/website PR: 4192
File: npm/runners/build-pages-runner.ts:7-14
Timestamp: 2025-06-19T13:51:27.459Z
Learning: In the AsyncAPI website build system, individual runner functions use try-catch blocks that rethrow errors to maintain consistent error boundaries, while the top-level orchestrator in npm/index.ts provides meaningful error logging with task-specific context using logger.error(`Error building ${taskName}:`, error). This separation of concerns allows for clean error handling at different architectural layers.
🧬 Code graph analysis (1)
pages/_app.tsx (1)
components/ErrorBoundary.tsx (1)
ErrorBoundary(7-49)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Redirect rules - asyncapi-website
- GitHub Check: Header rules - asyncapi-website
- GitHub Check: Pages changed - asyncapi-website
- GitHub Check: cypress-run
- GitHub Check: Lighthouse CI
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
components/ErrorBoundary.tsx (2)
3-11: Tighten props typing and avoidany/ constructor boilerplateCurrent class signature and constructor use loose typing and extra boilerplate:
React.PropsWithChildrenis used without a concrete props shape.constructor(props: any)defeats type-safety for props.- State can be initialized via a class field instead of a constructor.
You can simplify and get stronger typing like this:
-import React from 'react'; - -interface ErrorBoundaryState { - hasError: boolean; -} - -export default class ErrorBoundary extends React.Component<React.PropsWithChildren, ErrorBoundaryState> { - constructor(props: any) { - super(props); - this.state = { hasError: false }; - } +import React from 'react'; + +type ErrorBoundaryProps = React.PropsWithChildren<{}>; + +interface ErrorBoundaryState { + hasError: boolean; +} + +export default class ErrorBoundary extends React.Component<ErrorBoundaryProps, ErrorBoundaryState> { + state: ErrorBoundaryState = { hasError: false };This keeps behavior identical while improving type safety and readability.
Please re-run the TypeScript compiler / ESLint after this change to ensure it matches your React 18 + TS config.
13-20: Use React’s typed error signatures for lifecycle methodsTo align with React 18 typings and avoid
any, you can tighten the error lifecycle signatures:- static getDerivedStateFromError() { - return { hasError: true }; - } - - componentDidCatch(error: any, info: any) { + static getDerivedStateFromError(_error: Error): Partial<ErrorBoundaryState> { + return { hasError: true }; + } + + componentDidCatch(error: Error, info: React.ErrorInfo) { // eslint-disable-next-line no-console console.error('ErrorBoundary caught:', error, info); }This preserves behavior while matching React’s expected types and removes
anyfrom the public surface.Please confirm against your installed
@types/reactversion and adjust if its signatures differ.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/ErrorBoundary.tsx(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: PR testing - if Node project
components/ErrorBoundary.tsx
[error] 26-26: ESLint: 'spaced-comment' rule violated. Expected space or tab after '//' in comment.
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: cypress-run
2 participants
Description
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.