-
Notifications
You must be signed in to change notification settings - Fork 1
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
Add ErrorBoundary and ErrorBlock; update docs and dependency versions #81
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: 39f7393 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughThe changes introduce an Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Layout
participant ErrorBoundary
participant Outlet
participant ErrorBlock
User->>Layout: Initiate page load
Layout->>ErrorBoundary: Render with ErrorBoundary wrapping Outlet/Toaster
Outlet-->>ErrorBoundary: Occurrence of an error
ErrorBoundary->>ErrorBoundary: Evaluate error type/status
ErrorBoundary->>ErrorBlock: Render ErrorBlock with appropriate details
ErrorBlock-->>User: Display error message and controls
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #81 +/- ##
=======================================
Coverage 94.19% 94.19%
=======================================
Files 27 27
Lines 3563 3563
Branches 259 259
=======================================
Hits 3356 3356
Misses 207 207
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
Actionable comments posted: 0
🧹 Nitpick comments (3)
fdm-app/app/root.tsx (1)
91-143
: Consider improving error handling for specific status codes.The error handling implementation is good but could be improved:
- The status code check could be simplified using an array
- The 404 status is being used for multiple error types (400, 401, 403)
Consider this refactoring:
- if ( - error.status === 404 || - error.status === 401 || - error.status === 400 || - error.status === 403 - ) { + const clientErrors = [400, 401, 403, 404]; + if (clientErrors.includes(error.status)) { return ( <ErrorBlock - status={404} + status={error.status} message={error.statusText} stacktrace={error.data} page={page} timestamp={timestamp} /> ) }fdm-app/app/components/custom/error.tsx (2)
46-50
: Consider making the error image configurable.The image URL is hardcoded, which might not be ideal for reusability.
Consider making it a prop:
+ image?: string }) { const [isCopied, setIsCopied] = useState(false) + const defaultImage = "https://hebbkx1anhila5yf.public.blob.vercel-storage.com/giphy-zaMc9sEWI1lqXlXSKSKR164AvQCUjf.webp" // ... rest of the component <img - src="https://hebbkx1anhila5yf.public.blob.vercel-storage.com/giphy-zaMc9sEWI1lqXlXSKSKR164AvQCUjf.webp" + src={image || defaultImage} alt="A red tractor doing a wheelie" className="w-full rounded-lg" />
63-91
: Consider extracting navigation buttons into a separate component.The navigation buttons logic could be extracted to reduce duplication and improve maintainability.
Consider creating a separate component:
type NavigationButtonsProps = { is404?: boolean; onCopy?: () => void; isCopied?: boolean; }; function NavigationButtons({ is404, onCopy, isCopied }: NavigationButtonsProps) { if (is404) { return ( <div className="flex flex-col sm:flex-row gap-4"> <Button asChild> <NavLink to="/"> <ArrowLeft className="mr-2 h-4 w-4" /> Terug naar vorige pagina </NavLink> </Button> <Button variant="outline" asChild> <NavLink to="/"> <Home className="mr-2 h-4 w-4" /> Terug naar de hoofdpagina </NavLink> </Button> </div> ); } return ( <div className="flex flex-col sm:flex-row gap-4"> <Button asChild> <NavLink to="/"> <Home className="mr-2 h-4 w-4" /> Terug naar de hoofdpagina </NavLink> </Button> <Button variant="outline" onClick={onCopy}> <Copy className="mr-2 h-4 w-4" /> {isCopied ? "Gekopieerd!" : "Kopieer foutmelding"} </Button> </div> ); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
.changeset/six-buckets-flow.md
(1 hunks).changeset/weak-swans-protect.md
(1 hunks)fdm-app/app/components/custom/atlas/atlas-functions.tsx
(1 hunks)fdm-app/app/components/custom/atlas/atlas-panels.tsx
(2 hunks)fdm-app/app/components/custom/atlas/atlas.d.tsx
(2 hunks)fdm-app/app/components/custom/error.tsx
(1 hunks)fdm-app/app/root.tsx
(3 hunks)fdm-app/app/routes/farm.create.$b_id_farm.atlas.tsx
(1 hunks)fdm-app/package.json
(2 hunks)
✅ Files skipped from review due to trivial changes (4)
- fdm-app/app/routes/farm.create.$b_id_farm.atlas.tsx
- fdm-app/app/components/custom/atlas/atlas.d.tsx
- fdm-app/app/components/custom/atlas/atlas-functions.tsx
- fdm-app/app/components/custom/atlas/atlas-panels.tsx
🔇 Additional comments (18)
fdm-app/app/root.tsx (2)
2-4
: LGTM! Import statements are well organized.The new imports support the error handling implementation.
Also applies to: 8-8, 13-15, 21-23
76-79
: LGTM! ErrorBoundary implementation in Layout.The ErrorBoundary correctly wraps both Outlet and Toaster components to catch rendering errors.
fdm-app/app/components/custom/error.tsx (2)
6-18
: LGTM! Props interface is well defined.The props structure clearly defines the required information for error display.
19-26
: LGTM! Copy notification state management.The useEffect cleanup is properly implemented to clear the timeout.
.changeset/six-buckets-flow.md (1)
1-6
: LGTM! Changeset correctly describes the ErrorBoundary addition.The minor version bump is appropriate for this feature addition.
.changeset/weak-swans-protect.md (1)
1-6
: LGTM! Changeset correctly describes the styled error pages addition.The minor version bump is appropriate for this feature addition.
fdm-app/package.json (12)
18-21
: Radix UI Dependencies Update (Part 1)
The version bumps for@radix-ui/react-avatar
,@radix-ui/react-checkbox
,@radix-ui/react-dialog
, and@radix-ui/react-dropdown-menu
are updated to newer minor versions. These updates appear routine and help ensure you receive any bug fixes or improvements. Please verify that these updated versions remain fully compatible with your UI components.
23-30
: Radix UI Dependencies Update (Part 2)
The versions for@radix-ui/react-label
,@radix-ui/react-popover
,@radix-ui/react-select
,@radix-ui/react-separator
,@radix-ui/react-slot
,@radix-ui/react-tabs
,@radix-ui/react-toast
, and@radix-ui/react-tooltip
have been updated. Confirm that these changes do not introduce any unexpected behavior in your UI.
31-32
: React Router Tools Update
The dependencies@react-router/node
and@react-router/serve
are bumped to^7.1.5
. This update should align well with related changes in error handling (e.g., the new ErrorBoundary in the main layout). Ensure to run integration tests for the routing layer to confirm compatibility.
34-34
:better-auth
Dependency Update
Upgradingbetter-auth
to^1.1.18
is noted. Please double-check that your authentication flows and any related configurations align with the changes introduced in the new version.
39-40
: Geospatial and Bot Detection Updates
The updates offlatgeobuf
to^3.37.1
andisbot
to^5.1.22
are straightforward. Verify that functionalities relying on geospatial data processing and bot detection continue to operate as expected.
43-43
:mapbox-gl
Dependency Update
mapbox-gl
has been updated to^3.10.0
. Although this is a minor version bump, it’s important to check that mapping visualizations or related interactions stay consistent with previous behavior.
49-51
: Mapping and Routing Dependencies Update
The upgrades forreact-map-gl
,react-router
, andreact-router-dom
to^7.1.9
,^7.1.5
, and^7.1.5
respectively are in line with overall dependency maintenance. Please ensure that mapping components and routing logic—including any error boundary integrations—are thoroughly regression tested.
55-55
:sonner
Dependency Update
The packagesonner
is updated to^1.7.4
. Confirm that any toast notifications or interactive UI elements relying on this library work correctly with the new version.
60-60
:zod
Dependency Update
Bumpingzod
to^3.24.2
is a minor update; however, review any schema validations or type-checking logic that might be affected by subtle changes in validation behavior.
64-65
: DevDependencies – React Router Tools Update
The development dependencies@react-router/dev
and@react-router/fs-routes
are updated to^7.1.5
. Ensure that build-time and routing development functionalities continue to operate smoothly after this upgrade.
68-68
: Type Definitions Update for GeoJSON
The package@types/geojson
is now at^7946.0.16
. This update should provide enhanced type definitions; please run type checks to verify there are no unexpected type issues.
78-78
:postcss
Dependency Update
postcss
has been updated to^8.5.2
. Although it’s a minor update, confirm that your CSS processing and build pipeline are unaffected, especially if you have custom PostCSS configurations.
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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
fdm-app/app/root.tsx (1)
94-94
: Consider user's timezone for timestamp.The timestamp is generated using the server's timezone. Consider converting it to the user's local timezone for better user experience.
- const timestamp = new Date().toISOString() + const timestamp = new Date().toLocaleString()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
fdm-app/app/root.tsx
(3 hunks)
🔇 Additional comments (2)
fdm-app/app/root.tsx (2)
76-79
: Well-placed error boundary implementation!The ErrorBoundary correctly wraps both the Outlet and Toaster components, ensuring that any errors in child components are caught and handled appropriately.
2-2
:❓ Verification inconclusive
Verify usage of imported icons.
The
ArrowLeft
,Copy
, andHome
icons are imported but don't appear to be used in this file. They might be used in theErrorBlock
component, but if not, consider removing them.
🏁 Script executed:
#!/bin/bash # Check if the icons are used in ErrorBlock component ast-grep --pattern 'ArrowLeft|Copy|Home' ./fdm-app/app/components/custom/error.tsxLength of output: 173
Attention: Verify Unused Icon Imports
In
fdm-app/app/root.tsx
, the iconsArrowLeft
,Copy
, andHome
are imported yet don’t seem to be referenced in this file. The initial check for their usage in the anticipated ErrorBlock component (expected atfdm-app/app/components/custom/error.tsx
) failed because that file wasn’t found. Please verify manually if these icons are utilized elsewhere or within an alternative location for the ErrorBlock component. If they aren’t used anywhere in the repository, consider removing the unused imports to keep the code clean.
🏁 Script executed:
[run_scripts] #!/bin/bash echo "Searching for usage of imported icons (ArrowLeft, Copy, Home) in the repository:" echo "ArrowLeft:" rg "<ArrowLeft" fdm-app echo "Copy:" rg "<Copy" fdm-app echo "Home:" rg "<Home" fdm-app echo "Locating any file that might define the ErrorBlock component:" fd -t f -e tsx -e ts -e js -e jsx "ErrorBlock" fdm-appLength of output: 854
Verify Unused Icon Imports in fdm-app/app/root.tsx
- The file imports
ArrowLeft
,Copy
, andHome
from "lucide-react" but there’s no evidence in this file of them being used.- An automated search for these icons—especially in the anticipated
ErrorBlock
component—did not yield any results. Additionally, the expected file/directory structure (e.g.,fdm-app
) couldn’t be located, so further manual verification is suggested.- Please confirm if these icons are used elsewhere (such as within the
ErrorBlock
or any other component). If they’re not used, remove them to maintain code clarity.
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.
Actionable comments posted: 1
♻️ Duplicate comments (1)
fdm-app/app/root.tsx (1)
75-75
:⚠️ Potential issueRemove ErrorBoundary from Layout.
Based on React Router v7+ guidelines, the ErrorBoundary should not be used as a wrapper component in Layout. Instead, it should only be exported as a separate component, which you already have correctly defined.
Apply this diff:
- <ErrorBoundary />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
fdm-app/app/root.tsx
(4 hunks)
🧰 Additional context used
🧠 Learnings (1)
fdm-app/app/root.tsx (1)
Learnt from: SvenVw
PR: SvenVw/fdm#81
File: fdm-app/app/root.tsx:79-79
Timestamp: 2025-02-18T11:00:54.405Z
Learning: In React Router v7+, to ensure error pages are displayed correctly:
1. Export the ErrorBoundary component from the root route
2. Include the Scripts component in the Layout
3. Export the Layout through the default App component
🔇 Additional comments (2)
fdm-app/app/root.tsx (2)
2-2
: LGTM! Import changes are well-organized.The new imports are logically grouped and support the error handling functionality.
Also applies to: 11-11, 13-13, 19-19, 20-20
87-135
: LGTM! ErrorBoundary implementation follows React Router v7+ patterns.The error handling logic is well-structured and covers different error scenarios appropriately.
fdm-app/app/root.tsx
Outdated
export default function App() { | ||
return <Outlet /> | ||
} |
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.
Export Layout through App component.
According to React Router v7+ guidelines, the Layout should be exported through the default App component to ensure proper error handling.
Apply this diff:
- return <Outlet />
+ return <Layout />
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
export default function App() { | |
return <Outlet /> | |
} | |
export default function App() { | |
return <Layout /> | |
} |
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.
Will be resolved in 39f7393
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.
@SvenVw Thanks for confirming! Looking forward to the fix in commit 39f7393.
🧠 Learnings used
Learnt from: SvenVw
PR: SvenVw/fdm#81
File: fdm-app/app/root.tsx:79-79
Timestamp: 2025-02-18T11:00:54.405Z
Learning: In React Router v7+, to ensure error pages are displayed correctly:
1. Export the ErrorBoundary component from the root route
2. Include the Scripts component in the Layout
3. Export the Layout through the default App component
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
fdm-app/app/root.tsx (1)
126-128
: Consider adding logging for null errors.While returning null for null errors is correct, consider adding logging to help debug unexpected null errors in production.
if (error === null) { + console.debug('ErrorBoundary received null error') return null }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
fdm-app/app/root.tsx
(4 hunks)
🧰 Additional context used
🧠 Learnings (1)
fdm-app/app/root.tsx (1)
Learnt from: SvenVw
PR: SvenVw/fdm#81
File: fdm-app/app/root.tsx:79-79
Timestamp: 2025-02-18T11:00:54.405Z
Learning: In React Router v7+, to ensure error pages are displayed correctly:
1. Export the ErrorBoundary component from the root route
2. Include the Scripts component in the Layout
3. Export the Layout through the default App component
🔇 Additional comments (4)
fdm-app/app/root.tsx (4)
2-2
: LGTM! Import changes are well-organized.The new imports support the error handling implementation, including
isRouteErrorResponse
,useLocation
, and the customErrorBlock
component.Also applies to: 11-11, 13-13, 19-20
75-75
: LGTM! ErrorBoundary placement is correct.The ErrorBoundary is correctly placed as a sibling component, following React Router v7 guidelines.
83-85
: LGTM! App component follows React Router v7 guidelines.The Layout is correctly exported through the default App component, ensuring proper error handling.
92-104
: Reconsider error code handling strategy.The current implementation shows a 404 status for all client errors (400, 401, 403, 404). This might be misleading for users and logging systems.
) | ||
} | ||
if (error === null) { | ||
return null |
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.
errorblock is not automatically handlign null cases correctly? Its just a few lines, so not a problem.
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.
LGTM. Thanks
Closes #55, #51 and #7
Summary by CodeRabbit
New Features
ErrorBoundary
component for improved error handling, redirecting users to dedicated error pages.ErrorBlock
component for displaying error information in a user-friendly manner.Chores