feat: implement global React Error Boundary with fallback UI#497
Open
sohaa-khan11 wants to merge 2 commits intoAOSSIE-Org:mainfrom
Open
feat: implement global React Error Boundary with fallback UI#497sohaa-khan11 wants to merge 2 commits intoAOSSIE-Org:mainfrom
sohaa-khan11 wants to merge 2 commits intoAOSSIE-Org:mainfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a new React ErrorBoundary component and wraps the application's Routes with it in App.js; also renames two imports to camelCase (QuestionType, TextInput) and updates JSX to use the renamed components. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant App as App (Routes)
participant EB as ErrorBoundary
participant Page as ChildPageComponent
Browser->>App: Initial render
App->>EB: Render Routes (children)
EB->>Page: Render child component
alt Child throws error
Page-->>EB: Error thrown
EB->>EB: getDerivedStateFromError / componentDidCatch
EB->>Browser: Render fallback UI (Retry button)
Browser->>EB: Click Retry
EB->>Browser: window.location.reload()
else No error
Page-->>EB: Renders normally
EB->>Browser: Display page content
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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.
🧹 Nitpick comments (3)
eduaid_web/src/utils/ErrorBoundary.js (3)
27-52: Add minimal accessibility semantics to the fallback block.Please add alert semantics so assistive tech announces the error state, and set the button type explicitly.
♿ Proposed refactor
- <div style={{ + <div + role="alert" + aria-live="assertive" + style={{ display: 'flex', flexDirection: 'column', alignItems: 'center', justifyContent: 'center', height: '100vh', backgroundColor: '#02000F', color: 'white', fontFamily: 'sans-serif' }}> @@ - <button + <button + type="button" onClick={this.handleRetry} style={{🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eduaid_web/src/utils/ErrorBoundary.js` around lines 27 - 52, The fallback UI in ErrorBoundary's render (the div containing the error message and the Retry button) lacks accessibility semantics; add ARIA alert semantics (e.g., role="alert" or aria-live="assertive") on the container that holds the "Something went wrong." text so assistive tech announces the error, and make the Retry button explicit by adding type="button" to the element that uses this.handleRetry (the Retry button) to prevent accidental form submission.
19-21: Prefer local boundary reset over full page reload.Line 20 triggers a hard refresh, which is heavy and resets the whole app session. Resetting boundary state is usually enough to remount children.
♻️ Proposed refactor
- handleRetry = () => { - window.location.reload(); - }; + handleRetry = () => { + this.setState({ hasError: false }); + };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eduaid_web/src/utils/ErrorBoundary.js` around lines 19 - 21, The handleRetry method in the ErrorBoundary component currently forces a full page reload; instead update it to reset the boundary's local error state so children remount without wiping the entire app session. Inside ErrorBoundary.handleRetry set the internal error-related state (e.g., this.setState({ hasError: false, error: null, errorInfo: null }) or the equivalent hooks state reset) and ensure any derived values are cleared so the component re-renders its children rather than calling window.location.reload().
14-17: Route caught errors to monitoring, not only console.Consider forwarding
erroranderrorInfoto your error tracking pipeline (e.g., Sentry/LogRocket) so production failures are actionable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eduaid_web/src/utils/ErrorBoundary.js` around lines 14 - 17, In ErrorBoundary.componentDidCatch, replace the sole console.error call by forwarding the captured error and errorInfo to your monitoring pipeline: import and call your monitoring client (e.g., Sentry.captureException(error, { extra: errorInfo }) or LogRocket.log(error, errorInfo)) inside componentDidCatch, keeping a console.error fallback; ensure you include both `error` and `errorInfo` when invoking the monitoring API and handle any potential exceptions from the monitoring call so it doesn't crash the boundary.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@eduaid_web/src/utils/ErrorBoundary.js`:
- Around line 27-52: The fallback UI in ErrorBoundary's render (the div
containing the error message and the Retry button) lacks accessibility
semantics; add ARIA alert semantics (e.g., role="alert" or
aria-live="assertive") on the container that holds the "Something went wrong."
text so assistive tech announces the error, and make the Retry button explicit
by adding type="button" to the element that uses this.handleRetry (the Retry
button) to prevent accidental form submission.
- Around line 19-21: The handleRetry method in the ErrorBoundary component
currently forces a full page reload; instead update it to reset the boundary's
local error state so children remount without wiping the entire app session.
Inside ErrorBoundary.handleRetry set the internal error-related state (e.g.,
this.setState({ hasError: false, error: null, errorInfo: null }) or the
equivalent hooks state reset) and ensure any derived values are cleared so the
component re-renders its children rather than calling window.location.reload().
- Around line 14-17: In ErrorBoundary.componentDidCatch, replace the sole
console.error call by forwarding the captured error and errorInfo to your
monitoring pipeline: import and call your monitoring client (e.g.,
Sentry.captureException(error, { extra: errorInfo }) or LogRocket.log(error,
errorInfo)) inside componentDidCatch, keeping a console.error fallback; ensure
you include both `error` and `errorInfo` when invoking the monitoring API and
handle any potential exceptions from the monitoring call so it doesn't crash the
boundary.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addressed Issues:
Fixes #495
Screenshots/Recordings:
Additional Notes:
This PR introduces a minimal, reusable Global React Error Boundary to improve frontend stability.
Checklist
AI Usage Disclosure
Check one of the checkboxes below:
I have used the following AI models and tools:
ChatGPT (for guidance and review assistance)
We encourage contributors to use AI tools responsibly when creating Pull Requests. While AI can be a valuable aid, it is essential to ensure that your contributions meet the task requirements, build successfully, include relevant tests, and pass all linters. Submissions that do not meet these standards may be closed without warning to maintain the quality and integrity of the project. Please take the time to understand the changes you are proposing and their impact.
Summary by CodeRabbit