From 3706bf51af143c627709a848873124c8cc556817 Mon Sep 17 00:00:00 2001 From: Kasper Fabricius Kristensen <45367945+kasperkristensen@users.noreply.github.com> Date: Tue, 20 Aug 2024 15:09:30 +0200 Subject: [PATCH] feat(dashboard): Wrap each route in an ErrorBoundary (#8674) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **What** - Updates the copy of the different error types - Wraps each route (+ custom routes) in an ErrorBoundary to preserve app layout on error (sidebar and topbar) ![Skærmbillede 2024-08-20 kl 12 35 53](https://github.com/user-attachments/assets/0c589fc4-b279-4b66-9d66-1b99a7406696) **Note** If the user goes to a route that does not exist at all, e.g. `/some-weird-url`, then we have no way of knowing if the user is inside of a context where we can render the sidebar and topbar (as they require the user to be authenticated). So in this case we still show an ErrorBoundary where the two aren't included (see second picture), and include a button that takes the user to "/", which depending on whether the user is logged in will take them to "/login" or "/orders". ![image](https://github.com/user-attachments/assets/08dde48a-3bb8-41a1-9a0e-2c41716baf0b) Resolves CC-248 --- .../src/components/layout/shell/shell.tsx | 15 ++++- .../error-boundary/error-boundary.tsx | 58 +++++-------------- .../dashboard/src/i18n/translations/en.json | 16 ++--- .../dashboard/src/lib/extension-helpers.ts | 2 + .../src/providers/router-provider/index.ts | 2 +- .../providers/router-provider/route-map.tsx | 35 +++++++++-- .../src/routes/no-match/no-match.tsx | 34 ++++++++++- 7 files changed, 106 insertions(+), 56 deletions(-) diff --git a/packages/admin-next/dashboard/src/components/layout/shell/shell.tsx b/packages/admin-next/dashboard/src/components/layout/shell/shell.tsx index 626696b3bf..a177e48040 100644 --- a/packages/admin-next/dashboard/src/components/layout/shell/shell.tsx +++ b/packages/admin-next/dashboard/src/components/layout/shell/shell.tsx @@ -53,11 +53,24 @@ const Breadcrumbs = () => { .map((match) => { const handle = match.handle + let label: string | null = null + + try { + label = handle.crumb!(match.data) + } catch (error) { + // noop + } + + if (!label) { + return null + } + return { - label: handle.crumb!(match.data), + label: label, path: match.pathname, } }) + .filter(Boolean) as { label: string; path: string }[] return (
    with ErrorBoundary for more granular error handling. export const ErrorBoundary = () => { const error = useRouteError() const location = useLocation() @@ -45,47 +43,23 @@ export const ErrorBoundary = () => { } return ( -
    -
    - - - {title} - - - {message} - - +
    +
    +
    + +
    + + {title} + + + {message} + +
    +
    ) } - -/** - * Component that renders an error stack trace in development mode. - * - * We don't want to show stack traces in production, so this component is only - * rendered when the `NODE_ENV` is set to `development`. - * - * The reason for adding this is that `react-router-dom` can swallow certain types - * of errors, e.g. a missing export from a module that is exported, and will instead - * log a vague warning related to the route not exporting a Component. - */ -const DevelopmentStack = ({ error }: { error: unknown }) => { - const stack = error instanceof Error ? error.stack : null - const [stackType, stackMessage] = stack?.split(":") ?? ["", ""] - const isDevelopment = process.env.NODE_ENV === "development" - - if (!isDevelopment) { - return null - } - - return ( - - {stackMessage} - - ) -} diff --git a/packages/admin-next/dashboard/src/i18n/translations/en.json b/packages/admin-next/dashboard/src/i18n/translations/en.json index 4f58dcdf39..4c890b0318 100644 --- a/packages/admin-next/dashboard/src/i18n/translations/en.json +++ b/packages/admin-next/dashboard/src/i18n/translations/en.json @@ -245,14 +245,16 @@ "addFilter": "Add filter" }, "errorBoundary": { - "badRequestTitle": "Bad request", - "badRequestMessage": "The request was invalid.", - "notFoundTitle": "Not found", - "notFoundMessage": "The page you are looking for does not exist.", - "internalServerErrorTitle": "Internal server error", - "internalServerErrorMessage": "An error occurred on the server.", + "badRequestTitle": "400 - Bad request", + "badRequestMessage": "The request could not be understood by the server due to malformed syntax.", + "notFoundTitle": "404 - There is no page at this address", + "notFoundMessage": "Check the URL and try again, or use the search bar to find what you are looking for.", + "internalServerErrorTitle": "500 - Internal server error", + "internalServerErrorMessage": "An unexpected error occurred on the server. Please try again later.", "defaultTitle": "An error occurred", - "defaultMessage": "An error occurred while rendering this page." + "defaultMessage": "An unexpected error occurred while rendering this page.", + "noMatchMessage": "The page you are looking for does not exist.", + "backToDashboard": "Back to dashboard" }, "addresses": { "shippingAddress": { diff --git a/packages/admin-next/dashboard/src/lib/extension-helpers.ts b/packages/admin-next/dashboard/src/lib/extension-helpers.ts index 7a68a34923..e5d15cb675 100644 --- a/packages/admin-next/dashboard/src/lib/extension-helpers.ts +++ b/packages/admin-next/dashboard/src/lib/extension-helpers.ts @@ -1,4 +1,5 @@ import { RouteObject } from "react-router-dom" +import { ErrorBoundary } from "../components/utilities/error-boundary" /** * Used to test if a route is a settings route. @@ -32,6 +33,7 @@ export const createRouteMap = ( route.children ||= [] route.children.push({ path: "", + ErrorBoundary: ErrorBoundary, async lazy() { return { Component } }, diff --git a/packages/admin-next/dashboard/src/providers/router-provider/index.ts b/packages/admin-next/dashboard/src/providers/router-provider/index.ts index c259add166..997dca4e24 100644 --- a/packages/admin-next/dashboard/src/providers/router-provider/index.ts +++ b/packages/admin-next/dashboard/src/providers/router-provider/index.ts @@ -1 +1 @@ -export * from "./router-provider"; +export * from "./router-provider" diff --git a/packages/admin-next/dashboard/src/providers/router-provider/route-map.tsx b/packages/admin-next/dashboard/src/providers/router-provider/route-map.tsx index 2c2c4853bc..19c65ad4ff 100644 --- a/packages/admin-next/dashboard/src/providers/router-provider/route-map.tsx +++ b/packages/admin-next/dashboard/src/providers/router-provider/route-map.tsx @@ -26,10 +26,6 @@ export const RouteMap: RouteObject[] = [ path: "/login", lazy: () => import("../../routes/login"), }, - { - path: "/", - lazy: () => import("../../routes/home"), - }, { path: "*", lazy: () => import("../../routes/no-match"), @@ -46,8 +42,14 @@ export const RouteMap: RouteObject[] = [ path: "/", element: , children: [ + { + index: true, + errorElement: , + lazy: () => import("../../routes/home"), + }, { path: "/products", + errorElement: , handle: { crumb: () => "Products", }, @@ -158,6 +160,7 @@ export const RouteMap: RouteObject[] = [ }, { path: "/categories", + errorElement: , handle: { crumb: () => "Categories", }, @@ -206,6 +209,7 @@ export const RouteMap: RouteObject[] = [ }, { path: "/orders", + errorElement: , handle: { crumb: () => "Orders", }, @@ -264,6 +268,7 @@ export const RouteMap: RouteObject[] = [ }, { path: "/promotions", + errorElement: , handle: { crumb: () => "Promotions", }, @@ -305,6 +310,7 @@ export const RouteMap: RouteObject[] = [ }, { path: "/campaigns", + errorElement: , handle: { crumb: () => "Campaigns" }, children: [ { @@ -341,6 +347,7 @@ export const RouteMap: RouteObject[] = [ }, { path: "/collections", + errorElement: , handle: { crumb: () => "Collections", }, @@ -383,6 +390,7 @@ export const RouteMap: RouteObject[] = [ }, { path: "/price-lists", + errorElement: , handle: { crumb: () => "Price Lists", }, @@ -434,6 +442,7 @@ export const RouteMap: RouteObject[] = [ }, { path: "/customers", + errorElement: , handle: { crumb: () => "Customers", }, @@ -479,6 +488,7 @@ export const RouteMap: RouteObject[] = [ }, { path: "/customer-groups", + errorElement: , handle: { crumb: () => "Customer Groups", }, @@ -534,6 +544,7 @@ export const RouteMap: RouteObject[] = [ }, { path: "/reservations", + errorElement: , handle: { crumb: () => "Reservations", }, @@ -577,6 +588,7 @@ export const RouteMap: RouteObject[] = [ }, { path: "/inventory", + errorElement: , handle: { crumb: () => "Inventory", }, @@ -660,10 +672,12 @@ export const RouteMap: RouteObject[] = [ children: [ { index: true, + errorElement: , lazy: () => import("../../routes/settings"), }, { path: "profile", + errorElement: , lazy: () => import("../../routes/profile/profile-detail"), handle: { crumb: () => "Profile", @@ -677,6 +691,7 @@ export const RouteMap: RouteObject[] = [ }, { path: "regions", + errorElement: , element: , handle: { crumb: () => "Regions", @@ -715,6 +730,7 @@ export const RouteMap: RouteObject[] = [ }, { path: "store", + errorElement: , lazy: () => import("../../routes/store/store-detail"), handle: { crumb: () => "Store", @@ -736,6 +752,7 @@ export const RouteMap: RouteObject[] = [ }, { path: "users", + errorElement: , element: , handle: { crumb: () => "Users", @@ -768,6 +785,7 @@ export const RouteMap: RouteObject[] = [ }, { path: "sales-channels", + errorElement: , element: , handle: { crumb: () => "Sales Channels", @@ -814,6 +832,7 @@ export const RouteMap: RouteObject[] = [ }, { path: "locations", + errorElement: , element: , handle: { crumb: () => "Locations & Shipping", @@ -865,6 +884,7 @@ export const RouteMap: RouteObject[] = [ }, { path: "shipping-option-types", + errorElement: , element: , handle: { crumb: () => "Shipping Option Types", @@ -962,6 +982,7 @@ export const RouteMap: RouteObject[] = [ }, { path: "product-tags", + errorElement: , element: , handle: { crumb: () => "Product Tags", @@ -999,6 +1020,7 @@ export const RouteMap: RouteObject[] = [ }, { path: "workflows", + errorElement: , element: , handle: { crumb: () => "Workflows", @@ -1031,6 +1053,7 @@ export const RouteMap: RouteObject[] = [ }, { path: "product-types", + errorElement: , element: , handle: { crumb: () => "Product Types", @@ -1069,6 +1092,7 @@ export const RouteMap: RouteObject[] = [ { path: "publishable-api-keys", + errorElement: , element: , handle: { crumb: () => "Publishable API Keys", @@ -1128,6 +1152,7 @@ export const RouteMap: RouteObject[] = [ }, { path: "secret-api-keys", + errorElement: , element: , handle: { crumb: () => "Secret API Keys", @@ -1180,6 +1205,7 @@ export const RouteMap: RouteObject[] = [ }, { path: "tax-regions", + errorElement: , element: , handle: { crumb: () => "Tax Regions", @@ -1298,6 +1324,7 @@ export const RouteMap: RouteObject[] = [ }, { path: "return-reasons", + errorElement: , element: , handle: { crumb: () => "Return Reasons", diff --git a/packages/admin-next/dashboard/src/routes/no-match/no-match.tsx b/packages/admin-next/dashboard/src/routes/no-match/no-match.tsx index 795ded707c..1c1caacc30 100644 --- a/packages/admin-next/dashboard/src/routes/no-match/no-match.tsx +++ b/packages/admin-next/dashboard/src/routes/no-match/no-match.tsx @@ -1,4 +1,36 @@ +import { ExclamationCircle } from "@medusajs/icons" +import { Button, Text } from "@medusajs/ui" +import { useTranslation } from "react-i18next" +import { Link } from "react-router-dom" + // TODO: Add 404 page export const NoMatch = () => { - return
    404 Not Found
    + const { t } = useTranslation() + + const title = t("errorBoundary.notFoundTitle") + const message = t("errorBoundary.noMatchMessage") + + return ( +
    +
    +
    + +
    + + {title} + + + {message} + +
    +
    + +
    +
    + ) }