From 58f9a55ff1add5a0d8cb958a2bf1829f23b27c0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Mon, 9 Mar 2026 15:47:59 +0000 Subject: [PATCH 1/7] feat(ErrorBoundary): add error boundary With route and unexpected error handling. Uses StackTracey to display source-mapped, first-party-only stack traces. --- web/package-lock.json | 51 ++++++++ web/package.json | 1 + .../components/core/ErrorBoundary.test.tsx | 116 +++++++++++++++++ web/src/components/core/ErrorBoundary.tsx | 117 ++++++++++++++++++ web/src/components/layout/Icon.tsx | 2 + web/src/router.tsx | 2 + web/src/test-utils.tsx | 13 ++ 7 files changed, 302 insertions(+) create mode 100644 web/src/components/core/ErrorBoundary.test.tsx create mode 100644 web/src/components/core/ErrorBoundary.tsx diff --git a/web/package-lock.json b/web/package-lock.json index 965bf1155b..c187749290 100644 --- a/web/package-lock.json +++ b/web/package-lock.json @@ -21,6 +21,7 @@ "react-dom": "^18.3.1", "react-router": "^7.13.1", "sprintf-js": "^1.1.3", + "stacktracey": "^2.1.8", "xbytes": "^1.9.1" }, "devDependencies": { @@ -6553,6 +6554,15 @@ "url": "https://github.com/sponsors/ljharb" } }, + "node_modules/as-table": { + "version": "1.0.55", + "resolved": "https://registry.npmjs.org/as-table/-/as-table-1.0.55.tgz", + "integrity": "sha512-xvsWESUJn0JN421Xb9MQw6AsMHRCUknCe0Wjlxvjud80mU4E6hQf1A6NzQKcYNmYw62MfzEtXc+badstZP3JpQ==", + "license": "MIT", + "dependencies": { + "printable-characters": "^1.0.42" + } + }, "node_modules/asn1js": { "version": "3.0.7", "resolved": "https://registry.npmjs.org/asn1js/-/asn1js-3.0.7.tgz", @@ -8020,6 +8030,12 @@ "dev": true, "license": "MIT" }, + "node_modules/data-uri-to-buffer": { + "version": "2.0.2", + "resolved": "https://registry.npmjs.org/data-uri-to-buffer/-/data-uri-to-buffer-2.0.2.tgz", + "integrity": "sha512-ND9qDTLc6diwj+Xe5cdAgVTbLVdXbtxTJRXRhli8Mowuaan+0EJOtdqJ0QCHNSSPyoXGx9HX2/VMnKeC34AChA==", + "license": "MIT" + }, "node_modules/data-urls": { "version": "5.0.0", "resolved": "https://registry.npmjs.org/data-urls/-/data-urls-5.0.0.tgz", @@ -10451,6 +10467,25 @@ "node": ">= 0.4" } }, + "node_modules/get-source": { + "version": "2.0.12", + "resolved": "https://registry.npmjs.org/get-source/-/get-source-2.0.12.tgz", + "integrity": "sha512-X5+4+iD+HoSeEED+uwrQ07BOQr0kEDFMVqqpBuI+RaZBpBpHCuXxo70bjar6f0b0u/DQJsJ7ssurpP0V60Az+w==", + "license": "Unlicense", + "dependencies": { + "data-uri-to-buffer": "^2.0.0", + "source-map": "^0.6.1" + } + }, + "node_modules/get-source/node_modules/source-map": { + "version": "0.6.1", + "resolved": "https://registry.npmjs.org/source-map/-/source-map-0.6.1.tgz", + "integrity": "sha512-UjgapumWlbMhkBgzT7Ykc5YXUT46F0iKu8SGXq0bcwP5dz/h0Plj6enJqjz1Zbq2l5WaqYnrVbwWOWMyF3F47g==", + "license": "BSD-3-Clause", + "engines": { + "node": ">=0.10.0" + } + }, "node_modules/get-stream": { "version": "6.0.1", "resolved": "https://registry.npmjs.org/get-stream/-/get-stream-6.0.1.tgz", @@ -15233,6 +15268,12 @@ "url": "https://github.com/chalk/ansi-styles?sponsor=1" } }, + "node_modules/printable-characters": { + "version": "1.0.42", + "resolved": "https://registry.npmjs.org/printable-characters/-/printable-characters-1.0.42.tgz", + "integrity": "sha512-dKp+C4iXWK4vVYZmYSd0KBH5F/h1HoZRsbJ82AVKRO3PEo8L4lBS/vLwhVtpwwuYcoIsVY+1JYKR268yn480uQ==", + "license": "Unlicense" + }, "node_modules/process-nextick-args": { "version": "2.0.1", "resolved": "https://registry.npmjs.org/process-nextick-args/-/process-nextick-args-2.0.1.tgz", @@ -16608,6 +16649,16 @@ "dev": true, "license": "MIT" }, + "node_modules/stacktracey": { + "version": "2.1.8", + "resolved": "https://registry.npmjs.org/stacktracey/-/stacktracey-2.1.8.tgz", + "integrity": "sha512-Kpij9riA+UNg7TnphqjH7/CzctQ/owJGNbFkfEeve4Z4uxT5+JapVLFXcsurIfN34gnTWZNJ/f7NMG0E8JDzTw==", + "license": "Unlicense", + "dependencies": { + "as-table": "^1.0.36", + "get-source": "^2.0.12" + } + }, "node_modules/statuses": { "version": "2.0.2", "resolved": "https://registry.npmjs.org/statuses/-/statuses-2.0.2.tgz", diff --git a/web/package.json b/web/package.json index 6145d46bce..1b8cabd259 100644 --- a/web/package.json +++ b/web/package.json @@ -105,6 +105,7 @@ "react-dom": "^18.3.1", "react-router": "^7.13.1", "sprintf-js": "^1.1.3", + "stacktracey": "^2.1.8", "xbytes": "^1.9.1" } } diff --git a/web/src/components/core/ErrorBoundary.test.tsx b/web/src/components/core/ErrorBoundary.test.tsx new file mode 100644 index 0000000000..1731b74a3d --- /dev/null +++ b/web/src/components/core/ErrorBoundary.test.tsx @@ -0,0 +1,116 @@ +/* + * Copyright (c) [2022-2026] SUSE LLC + * + * All Rights Reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the Free + * Software Foundation; either version 2 of the License, or (at your option) + * any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, contact SUSE LLC. + * + * To contact SUSE LLC about this file by physical or electronic mail, you may + * find current contact information at www.suse.com. + */ + +import React from "react"; +import { screen } from "@testing-library/react"; +import { installerRender, mockRouteError } from "~/test-utils"; +import ErrorBoundary from "./ErrorBoundary"; + +jest.mock("stacktracey", () => + jest.fn().mockImplementation(() => ({ + withSources: jest.fn().mockReturnThis(), + filter: jest.fn().mockReturnThis(), + asTable: jest.fn().mockReturnValue("app.ts:10 myFunc\napp.ts:20 caller"), + })), +); + +const routeError = (status: number, statusText: string, data: unknown) => ({ + __isRouteError: true, + status, + statusText, + data, +}); + +describe("ErrorBoundary", () => { + describe("when the error is a route error response", () => { + describe("when it is a 404", () => { + beforeEach(() => { + mockRouteError(routeError(404, "Not Found", null)); + }); + + it("shows the HTTP status and statusText", () => { + installerRender(); + screen.getByText("404 Not Found"); + }); + + it("does not show a stack trace", () => { + installerRender(); + expect(screen.queryByRole("code")).not.toBeInTheDocument(); + }); + }); + + describe("when it carries a data payload", () => { + beforeEach(() => { + mockRouteError(routeError(403, "Forbidden", "You do not have access")); + }); + + it("shows the data payload", () => { + installerRender(); + screen.getByText("You do not have access"); + }); + }); + }); + + describe("when the error is an unexpected error", () => { + describe("when it is a standard Error", () => { + beforeEach(() => { + mockRouteError(new Error("Something exploded")); + }); + + it("shows the 'Unexpected error' heading", () => { + installerRender(); + screen.getByText("Unexpected error"); + }); + + it("shows the error message", () => { + installerRender(); + screen.getByText("Something exploded"); + }); + + it("shows the stack trace", () => { + installerRender(); + screen.getByText(/app\.ts:10.*myFunc/); + }); + }); + + describe("when the thrown value is not an Error instance", () => { + beforeEach(() => { + mockRouteError("a plain string"); + }); + + it("shows the 'Unexpected error' heading", () => { + installerRender(); + screen.getByText("Unexpected error"); + }); + + it("shows 'Unknown error' as the message", () => { + installerRender(); + screen.getByText("Unknown error"); + }); + + it("does not show a stack trace", () => { + installerRender(); + expect(screen.queryByRole("code")).not.toBeInTheDocument(); + }); + }); + }); +}); diff --git a/web/src/components/core/ErrorBoundary.tsx b/web/src/components/core/ErrorBoundary.tsx new file mode 100644 index 0000000000..462979f9f1 --- /dev/null +++ b/web/src/components/core/ErrorBoundary.tsx @@ -0,0 +1,117 @@ +/* + * Copyright (c) [2022-2026] SUSE LLC + * + * All Rights Reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the Free + * Software Foundation; either version 2 of the License, or (at your option) + * any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, contact SUSE LLC. + * + * To contact SUSE LLC about this file by physical or electronic mail, you may + * find current contact information at www.suse.com. + */ + +import React from "react"; +import { isRouteErrorResponse, useRouteError, ErrorResponse } from "react-router"; +import Page from "~/components/core/Page"; +import Text from "~/components/core/Text"; +import SplitInfoLayout from "~/components/layout/SplitInfoLayout"; +import { _ } from "~/i18n"; +import StackTracey from "stacktracey"; +import { Content } from "@patternfly/react-core"; +import NestedContent from "./NestedContent"; + +/** + * Rendered when React Router surfaces an `ErrorResponse`. + * + * Typically a 4xx/5xx thrown from a loader or action via `json()`, or an + * automatic 404 for an unmatched route. + */ +function RouteError({ error }: { error: ErrorResponse }) { + return ( + + + {error.data} + + + } + /> + ); +} + +/** + * Rendered for any error that is not a React Router `ErrorResponse` + * + * Most commonly an unhandled `Error` thrown inside a component, loader, or + * action. + * + * The error's `.message` is shown as the primary heading. When the value is a + * proper `Error` instance, its stack trace is parsed by **StackTracey**, + * enriched with original source locations via `.withSources()`, stripped of + * third-party frames, and formatted as a plain-text table via `.asTable()`. + * + * ### Dependency note + * + * `.asTable()` delegates to `as-table`, which in turn depends only on + * `printable-characters` (no further dependencies), and both are from + * the same share author as `stacktracey`. It can be dropped at any point if any + * problem is found, but for now it provided better formatted error messages + * at almost no cost. + */ +function UnexpectedError({ error }: { error: unknown }) { + const message = error instanceof Error ? error.message : _("Unknown error"); + const trace = + error instanceof Error + ? new StackTracey(error.stack) + .withSources() + .filter((x) => !x.thirdParty) + .asTable() + : null; + + return ( + + + {message} + + {trace && {trace}} + + } + /> + ); +} + +/** + * Top-level error boundary rendered by React Router when an unhandled error + * propagates out of a route's component tree, loader, or action. + */ +export default function ErrorBoundary() { + const error = useRouteError(); + return ( + + + {isRouteErrorResponse(error) ? ( + + ) : ( + + )} + + + ); +} diff --git a/web/src/components/layout/Icon.tsx b/web/src/components/layout/Icon.tsx index 38a30e1850..08e3c5a3a8 100644 --- a/web/src/components/layout/Icon.tsx +++ b/web/src/components/layout/Icon.tsx @@ -37,6 +37,7 @@ import ChevronRight from "@icons/chevron_right.svg?component"; import Delete from "@icons/delete.svg?component"; import DoneAll from "@icons/done_all.svg?component"; import DeployedCodeUpdate from "@icons/deployed_code_update.svg?component"; +import DeployedCodeAlert from "@icons/deployed_code_alert.svg?component"; import Download from "@icons/download.svg?component"; import EditSquare from "@icons/edit_square.svg?component"; import Emergency from "@icons/emergency.svg?component"; @@ -85,6 +86,7 @@ const icons = { delete: Delete, done_all: DoneAll, deployed_code_update: DeployedCodeUpdate, + deployed_code_alert: DeployedCodeAlert, download: Download, edit_square: EditSquare, emergency: Emergency, diff --git a/web/src/router.tsx b/web/src/router.tsx index bf69bdd606..a9300fb11b 100644 --- a/web/src/router.tsx +++ b/web/src/router.tsx @@ -30,6 +30,7 @@ import { InstallationProgress, LoginPage, } from "~/components/core"; +import ErrorBoundary from "./components/core/ErrorBoundary"; import StorageProgress from "~/components/storage/Progress"; import HostnamePage from "~/components/system/HostnamePage"; import OverviewPage from "~/components/overview/OverviewPage"; @@ -112,6 +113,7 @@ const router = () => { path: PATHS.root, element: , + ErrorBoundary, children: [...protectedRoutes()], }, ]); diff --git a/web/src/test-utils.tsx b/web/src/test-utils.tsx index 57f66c2b24..81547aa5e4 100644 --- a/web/src/test-utils.tsx +++ b/web/src/test-utils.tsx @@ -92,6 +92,16 @@ const mockRoutes = (...routes) => initialRoutes.mockReturnValueOnce(routes); */ const mockParams = (params: ReturnType) => (paramsMock = params); +/** + * Allows mocking the error returned by useRouteError for testing error boundaries + * + * @example + * mockRouteError(new Error("Something exploded")); + * mockRouteError({ __isRouteError: true, status: 404, statusText: "Not Found", data: null }); + */ +const mockRouteErrorFn = jest.fn(); +const mockRouteError = (error: unknown) => mockRouteErrorFn.mockReturnValue(error); + // Centralize the react-router mock here jest.mock("react-router", () => ({ ...jest.requireActual("react-router"), @@ -107,6 +117,8 @@ jest.mock("react-router", () => ({ () => { to; }, + useRouteError: () => mockRouteErrorFn(), + isRouteErrorResponse: (e: unknown) => e instanceof Object && "__isRouteError" in e, })); /** @@ -387,6 +399,7 @@ export { mockNavigateFn, mockParams, mockRoutes, + mockRouteError, mockUseRevalidator, resetLocalStorage, getColumnValues, From 443407fd10561699453ed40740e4d541bbca8f45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Mon, 9 Mar 2026 18:45:31 +0000 Subject: [PATCH 2/7] fix(web): rename and reorganization --- ...orBoundary.test.tsx => ErrorPage.test.tsx} | 22 +++++++++---------- .../core/{ErrorBoundary.tsx => ErrorPage.tsx} | 9 ++++---- web/src/router.tsx | 18 +++++++-------- 3 files changed, 24 insertions(+), 25 deletions(-) rename web/src/components/core/{ErrorBoundary.test.tsx => ErrorPage.test.tsx} (86%) rename web/src/components/core/{ErrorBoundary.tsx => ErrorPage.tsx} (97%) diff --git a/web/src/components/core/ErrorBoundary.test.tsx b/web/src/components/core/ErrorPage.test.tsx similarity index 86% rename from web/src/components/core/ErrorBoundary.test.tsx rename to web/src/components/core/ErrorPage.test.tsx index 1731b74a3d..442f0d13ac 100644 --- a/web/src/components/core/ErrorBoundary.test.tsx +++ b/web/src/components/core/ErrorPage.test.tsx @@ -23,7 +23,7 @@ import React from "react"; import { screen } from "@testing-library/react"; import { installerRender, mockRouteError } from "~/test-utils"; -import ErrorBoundary from "./ErrorBoundary"; +import ErrorPage from "./ErrorPage"; jest.mock("stacktracey", () => jest.fn().mockImplementation(() => ({ @@ -40,7 +40,7 @@ const routeError = (status: number, statusText: string, data: unknown) => ({ data, }); -describe("ErrorBoundary", () => { +describe("ErrorPage", () => { describe("when the error is a route error response", () => { describe("when it is a 404", () => { beforeEach(() => { @@ -48,12 +48,12 @@ describe("ErrorBoundary", () => { }); it("shows the HTTP status and statusText", () => { - installerRender(); + installerRender(); screen.getByText("404 Not Found"); }); it("does not show a stack trace", () => { - installerRender(); + installerRender(); expect(screen.queryByRole("code")).not.toBeInTheDocument(); }); }); @@ -64,7 +64,7 @@ describe("ErrorBoundary", () => { }); it("shows the data payload", () => { - installerRender(); + installerRender(); screen.getByText("You do not have access"); }); }); @@ -77,17 +77,17 @@ describe("ErrorBoundary", () => { }); it("shows the 'Unexpected error' heading", () => { - installerRender(); + installerRender(); screen.getByText("Unexpected error"); }); it("shows the error message", () => { - installerRender(); + installerRender(); screen.getByText("Something exploded"); }); it("shows the stack trace", () => { - installerRender(); + installerRender(); screen.getByText(/app\.ts:10.*myFunc/); }); }); @@ -98,17 +98,17 @@ describe("ErrorBoundary", () => { }); it("shows the 'Unexpected error' heading", () => { - installerRender(); + installerRender(); screen.getByText("Unexpected error"); }); it("shows 'Unknown error' as the message", () => { - installerRender(); + installerRender(); screen.getByText("Unknown error"); }); it("does not show a stack trace", () => { - installerRender(); + installerRender(); expect(screen.queryByRole("code")).not.toBeInTheDocument(); }); }); diff --git a/web/src/components/core/ErrorBoundary.tsx b/web/src/components/core/ErrorPage.tsx similarity index 97% rename from web/src/components/core/ErrorBoundary.tsx rename to web/src/components/core/ErrorPage.tsx index 462979f9f1..48a020bac5 100644 --- a/web/src/components/core/ErrorBoundary.tsx +++ b/web/src/components/core/ErrorPage.tsx @@ -21,14 +21,14 @@ */ import React from "react"; +import StackTracey from "stacktracey"; import { isRouteErrorResponse, useRouteError, ErrorResponse } from "react-router"; +import { Content } from "@patternfly/react-core"; +import NestedContent from "~/components/core/NestedContent"; import Page from "~/components/core/Page"; import Text from "~/components/core/Text"; import SplitInfoLayout from "~/components/layout/SplitInfoLayout"; import { _ } from "~/i18n"; -import StackTracey from "stacktracey"; -import { Content } from "@patternfly/react-core"; -import NestedContent from "./NestedContent"; /** * Rendered when React Router surfaces an `ErrorResponse`. @@ -101,8 +101,9 @@ function UnexpectedError({ error }: { error: unknown }) { * Top-level error boundary rendered by React Router when an unhandled error * propagates out of a route's component tree, loader, or action. */ -export default function ErrorBoundary() { +export default function ErrorPage() { const error = useRouteError(); + return ( diff --git a/web/src/router.tsx b/web/src/router.tsx index a9300fb11b..ae8c2d90c0 100644 --- a/web/src/router.tsx +++ b/web/src/router.tsx @@ -24,16 +24,14 @@ import React from "react"; import { createHashRouter, Outlet } from "react-router"; import App from "~/App"; import Protected from "~/Protected"; -import { - InstallationExit, - InstallationFinished, - InstallationProgress, - LoginPage, -} from "~/components/core"; -import ErrorBoundary from "./components/core/ErrorBoundary"; -import StorageProgress from "~/components/storage/Progress"; -import HostnamePage from "~/components/system/HostnamePage"; +import ErrorPage from "~/components/core/ErrorPage"; +import InstallationExit from "~/components/core/InstallationExit"; +import InstallationFinished from "~/components/core/InstallationFinished"; +import InstallationProgress from "~/components/core/InstallationProgress"; +import LoginPage from "~/components/core/LoginPage"; import OverviewPage from "~/components/overview/OverviewPage"; +import HostnamePage from "~/components/system/HostnamePage"; +import StorageProgress from "~/components/storage/Progress"; import l10nRoutes from "~/routes/l10n"; import networkRoutes from "~/routes/network"; import productsRoutes from "~/routes/products"; @@ -113,7 +111,7 @@ const router = () => { path: PATHS.root, element: , - ErrorBoundary, + ErrorBoundary: ErrorPage, children: [...protectedRoutes()], }, ]); From 53e580590b6eb0d30f814c32c3765a914845cbb8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Mon, 9 Mar 2026 19:22:07 +0000 Subject: [PATCH 3/7] fix(web): use withSourcesAsync instead of withSources As "recommended" in the StackTracey README file. --- web/src/components/core/ErrorPage.test.tsx | 32 ++++++++++++----- web/src/components/core/ErrorPage.tsx | 40 ++++++++++++++++------ 2 files changed, 53 insertions(+), 19 deletions(-) diff --git a/web/src/components/core/ErrorPage.test.tsx b/web/src/components/core/ErrorPage.test.tsx index 442f0d13ac..c95e60814e 100644 --- a/web/src/components/core/ErrorPage.test.tsx +++ b/web/src/components/core/ErrorPage.test.tsx @@ -27,9 +27,10 @@ import ErrorPage from "./ErrorPage"; jest.mock("stacktracey", () => jest.fn().mockImplementation(() => ({ - withSources: jest.fn().mockReturnThis(), - filter: jest.fn().mockReturnThis(), - asTable: jest.fn().mockReturnValue("app.ts:10 myFunc\napp.ts:20 caller"), + withSourcesAsync: jest.fn().mockResolvedValue({ + filter: jest.fn().mockReturnThis(), + asTable: jest.fn().mockReturnValue("app.ts:10 myFunc\napp.ts:20 caller"), + }), })), ); @@ -76,19 +77,33 @@ describe("ErrorPage", () => { mockRouteError(new Error("Something exploded")); }); - it("shows the 'Unexpected error' heading", () => { + it("shows the 'Unexpected error' heading", async () => { installerRender(); screen.getByText("Unexpected error"); + await screen.findByText(/app\.ts:10.*myFunc/); }); - it("shows the error message", () => { + it("shows the error message", async () => { installerRender(); screen.getByText("Something exploded"); + await screen.findByText(/app\.ts:10.*myFunc/); + }); + + it("shows a skeleton while the trace is loading", async () => { + installerRender(); + screen.getByText("Retrieving error details"); + await screen.findByText(/app\.ts:10.*myFunc/); }); - it("shows the stack trace", () => { + it("shows the stack trace once loaded", async () => { installerRender(); - screen.getByText(/app\.ts:10.*myFunc/); + await screen.findByText(/app\.ts:10.*myFunc/); + }); + + it("hides the skeleton once the trace is loaded", async () => { + installerRender(); + await screen.findByText(/app\.ts:10.*myFunc/); + expect(screen.queryByText("Retrieving error details")).not.toBeInTheDocument(); }); }); @@ -107,8 +122,9 @@ describe("ErrorPage", () => { screen.getByText("Unknown error"); }); - it("does not show a stack trace", () => { + it("does not show a skeleton or stack trace", () => { installerRender(); + expect(screen.queryByText("Retrieving error details")).not.toBeInTheDocument(); expect(screen.queryByRole("code")).not.toBeInTheDocument(); }); }); diff --git a/web/src/components/core/ErrorPage.tsx b/web/src/components/core/ErrorPage.tsx index 48a020bac5..34c7166b65 100644 --- a/web/src/components/core/ErrorPage.tsx +++ b/web/src/components/core/ErrorPage.tsx @@ -20,10 +20,10 @@ * find current contact information at www.suse.com. */ -import React from "react"; +import React, { useEffect, useState } from "react"; import StackTracey from "stacktracey"; import { isRouteErrorResponse, useRouteError, ErrorResponse } from "react-router"; -import { Content } from "@patternfly/react-core"; +import { Content, Skeleton, Stack } from "@patternfly/react-core"; import NestedContent from "~/components/core/NestedContent"; import Page from "~/components/core/Page"; import Text from "~/components/core/Text"; @@ -60,7 +60,7 @@ function RouteError({ error }: { error: ErrorResponse }) { * * The error's `.message` is shown as the primary heading. When the value is a * proper `Error` instance, its stack trace is parsed by **StackTracey**, - * enriched with original source locations via `.withSources()`, stripped of + * enriched with original source locations via `.withSourcesAsync()`, stripped of * third-party frames, and formatted as a plain-text table via `.asTable()`. * * ### Dependency note @@ -72,14 +72,17 @@ function RouteError({ error }: { error: ErrorResponse }) { * at almost no cost. */ function UnexpectedError({ error }: { error: unknown }) { + const [trace, setTrace] = useState(null); const message = error instanceof Error ? error.message : _("Unknown error"); - const trace = - error instanceof Error - ? new StackTracey(error.stack) - .withSources() - .filter((x) => !x.thirdParty) - .asTable() - : null; + + useEffect(() => { + if (!(error instanceof Error)) return; + + const stackTracey = new StackTracey(error.stack); + stackTracey + .withSourcesAsync() + .then((stack) => setTrace(stack.filter((x) => !x.thirdParty).asTable())); + }, [error]); return ( {message} - {trace && {trace}} + {error instanceof Error && + (trace ? ( + {trace} + ) : ( + + + + + + + + ))} } /> From 34b9cd0fe1e645477e58a088abd35b92226db72f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Mon, 9 Mar 2026 19:46:36 +0000 Subject: [PATCH 4/7] doc(web): add entry in changes file --- web/package/agama-web-ui.changes | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/web/package/agama-web-ui.changes b/web/package/agama-web-ui.changes index a7b809aa25..b8019a975c 100644 --- a/web/package/agama-web-ui.changes +++ b/web/package/agama-web-ui.changes @@ -1,3 +1,9 @@ +------------------------------------------------------------------- +Mon Mar 9 19:44:43 UTC 2026 - David Diaz + +- Improve error page with filtered, source-mapped stack traces + for better error reporting (gh#agama-project/agama#3261). + ------------------------------------------------------------------- Mon Mar 9 11:05:21 UTC 2026 - José Iván López González From 346223a7de60f3a720f8e4533da656243c7e06c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Tue, 10 Mar 2026 09:29:39 +0000 Subject: [PATCH 5/7] fix(web): improvements based on code review - Extract ErrorTrace component to handle stack trace rendering and loading state for making code easier to read and maintain. - Extract relevantTrace helper to encapsulate first-party frame filtering with fallback to full stack when all frames are third-party - Use isError from radashi instead of instanceof Error --- web/src/components/core/ErrorPage.test.tsx | 38 +++++-- web/src/components/core/ErrorPage.tsx | 116 ++++++++++++++------- 2 files changed, 105 insertions(+), 49 deletions(-) diff --git a/web/src/components/core/ErrorPage.test.tsx b/web/src/components/core/ErrorPage.test.tsx index c95e60814e..544baadf58 100644 --- a/web/src/components/core/ErrorPage.test.tsx +++ b/web/src/components/core/ErrorPage.test.tsx @@ -31,6 +31,8 @@ jest.mock("stacktracey", () => filter: jest.fn().mockReturnThis(), asTable: jest.fn().mockReturnValue("app.ts:10 myFunc\napp.ts:20 caller"), }), + filter: jest.fn().mockReturnThis(), + asTable: jest.fn().mockReturnValue("app.ts:10 myFunc\napp.ts:20 caller"), })), ); @@ -53,9 +55,9 @@ describe("ErrorPage", () => { screen.getByText("404 Not Found"); }); - it("does not show a stack trace", () => { + it("does not show a skeleton", () => { installerRender(); - expect(screen.queryByRole("code")).not.toBeInTheDocument(); + expect(screen.queryByText("Retrieving error details")).not.toBeInTheDocument(); }); }); @@ -107,25 +109,43 @@ describe("ErrorPage", () => { }); }); + describe("when withSourcesAsync fails", () => { + beforeEach(() => { + const StackTracey = require("stacktracey"); + StackTracey.mockImplementationOnce(() => ({ + withSourcesAsync: jest.fn().mockRejectedValue(new Error("network error")), + filter: jest.fn().mockReturnThis(), + asTable: jest.fn().mockReturnValue("app.ts:10 myFunc (no sources)"), + })); + mockRouteError(new Error("Something exploded")); + }); + + it("falls back to the raw stack table", async () => { + installerRender(); + await screen.findByText(/app\.ts:10.*myFunc \(no sources\)/); + }); + }); + describe("when the thrown value is not an Error instance", () => { beforeEach(() => { - mockRouteError("a plain string"); + mockRouteError({ code: 42, reason: "unknown" }); }); - it("shows the 'Unexpected error' heading", () => { + it("shows the 'Something went wrong' heading", async () => { installerRender(); - screen.getByText("Unexpected error"); + screen.getByText("Something went wrong"); + await screen.findByText(/\"code\":42/); }); - it("shows 'Unknown error' as the message", () => { + it("shows 'Unknown error' as the message", async () => { installerRender(); screen.getByText("Unknown error"); + await screen.findByText(/\"code\":42/); }); - it("does not show a skeleton or stack trace", () => { + it("shows the JSON-serialised value", async () => { installerRender(); - expect(screen.queryByText("Retrieving error details")).not.toBeInTheDocument(); - expect(screen.queryByRole("code")).not.toBeInTheDocument(); + await screen.findByText(/\"code\":42/); }); }); }); diff --git a/web/src/components/core/ErrorPage.tsx b/web/src/components/core/ErrorPage.tsx index 34c7166b65..4cff8a9324 100644 --- a/web/src/components/core/ErrorPage.tsx +++ b/web/src/components/core/ErrorPage.tsx @@ -22,6 +22,7 @@ import React, { useEffect, useState } from "react"; import StackTracey from "stacktracey"; +import { isError } from "radashi"; import { isRouteErrorResponse, useRouteError, ErrorResponse } from "react-router"; import { Content, Skeleton, Stack } from "@patternfly/react-core"; import NestedContent from "~/components/core/NestedContent"; @@ -30,6 +31,64 @@ import Text from "~/components/core/Text"; import SplitInfoLayout from "~/components/layout/SplitInfoLayout"; import { _ } from "~/i18n"; +/** + * Returns first-party frames from a stack, falling back to the full stack + * if filtering leaves nothing (e.g. all frames are third-party). + */ +const relevantTrace = (stack: StackTracey) => stack.filter((x) => !x.thirdParty) || stack; + +/** + * Renders a formatted, source-mapped stack trace for any thrown value. + * + * Uses **StackTracey** to parse and enrich the stack with original source + * locations via `.withSourcesAsync()`. {@link relevantFrames} filters out + * third-party frames, falling back to the full stack if none remain. + * If `.withSourcesAsync()` rejects (e.g. source maps unavailable), the + * same filtering is applied to the raw stack instead. + * + * For non-`Error` values (checked via `isError` from **radashi**), the value + * is JSON-serialised and displayed as-is. + * + * A skeleton placeholder is shown while async resolution is in progress. + * + * ### Dependency note + * + * `.asTable()` delegates to `as-table`, which in turn depends only on + * `printable-characters` (no further dependencies) — all three packages share + * the same author as `stacktracey`. It can be dropped at any point if any + * problem is found, but for now it gives us better formatted error messages + * at almost no cost. + */ +function ErrorTrace({ error }) { + const [trace, setTrace] = useState(null); + + useEffect(() => { + if (isError(error)) { + const stackTracey = new StackTracey(error); + stackTracey + .withSourcesAsync() + .then((stack) => { + setTrace(relevantTrace(stack).asTable()); + }) + .catch(() => { + setTrace(relevantTrace(stackTracey).asTable()); + }); + } else { + setTrace(JSON.stringify(error)); + } + }, [error]); + + if (trace) return trace; + + return ( + + + + + + ); +} + /** * Rendered when React Router surfaces an `ErrorResponse`. * @@ -53,62 +112,39 @@ function RouteError({ error }: { error: ErrorResponse }) { } /** - * Rendered for any error that is not a React Router `ErrorResponse` + * Rendered for any error that is not a React Router `ErrorResponse`. * * Most commonly an unhandled `Error` thrown inside a component, loader, or * action. * - * The error's `.message` is shown as the primary heading. When the value is a - * proper `Error` instance, its stack trace is parsed by **StackTracey**, - * enriched with original source locations via `.withSourcesAsync()`, stripped of - * third-party frames, and formatted as a plain-text table via `.asTable()`. - * - * ### Dependency note + * The heading varies depending on the thrown value: + * - `"Unexpected error"` for proper `Error` instances (checked via `isError` + * from **radashi**), with the error's `.message` shown below. + * - `"Something went wrong"` for any other value (plain objects, strings, + * etc.), with `"Unknown error"` as the message. * - * `.asTable()` delegates to `as-table`, which in turn depends only on - * `printable-characters` (no further dependencies), and both are from - * the same share author as `stacktracey`. It can be dropped at any point if any - * problem is found, but for now it provided better formatted error messages - * at almost no cost. + * In both cases {@link ErrorTrace} renders below the message, showing a + * skeleton placeholder while source-map enrichment is in progress. */ function UnexpectedError({ error }: { error: unknown }) { - const [trace, setTrace] = useState(null); - const message = error instanceof Error ? error.message : _("Unknown error"); - - useEffect(() => { - if (!(error instanceof Error)) return; - - const stackTracey = new StackTracey(error.stack); - stackTracey - .withSourcesAsync() - .then((stack) => setTrace(stack.filter((x) => !x.thirdParty).asTable())); - }, [error]); + const isAnError = isError(error); + const title = isAnError ? _("Unexpected error") : _("Something went wrong"); + const message = isAnError ? error.message : _("Unknown error"); return ( {message} - {error instanceof Error && - (trace ? ( - {trace} - ) : ( - - - - - - - - ))} + + + + + } /> From 82c0f87f890d6bb194ae01fb1f9b0dca55378024 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Tue, 10 Mar 2026 09:41:07 +0000 Subject: [PATCH 6/7] fix(web): serialize non-string RouteError data payload as JSON --- web/src/components/core/ErrorPage.test.tsx | 17 +++++++++++++++-- web/src/components/core/ErrorPage.tsx | 7 +++++-- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/web/src/components/core/ErrorPage.test.tsx b/web/src/components/core/ErrorPage.test.tsx index 544baadf58..1226cc0df2 100644 --- a/web/src/components/core/ErrorPage.test.tsx +++ b/web/src/components/core/ErrorPage.test.tsx @@ -61,16 +61,29 @@ describe("ErrorPage", () => { }); }); - describe("when it carries a data payload", () => { + describe("when the data payload is a string", () => { beforeEach(() => { mockRouteError(routeError(403, "Forbidden", "You do not have access")); }); - it("shows the data payload", () => { + it("shows the data payload as-is", () => { installerRender(); screen.getByText("You do not have access"); }); }); + + describe("when the data payload is not a string", () => { + beforeEach(() => { + mockRouteError( + routeError(422, "Unprocessable Entity", { field: "email", issue: "invalid" }), + ); + }); + + it("shows the JSON-serialised payload", () => { + installerRender(); + screen.getByText(/\"field\":\"email\"/); + }); + }); }); describe("when the error is an unexpected error", () => { diff --git a/web/src/components/core/ErrorPage.tsx b/web/src/components/core/ErrorPage.tsx index 4cff8a9324..ba50746a4b 100644 --- a/web/src/components/core/ErrorPage.tsx +++ b/web/src/components/core/ErrorPage.tsx @@ -22,7 +22,7 @@ import React, { useEffect, useState } from "react"; import StackTracey from "stacktracey"; -import { isError } from "radashi"; +import { isError, isString } from "radashi"; import { isRouteErrorResponse, useRouteError, ErrorResponse } from "react-router"; import { Content, Skeleton, Stack } from "@patternfly/react-core"; import NestedContent from "~/components/core/NestedContent"; @@ -94,6 +94,9 @@ function ErrorTrace({ error }) { * * Typically a 4xx/5xx thrown from a loader or action via `json()`, or an * automatic 404 for an unmatched route. + * + * The `data` payload is rendered as-is when it is a string, or + * JSON-serialised otherwise. */ function RouteError({ error }: { error: ErrorResponse }) { return ( @@ -103,7 +106,7 @@ function RouteError({ error }: { error: ErrorResponse }) { firstRowEnd={ - {error.data} + {isString(error.data) ? error.data : JSON.stringify(error.data)} } From 0a7045dbd4da1f03210e5846638b6840cdbb8422 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Tue, 10 Mar 2026 10:08:57 +0000 Subject: [PATCH 7/7] fix(web): please ESLint --- web/src/components/core/ErrorPage.test.tsx | 33 +++++++++++----------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/web/src/components/core/ErrorPage.test.tsx b/web/src/components/core/ErrorPage.test.tsx index 1226cc0df2..c08e657fcb 100644 --- a/web/src/components/core/ErrorPage.test.tsx +++ b/web/src/components/core/ErrorPage.test.tsx @@ -25,16 +25,8 @@ import { screen } from "@testing-library/react"; import { installerRender, mockRouteError } from "~/test-utils"; import ErrorPage from "./ErrorPage"; -jest.mock("stacktracey", () => - jest.fn().mockImplementation(() => ({ - withSourcesAsync: jest.fn().mockResolvedValue({ - filter: jest.fn().mockReturnThis(), - asTable: jest.fn().mockReturnValue("app.ts:10 myFunc\napp.ts:20 caller"), - }), - filter: jest.fn().mockReturnThis(), - asTable: jest.fn().mockReturnValue("app.ts:10 myFunc\napp.ts:20 caller"), - })), -); +jest.mock("stacktracey", () => jest.fn()); +const mockStackTracey = jest.requireMock("stacktracey"); const routeError = (status: number, statusText: string, data: unknown) => ({ __isRouteError: true, @@ -44,6 +36,16 @@ const routeError = (status: number, statusText: string, data: unknown) => ({ }); describe("ErrorPage", () => { + beforeEach(() => { + mockStackTracey.mockImplementation(() => ({ + withSourcesAsync: jest.fn().mockResolvedValue({ + filter: jest.fn().mockReturnThis(), + asTable: jest.fn().mockReturnValue("app.ts:10 myFunc\napp.ts:20 caller"), + }), + filter: jest.fn().mockReturnThis(), + asTable: jest.fn().mockReturnValue("app.ts:10 myFunc\napp.ts:20 caller"), + })); + }); describe("when the error is a route error response", () => { describe("when it is a 404", () => { beforeEach(() => { @@ -81,7 +83,7 @@ describe("ErrorPage", () => { it("shows the JSON-serialised payload", () => { installerRender(); - screen.getByText(/\"field\":\"email\"/); + screen.getByText(/"field":"email"/); }); }); }); @@ -124,8 +126,7 @@ describe("ErrorPage", () => { describe("when withSourcesAsync fails", () => { beforeEach(() => { - const StackTracey = require("stacktracey"); - StackTracey.mockImplementationOnce(() => ({ + mockStackTracey.mockImplementationOnce(() => ({ withSourcesAsync: jest.fn().mockRejectedValue(new Error("network error")), filter: jest.fn().mockReturnThis(), asTable: jest.fn().mockReturnValue("app.ts:10 myFunc (no sources)"), @@ -147,18 +148,18 @@ describe("ErrorPage", () => { it("shows the 'Something went wrong' heading", async () => { installerRender(); screen.getByText("Something went wrong"); - await screen.findByText(/\"code\":42/); + await screen.findByText(/"code":42/); }); it("shows 'Unknown error' as the message", async () => { installerRender(); screen.getByText("Unknown error"); - await screen.findByText(/\"code\":42/); + await screen.findByText(/"code":42/); }); it("shows the JSON-serialised value", async () => { installerRender(); - await screen.findByText(/\"code\":42/); + await screen.findByText(/"code":42/); }); }); });