Add unified client error pages and preserve route shells on errors#706
Conversation
…er instead of the generic error page
|
Warning Review limit reached
Next review available in: 38 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (13)
📝 WalkthroughWalkthroughAdds unified client-error pages for statuses 400, 403, and 404, centralizes error classification, preserves application shells during route failures, handles inaccessible farms without throwing, and returns standardized action error responses. ChangesError handling and UI
Farm access and shell preservation
Nested route error boundaries
Route error response propagation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Browser
participant RouteLoader
participant RouteShell
participant RouteErrorFallback
participant ErrorPage
Browser->>RouteLoader: request route
RouteLoader->>RouteShell: provide loader data or boundary state
RouteShell->>RouteErrorFallback: render failed content pane
RouteErrorFallback->>ErrorPage: classify status and render page
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@fdm-app/app/routes/farm.tsx`:
- Around line 340-352: Guard useLoaderData() in ErrorBoundary implementations
across fdm-app/app/routes/farm.tsx:340-352,
fdm-app/app/routes/organization.tsx:213-221,
fdm-app/app/routes/support.tsx:165-173, and fdm-app/app/routes/user.tsx:114-122.
When route loader data is missing, return a bare ClientErrorPage before invoking
the corresponding shell or useFarmContextSync; otherwise preserve the existing
shell rendering with valid loader data.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: b0aa1749-03b8-464d-8a34-42b22feafe5f
📒 Files selected for processing (8)
.changeset/smart-stamps-hope.mdfdm-app/app/components/custom/error.tsxfdm-app/app/lib/error.tsfdm-app/app/root.tsxfdm-app/app/routes/farm.tsxfdm-app/app/routes/organization.tsxfdm-app/app/routes/support.tsxfdm-app/app/routes/user.tsx
BoraIneviNMI
left a comment
There was a problem hiding this comment.
I found two problems:
- Support pages used to depend on styling on the
support.tsxlayout. Now the TicketViewer and AgentManager components can go out of bounds of the page. - If a component throws an error while rendering, "Deze pagina is niet beschikbaar" is shown. Component errors should be classified as unknown error. To try it for yourself, add
throw new Error("failed")to any JSX component.
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@fdm-app/app/components/custom/error.tsx`:
- Around line 264-275: Update RouteErrorFallback so it does not throw
redirect(signInUrl) during render. Move the 401 navigation into the relevant
route loader/action, or trigger the redirect after render using the router’s
navigation mechanism, while preserving the currentPath redirect target and
sign-in behavior.
In `@fdm-app/app/lib/error.ts`:
- Around line 293-316: Add a dedicated 405 branch to the status switch in the
error handling function, mapping method-not-allowed responses to an appropriate
client-facing warning without calling reportError. Ensure other unclassified 4xx
statuses also avoid the unexpected-error/default reporting path, while
preserving the existing handling for 400, 401, 403, 404, and server-side errors.
- Around line 362-370: Update the default branch of handleActionError to pass
the error message as a data object containing the warning key, matching the
existing dataWithError/dataWithWarning contract used by the other branches.
Preserve the current error message, user-facing message, status, and errorId
values.
- Around line 293-331: The warning and error response branches currently pass
status in toast metadata instead of the response init options. Update both
dataWithWarning and dataWithError calls in the surrounding error handler to keep
the message in the second argument and move status into the third init argument.
In `@fdm-app/app/routes/organization`.$slug.members.tsx:
- Around line 371-373: Classify missing organizations as 404 responses instead
of passing unrecognized plain strings to handleActionError. Update both
organization-missing branches in
fdm-app/app/routes/organization.$slug.members.tsx at lines 371-373 and 401-403
to throw a 404 data(...) response, or extend the shared handleActionError
classifier to recognize the existing “not found:” prefix while preserving the
intended 404 behavior at both sites.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 2ab275f6-579b-4794-8846-90dab23a1dee
📒 Files selected for processing (54)
fdm-app/app/components/blocks/helpdesk/ticket-viewer.tsxfdm-app/app/components/custom/error.tsxfdm-app/app/lib/error.tsfdm-app/app/lib/form.tsfdm-app/app/root.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.atlas.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.bcs.$a_id.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.bcs._index.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.bcs.new.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.$b_lu.harvest.$b_id_harvesting.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.$b_lu.harvest.new.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.$b_lu.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer._index.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.soil._index.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.soil.analysis.$a_id.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.soil.analysis.new.$analysis_type.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.soil.analysis.new.upload.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.fertilizer._index.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.new._index.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.new.fields.$b_id._index.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.measures.$b_id.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.measures._index.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.rotation.modify_fertilizer.$p_id.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.rotation.modify_harvest.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.rotation.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.rotation_.fertilizer._index.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.rotation_.harvest._index.tsxfdm-app/app/routes/farm.$b_id_farm.fertilizers.$p_id.tsxfdm-app/app/routes/farm.$b_id_farm.fertilizers.new.custom.tsxfdm-app/app/routes/farm.$b_id_farm.settings.derogation.tsxfdm-app/app/routes/farm.$b_id_farm.settings.grazing-intention.tsxfdm-app/app/routes/farm.$b_id_farm.settings.properties.tsxfdm-app/app/routes/farm.$b_id_farm.tsxfdm-app/app/routes/farm.create.$b_id_farm.$calendar.atlas.tsxfdm-app/app/routes/farm.create.$b_id_farm.$calendar.fields.$b_id._index.tsxfdm-app/app/routes/farm.create.$b_id_farm.$calendar.fields.$b_id.soil.analysis.$analysis_type.tsxfdm-app/app/routes/farm.create.$b_id_farm.$calendar.fields.$b_id.soil.analysis.upload.tsxfdm-app/app/routes/farm.create._index.tsxfdm-app/app/routes/farm.tsxfdm-app/app/routes/logout.tsxfdm-app/app/routes/organization.$slug.members.tsxfdm-app/app/routes/organization.$slug.settings.tsxfdm-app/app/routes/organization._index.tsxfdm-app/app/routes/organization.new.tsxfdm-app/app/routes/organization.tsxfdm-app/app/routes/support._ticketviewer.ticket.$ticket_id.tsxfdm-app/app/routes/support.new.tsxfdm-app/app/routes/support.settings.agents.tsxfdm-app/app/routes/support.settings.blocked-emails.tsxfdm-app/app/routes/support.settings.tags.tsxfdm-app/app/routes/support.tsxfdm-app/app/routes/user.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- fdm-app/app/routes/organization.tsx
- fdm-app/app/routes/user.tsx
- fdm-app/app/routes/support.tsx
- fdm-app/app/routes/farm.tsx
There was a problem hiding this comment.
Pull request overview
This PR improves fdm-app’s error UX by introducing unified client-error pages (400/403/404), keeping route shells (sidebar/header) visible when nested routes error, and aligning loader/action error handling so expected failures show friendly UI/toasts instead of collapsing the whole app view.
Changes:
- Added unified client-error vs unexpected-error pages and a shared
RouteErrorFallbackclassifier used by route ErrorBoundaries. - Added layout-route shells +
ErrorBoundaryexports for/farm,/organization,/support,/userto preserve the app chrome on nested errors. - Updated many route
actions toreturn handleActionError(...)(toast-driven) instead of throwing, plus improved status extraction inhandleLoaderError/handleActionError.
Reviewed changes
Copilot reviewed 55 out of 55 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| fdm-app/app/routes/user.tsx | Introduces UserShell + route-level ErrorBoundary to keep sidebar visible on nested errors. |
| fdm-app/app/routes/support.tsx | Introduces SupportShell + ErrorBoundary, and adjusts content pane sizing for nested layouts. |
| fdm-app/app/routes/support.settings.tags.tsx | Returns handleActionError from action (toast flow) instead of throwing. |
| fdm-app/app/routes/support.settings.blocked-emails.tsx | Returns handleActionError from action (toast flow) instead of throwing. |
| fdm-app/app/routes/support.settings.agents.tsx | Returns handleActionError from action (toast flow) instead of throwing. |
| fdm-app/app/routes/support.new.tsx | Returns handleActionError from action (toast flow) instead of throwing. |
| fdm-app/app/routes/support._ticketviewer.ticket.$ticket_id.tsx | Aligns action error returns; contains special-case logic for extractFormValuesFromRequest-thrown responses. |
| fdm-app/app/routes/organization.tsx | Introduces OrganizationShell + ErrorBoundary to preserve sidebar/header on nested errors. |
| fdm-app/app/routes/organization.new.tsx | Returns handleActionError from action (toast flow) instead of throwing. |
| fdm-app/app/routes/organization.$slug.settings.tsx | Returns handleActionError from action (toast flow) instead of throwing. |
| fdm-app/app/routes/organization.$slug.members.tsx | Uses explicit data(..., {status: 404}) for not-found org cases; returns handleActionError in catch. |
| fdm-app/app/routes/organization._index.tsx | Returns handleActionError from action (toast flow) instead of throwing. |
| fdm-app/app/routes/logout.tsx | Returns handleActionError from action (toast flow) instead of throwing. |
| fdm-app/app/routes/farm.tsx | Adds farm-shell preservation on errors, handles permission-denied farm access without blowing away shell, and adds farm route ErrorBoundary. |
| fdm-app/app/routes/farm.create.$b_id_farm.$calendar.fields.$b_id.soil.analysis.upload.tsx | Returns handleActionError from action (toast flow) instead of throwing. |
| fdm-app/app/routes/farm.create.$b_id_farm.$calendar.fields.$b_id.soil.analysis.$analysis_type.tsx | Returns handleActionError from action (toast flow) instead of throwing. |
| fdm-app/app/routes/farm.create.$b_id_farm.$calendar.fields.$b_id._index.tsx | Returns handleActionError from action (toast flow) instead of throwing. |
| fdm-app/app/routes/farm.create.$b_id_farm.$calendar.atlas.tsx | Returns handleActionError from action (toast flow) instead of throwing. |
| fdm-app/app/routes/farm.create._index.tsx | Returns handleActionError from action (toast flow) instead of throwing. |
| fdm-app/app/routes/farm.$b_id_farm.tsx | Fixes loader error handling to use handleLoaderError (not action error handler). |
| fdm-app/app/routes/farm.$b_id_farm.settings.properties.tsx | Returns handleActionError from action (toast flow) instead of throwing. |
| fdm-app/app/routes/farm.$b_id_farm.settings.grazing-intention.tsx | Returns handleActionError from action (toast flow) instead of throwing. |
| fdm-app/app/routes/farm.$b_id_farm.settings.derogation.tsx | Returns handleActionError from action (toast flow) instead of throwing. |
| fdm-app/app/routes/farm.$b_id_farm.fertilizers.new.custom.tsx | Returns handleActionError from action (toast flow) instead of throwing. |
| fdm-app/app/routes/farm.$b_id_farm.fertilizers.$p_id.tsx | Returns handleActionError from action (toast flow) instead of throwing. |
| fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation.tsx | Returns handleActionError from action (toast flow) instead of throwing. |
| fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation.modify_harvest.tsx | Returns handleActionError from action (toast flow) instead of throwing. |
| fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation.modify_fertilizer.$p_id.tsx | Returns handleActionError from action (toast flow) instead of throwing. |
| fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation_.harvest._index.tsx | Returns handleActionError from action (toast flow) instead of throwing. |
| fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation_.fertilizer._index.tsx | Returns handleActionError from action (toast flow) instead of throwing. |
| fdm-app/app/routes/farm.$b_id_farm.$calendar.measures.$b_id.tsx | Returns handleActionError from action (toast flow) instead of throwing. |
| fdm-app/app/routes/farm.$b_id_farm.$calendar.measures._index.tsx | Returns handleActionError from action (toast flow) instead of throwing. |
| fdm-app/app/routes/farm.$b_id_farm.$calendar.field.tsx | Returns handleActionError from action (toast flow) instead of throwing. |
| fdm-app/app/routes/farm.$b_id_farm.$calendar.field.new.fields.$b_id._index.tsx | Returns handleActionError from action (toast flow) instead of throwing. |
| fdm-app/app/routes/farm.$b_id_farm.$calendar.field.new._index.tsx | Returns handleActionError from action (toast flow) instead of throwing. |
| fdm-app/app/routes/farm.$b_id_farm.$calendar.field.fertilizer._index.tsx | Returns handleActionError from action (toast flow) instead of throwing. |
| fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.soil.analysis.new.upload.tsx | Returns handleActionError from action (toast flow) instead of throwing. |
| fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.soil.analysis.new.$analysis_type.tsx | Returns handleActionError from action (toast flow) instead of throwing. |
| fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.soil.analysis.$a_id.tsx | Returns handleActionError from action (toast flow) instead of throwing. |
| fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.soil._index.tsx | Returns handleActionError from action (toast flow) instead of throwing. |
| fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer._index.tsx | Reorders an import and returns handleActionError from action (toast flow). |
| fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.tsx | Returns handleActionError from action (toast flow) instead of throwing. |
| fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.$b_lu.tsx | Returns handleActionError from action (toast flow) instead of throwing. |
| fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.$b_lu.harvest.new.tsx | Returns handleActionError from action (toast flow) instead of throwing. |
| fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.$b_lu.harvest.$b_id_harvesting.tsx | Returns handleActionError from action (toast flow) instead of throwing. |
| fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.bcs.new.tsx | Returns handleActionError from action (toast flow) instead of throwing. |
| fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.bcs.$a_id.tsx | Returns handleActionError from action (toast flow) instead of throwing. |
| fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.bcs._index.tsx | Returns handleActionError from action (toast flow) instead of throwing. |
| fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.atlas.tsx | Fixes loader to use handleLoaderError; returns handleActionError from action. |
| fdm-app/app/root.tsx | Switches to RouteErrorFallback and augments toast UX (analytics + copy-to-clipboard). |
| fdm-app/app/lib/form.ts | Ensures thrown handleActionError is awaited (so the thrown value is a response, not a promise). |
| fdm-app/app/lib/error.ts | Adds thrown-status extraction, exports containsErrorMessage, and documents/standardizes action vs loader error handling. |
| fdm-app/app/components/custom/error.tsx | Adds ClientErrorPage, UnexpectedErrorPage, and RouteErrorFallback for unified error rendering + analytics hooks. |
| fdm-app/app/components/blocks/helpdesk/ticket-viewer.tsx | Adjusts sizing to work with the new min-h-0 / h-full shell layout. |
| .changeset/smart-stamps-hope.md | Minor version bump note for fdm-app describing the new client-error page behavior. |
BoraIneviNMI
left a comment
There was a problem hiding this comment.
Looks good. Thank you for fixing the stuff I mentioned.
Co-authored-by: BoraIneviNMI <bora.inevi@nmi-agro.nl> Signed-off-by: Sven Verweij <37927107+SvenVw@users.noreply.github.com>
Signed-off-by: Sven Verweij <37927107+SvenVw@users.noreply.github.com>
Signed-off-by: Sven Verweij <37927107+SvenVw@users.noreply.github.com>
Summary by CodeRabbit
Closes #705