From d31fb7cc3d99ddc1b6de4367009da1fb1496b0df Mon Sep 17 00:00:00 2001 From: Kevin Nguyen Date: Thu, 2 Jan 2025 15:58:30 -0600 Subject: [PATCH] fix(RV-426): Adds error framework --- frontend/package-lock.json | 15 +++- frontend/package.json | 1 + frontend/src/App.tsx | 5 +- frontend/src/components/ExtractUploadFile.tsx | 39 ++++++----- frontend/src/components/ReviewBulk.tsx | 13 +++- frontend/src/components/ReviewTable.tsx | 4 ++ frontend/src/contexts/ErrorContext.test.tsx | 70 +++++++++++++++++++ frontend/src/contexts/ErrorContext.tsx | 68 ++++++++++++++++++ frontend/src/error/ErrorBanner.scss | 47 +++++++++++++ frontend/src/error/ErrorBanner.tsx | 22 ++++++ frontend/src/error/ErrorBoundary.test.tsx | 59 ++++++++++++++++ frontend/src/error/ErrorBoundary.tsx | 68 ++++++++++++++++++ frontend/src/error/ErrorFallback.tsx | 28 ++++++++ frontend/src/main.tsx | 22 +++--- .../{404Page.test.tsx => ErrorPage.test.tsx} | 8 +-- .../src/pages/{404Page.tsx => ErrorPage.tsx} | 13 ++-- frontend/src/pages/ReviewTemplate.tsx | 8 ++- 17 files changed, 447 insertions(+), 43 deletions(-) create mode 100644 frontend/src/contexts/ErrorContext.test.tsx create mode 100644 frontend/src/contexts/ErrorContext.tsx create mode 100644 frontend/src/error/ErrorBanner.scss create mode 100644 frontend/src/error/ErrorBanner.tsx create mode 100644 frontend/src/error/ErrorBoundary.test.tsx create mode 100644 frontend/src/error/ErrorBoundary.tsx create mode 100644 frontend/src/error/ErrorFallback.tsx rename frontend/src/pages/{404Page.test.tsx => ErrorPage.test.tsx} (80%) rename frontend/src/pages/{404Page.tsx => ErrorPage.tsx} (74%) diff --git a/frontend/package-lock.json b/frontend/package-lock.json index ef16ebe5..061b107b 100644 --- a/frontend/package-lock.json +++ b/frontend/package-lock.json @@ -20,6 +20,7 @@ "prop-types": "^15.8.1", "react": "^18.3.1", "react-dom": "^18.3.1", + "react-error-boundary": "^5.0.0", "react-image-label": "^1.3.4", "react-router-dom": "^6.26.0", "zustand": "^5.0.1" @@ -191,7 +192,6 @@ "version": "7.25.0", "resolved": "https://registry.npmjs.org/@babel/runtime/-/runtime-7.25.0.tgz", "integrity": "sha512-7dRy4DwXwtzBrPbZflqxnvfxLF8kdZXPkhymtDeFoFqE6ldzjQFgYTtYIFARcLEYDrqfBfYcZt1WqFxRoyC9Rw==", - "dev": true, "license": "MIT", "dependencies": { "regenerator-runtime": "^0.14.0" @@ -5446,6 +5446,18 @@ "react": "^18.3.1" } }, + "node_modules/react-error-boundary": { + "version": "5.0.0", + "resolved": "https://registry.npmjs.org/react-error-boundary/-/react-error-boundary-5.0.0.tgz", + "integrity": "sha512-tnjAxG+IkpLephNcePNA7v6F/QpWLH8He65+DmedchDwg162JZqx4NmbXj0mlAYVVEd81OW7aFhmbsScYfiAFQ==", + "license": "MIT", + "dependencies": { + "@babel/runtime": "^7.12.5" + }, + "peerDependencies": { + "react": ">=16.13.1" + } + }, "node_modules/react-image-label": { "version": "1.3.4", "resolved": "https://registry.npmjs.org/react-image-label/-/react-image-label-1.3.4.tgz", @@ -5590,7 +5602,6 @@ "version": "0.14.1", "resolved": "https://registry.npmjs.org/regenerator-runtime/-/regenerator-runtime-0.14.1.tgz", "integrity": "sha512-dYnhHh0nJoMfnkZs6GmmhFknAGRrLznOu5nc9ML+EJxGvrx6H7teuevqVqCuPcPK//3eDrrjQhehXVx9cnkGdw==", - "dev": true, "license": "MIT" }, "node_modules/regexp.prototype.flags": { diff --git a/frontend/package.json b/frontend/package.json index d18448f5..8929280f 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -28,6 +28,7 @@ "prop-types": "^15.8.1", "react": "^18.3.1", "react-dom": "^18.3.1", + "react-error-boundary": "^5.0.0", "react-image-label": "^1.3.4", "react-router-dom": "^6.26.0", "zustand": "^5.0.1" diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index 316bcf11..1018b216 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -11,6 +11,7 @@ import "./App.scss"; import { useFiles } from "./contexts/FilesContext.tsx"; import { useQuery } from "@tanstack/react-query"; import { TemplateAPI } from "./types/templates.ts"; +import { useError } from "./contexts/ErrorContext.tsx"; function App() { const { pathname } = useLocation(); @@ -40,6 +41,7 @@ function App() { }; getTemplates(); }, [templateQuery.data]); + const { setError } = useError(); useEffect(() => { if (templates.length > 0) { @@ -51,7 +53,8 @@ function App() { useEffect(() => { setFiles([]); - },[]); + setError(null); + }, []); const navLinks = [ { text: "Annotate and Extract", url: "/" }, diff --git a/frontend/src/components/ExtractUploadFile.tsx b/frontend/src/components/ExtractUploadFile.tsx index 3faa91a9..255182ee 100644 --- a/frontend/src/components/ExtractUploadFile.tsx +++ b/frontend/src/components/ExtractUploadFile.tsx @@ -1,5 +1,5 @@ import React, { ChangeEvent, useEffect, useId, useState } from "react"; -import { Button, Icon, Select } from "@trussworks/react-uswds"; +import { Button, Select } from "@trussworks/react-uswds"; import { useFiles } from "../contexts/FilesContext"; import { useNavigate } from "react-router-dom"; import image from "../assets/green_check.svg"; @@ -11,6 +11,8 @@ import "./ExtractUploadFile.scss"; import { FileInput } from "./FileInput/file-input"; import { TemplateAPI } from "../types/templates.ts"; import { useQuery } from "@tanstack/react-query"; +import ErrorBanner from "../error/ErrorBanner.tsx"; +import { ERRORS, useError } from "../contexts/ErrorContext.tsx"; pdfjsLib.GlobalWorkerOptions.workerSrc = `//unpkg.com/pdfjs-dist@${pdfjsLib.version}/build/pdf.worker.min.mjs`; @@ -51,7 +53,7 @@ export const ExtractUploadFile: React.FC = ({ } = useFiles(); const navigate = useNavigate(); const [templates, setTemplates] = useState([]); - const [hasError, setHasError] = useState(false); + const { error, setError } = useError(); const [extractedImages, setExtractedImages] = useState([]); const _templates = useQuery({ @@ -181,7 +183,7 @@ export const ExtractUploadFile: React.FC = ({ tempSelectedTemplates.push(template); } - const error = tempSelectedTemplates.some((template) => { + const localErr = tempSelectedTemplates.some((template) => { const tempplate = templates.find( (tpl) => tpl.name === template.templateName, ); @@ -190,7 +192,11 @@ export const ExtractUploadFile: React.FC = ({ ); return tempplate?.pages.length !== extractedImage?.images.length; }); - setHasError(error); + if (localErr) { + setError(ERRORS.MISMATCH_ERROR); + } else { + setError(null); + } } if (selectedTemplates.length === 0) { @@ -198,9 +204,13 @@ export const ExtractUploadFile: React.FC = ({ const extractedImage = extractedImages.find( (img) => img.file === fileName, ); - const error = + const localErr = checkTemplate?.pages.length !== extractedImage?.images.length; - setHasError(error); + if (localErr) { + setError(ERRORS.MISMATCH_ERROR); + } else { + setError(null); + } } }; @@ -209,7 +219,6 @@ export const ExtractUploadFile: React.FC = ({ clearTemplates(); clearFiles(); }; - return (
@@ -221,18 +230,10 @@ export const ExtractUploadFile: React.FC = ({

Select one or more files

- {hasError && ( -
-
- -

Mismatch error:

-
-

- The uploaded file has a different number of pages than the template. - Please upload a file with the correct pages to proceed or choose a - different template. -

-
+ {error?.title && ( + )}
{ const navigate = useNavigate(); + const { error } = useError(); const onClick = (index: number) => { setIsReviewing(true); @@ -77,8 +80,13 @@ const ReviewBulk = ({ }; const handleCSVDownload = () => { - onDownload(); - navigate("/"); + try { + onDownload(); + navigate("/"); + } catch (error) { + console.error("Error downloading CSV", error); + } + } return ( @@ -118,6 +126,7 @@ const ReviewBulk = ({ Download CSV
+ {error?.title &&
}
{ const navigate = useNavigate(); + const { error } = useError(); const handleImageChange = (index: number) => { setIndex(index); }; @@ -136,6 +139,7 @@ const ReviewTable = ({ )}
+ {error?.title && }
+

+ {error ? error.message : "No error"} +

+ + +
+ ); +} + +describe("ErrorContext", () => { + it("should default to null error", () => { + render( + + + + ); + + const messageEl = screen.getByTestId("errorMessage"); + expect(messageEl).toHaveTextContent("No error"); + }); + + it("should set and display the error message", () => { + render( + + + + ); + + const triggerButton = screen.getByTestId("triggerError"); + fireEvent.click(triggerButton); + + const messageEl = screen.getByTestId("errorMessage"); + expect(messageEl).toHaveTextContent("Test error"); + }); + + it("should reset the error message", () => { + render( + + + + ); + + // Trigger the error first + const triggerButton = screen.getByTestId("triggerError"); + fireEvent.click(triggerButton); + + // Reset the error + const resetButton = screen.getByTestId("resetError"); + fireEvent.click(resetButton); + + const messageEl = screen.getByTestId("errorMessage"); + expect(messageEl).toHaveTextContent("No error"); + }); +}); diff --git a/frontend/src/contexts/ErrorContext.tsx b/frontend/src/contexts/ErrorContext.tsx new file mode 100644 index 00000000..afec28b9 --- /dev/null +++ b/frontend/src/contexts/ErrorContext.tsx @@ -0,0 +1,68 @@ +import { createContext, useContext, useState, ReactNode } from "react"; + + +type Error = typeof ERRORS[keyof typeof ERRORS] | null; + +interface ErrorContextProps { + error: Error | null; + setError: (error: Error | null) => void; + resetError: () => void; +} + +const ErrorContext = createContext({ + error: null, + setError: () => { + /* no-op */ + }, + resetError: () => { + /* no-op */ + }, +}); + +interface ErrorProviderProps { + children: ReactNode; +} + +/** ErrorProvider component that wraps your entire application */ +export function ErrorProvider({ children }: ErrorProviderProps) { + const [error, setErrorState] = useState(null); + + const setError = (newError: Error | null) => { + setErrorState(newError); + }; + + const resetError = () => { + setErrorState(null); + }; + return ( + + {children} + + ); +} + +export function useError() { + return useContext(ErrorContext); +}; + +export const ERRORS = { + NO_ERROR: null, + GENERIC_ERROR: { + title: "An error occurred", + message: "Please try again later", + }, + MISMATCH_ERROR: { + title: 'Mismatch Error', + message: 'The uploaded file has a different number of pages than template. Please upload a file with correct pages pages to proceed or choose different template.' + }, + CSV_ERROR: { + title: 'Error downloading file', + message: 'There was an issue with downloading your file. Please try again.' + } +} \ No newline at end of file diff --git a/frontend/src/error/ErrorBanner.scss b/frontend/src/error/ErrorBanner.scss new file mode 100644 index 00000000..adb76397 --- /dev/null +++ b/frontend/src/error/ErrorBanner.scss @@ -0,0 +1,47 @@ +.error-container { + display: flex; + flex-direction: column; + height: 150px; + width: 100%; + border-left: 5px solid #d63e04; + background-color: #f4e3db; + } + + .error-header { + display: flex; + flex-direction: row; + align-items: center; + } + + .error-icon { + margin-left: 2rem; + height: 32px; + width: 32px; + } + + .error-title { + margin-left: 1rem; + color: #000; + } + + .error-message { + margin-left: 5rem; + margin-top: 0; + padding-bottom: 0; + padding-right: 2rem; + } + + .error-highlight { + color: #005ea2; + font-size: 16px; + font-weight: bold; + } + + .error-text { + font-size: 14px; + text-wrap: nowrap; + margin-inline: 8px; + color: #b51d09; + font-weight: 600; + } + \ No newline at end of file diff --git a/frontend/src/error/ErrorBanner.tsx b/frontend/src/error/ErrorBanner.tsx new file mode 100644 index 00000000..5d62774c --- /dev/null +++ b/frontend/src/error/ErrorBanner.tsx @@ -0,0 +1,22 @@ +import { Icon } from "@trussworks/react-uswds"; + +interface ErrorBannerProps { + title: string + message: string +} + +const ErrorBanner = ({ title, message }: ErrorBannerProps) => { + return ( +
+
+ +

{title}

+
+

+ {message} +

+
+ ); +} + +export default ErrorBanner; \ No newline at end of file diff --git a/frontend/src/error/ErrorBoundary.test.tsx b/frontend/src/error/ErrorBoundary.test.tsx new file mode 100644 index 00000000..903533bc --- /dev/null +++ b/frontend/src/error/ErrorBoundary.test.tsx @@ -0,0 +1,59 @@ +import { useEffect } from "react"; +import { render, screen } from "@testing-library/react"; +import "@testing-library/jest-dom"; +import { ErrorProvider, useError } from "../contexts/ErrorContext"; +import GlobalErrorBoundary from "./ErrorBoundary"; + +/** + * A component that throws an error on mount to simulate a runtime error + */ +function ErrorThrowingComponent() { + useEffect(() => { + throw new Error("Component crashed!"); + }, []); + return
Should never see this
; +} + +function TestConsumer() { + const { error } = useError(); + return ( +
+ {error ? error.message : "No error in context"} +
+ ); +} + +describe("GlobalErrorBoundary", () => { + it("should catch errors and set them in context", () => { + render( + + + + + + + ); + + // The boundary will catch the error thrown by ErrorThrowingComponent. + // That means the child won't render, but the error should be stored in context. + expect(screen.getByTestId("error-context")).toHaveTextContent( + "Component crashed!" + ); + }); + + it("should not set an error in context if no error occurs", () => { + render( + + +
Safe Child
+ + + + ); + + // Nothing was thrown, so we expect the context to have no error + expect(screen.getByTestId("error-context")).toHaveTextContent( + "No error in context" + ); + }); +}); diff --git a/frontend/src/error/ErrorBoundary.tsx b/frontend/src/error/ErrorBoundary.tsx new file mode 100644 index 00000000..eba4dd19 --- /dev/null +++ b/frontend/src/error/ErrorBoundary.tsx @@ -0,0 +1,68 @@ +// GlobalErrorBoundary.tsx +import React from "react"; +import { useError } from "../contexts/ErrorContext"; + +interface GlobalErrorBoundaryProps { + children: React.ReactNode; +} + +interface GlobalErrorBoundaryState { + hasError: boolean; +} + +class ErrorBoundaryWrapper extends React.Component< + GlobalErrorBoundaryProps & { onError: (error: Error) => void }, + GlobalErrorBoundaryState +> { + constructor(props: GlobalErrorBoundaryProps & { onError: (error: Error) => void }) { + super(props); + this.state = { + hasError: false, + }; + } + + /** + * getDerivedStateFromError is called during the render phase if + * a child component throws an error. This sets hasError to true + * so we don't re-render the same child that threw. + */ + static getDerivedStateFromError() { + return { hasError: true }; + } + + /** + * componentDidCatch is called after the commit phase when + * there's an error. We log or handle the error (e.g., send to Sentry), + * and call the onError callback to update our global context. + */ + componentDidCatch(error: Error, errorInfo: React.ErrorInfo) { + this.props.onError(error); + console.error("GlobalErrorBoundary caught an error:", error, errorInfo); + } + + render() { + if (this.state.hasError) { + // Render a fallback UI or nothing at all + return null; + } + + // If no error, just render children + return this.props.children; + } +} + +/** + * Functional wrapper that can use the `useError` hook and pass it down + * to the class-based boundary + */ +const GlobalErrorBoundary: React.FC = ({ children }) => { + const { setError } = useError(); + + return ( + + {children} + + ); +}; + +export default GlobalErrorBoundary; diff --git a/frontend/src/error/ErrorFallback.tsx b/frontend/src/error/ErrorFallback.tsx new file mode 100644 index 00000000..3f85aee1 --- /dev/null +++ b/frontend/src/error/ErrorFallback.tsx @@ -0,0 +1,28 @@ +import React from "react"; +import { useError } from "../contexts/ErrorContext"; + +const FallbackUI: React.FC = () => { + const { error, resetError } = useError(); + + if (!error) return null; + + return ( +
+

Something went wrong:

+

{error.message}

+ +
+ ); +}; + +export default FallbackUI; diff --git a/frontend/src/main.tsx b/frontend/src/main.tsx index 9a57493c..17d1038b 100644 --- a/frontend/src/main.tsx +++ b/frontend/src/main.tsx @@ -18,8 +18,10 @@ import ExtractProcess from "./pages/ExtractProcess.tsx"; import { SaveTemplate } from "./pages/SaveTemplate.tsx"; import ReviewTemplate from "./pages/ReviewTemplate.tsx"; import SubmissionTemplate from "./pages/SubmissionTemplate.tsx"; -import NotFound from "./pages/404Page.tsx"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import ErrorPage from "./pages/ErrorPage.tsx"; +import { ErrorProvider } from "./contexts/ErrorContext.tsx"; + const router = createBrowserRouter([ { @@ -56,7 +58,7 @@ const router = createBrowserRouter([ }, { path: "*", - element: , + element: , }, ]); @@ -64,12 +66,14 @@ const queryClient = new QueryClient(); createRoot(document.getElementById("root")!).render( - - - - - - - + + + + + + + + + , ); diff --git a/frontend/src/pages/404Page.test.tsx b/frontend/src/pages/ErrorPage.test.tsx similarity index 80% rename from frontend/src/pages/404Page.test.tsx rename to frontend/src/pages/ErrorPage.test.tsx index bb95a83f..e90f8f0d 100644 --- a/frontend/src/pages/404Page.test.tsx +++ b/frontend/src/pages/ErrorPage.test.tsx @@ -2,7 +2,7 @@ import { render, screen, fireEvent } from "@testing-library/react"; import { BrowserRouter } from "react-router-dom"; import { describe, it, expect, vi } from "vitest"; -import NotFound from "./404Page"; +import ErrorPage from "./ErrorPage"; const mockNavigate = vi.fn(); // Create a mock function for navigation // Mock the useNavigate hook from react-router-dom @@ -17,7 +17,7 @@ describe("NotFound Component", () => { it("should render the NotFound component", () => { render( - + , ); @@ -43,8 +43,8 @@ describe("NotFound Component", () => { it("should navigate back when the back button is clicked", () => { render( - - , + + , ); fireEvent.click(screen.getByText(/back/i)); diff --git a/frontend/src/pages/404Page.tsx b/frontend/src/pages/ErrorPage.tsx similarity index 74% rename from frontend/src/pages/404Page.tsx rename to frontend/src/pages/ErrorPage.tsx index 83d6cb22..05ab6607 100644 --- a/frontend/src/pages/404Page.tsx +++ b/frontend/src/pages/ErrorPage.tsx @@ -6,15 +6,20 @@ import { useNavigate } from "react-router-dom"; import "./404Page.scss"; -const NotFound: React.FC = () => { +interface ErrorPageProps { + title: string; + body: string; +} + +const ErrorPage: React.FC = ({ title, body }) => { const navigate = useNavigate(); return (
-

Sorry, this page can’t be found

+

{title}

404 -

The page you are looking for doesn’t exist or has been moved.

+

{body}

@@ -23,4 +28,4 @@ const NotFound: React.FC = () => { ); }; -export default NotFound; +export default ErrorPage; diff --git a/frontend/src/pages/ReviewTemplate.tsx b/frontend/src/pages/ReviewTemplate.tsx index 2326dad5..844d334f 100644 --- a/frontend/src/pages/ReviewTemplate.tsx +++ b/frontend/src/pages/ReviewTemplate.tsx @@ -14,6 +14,7 @@ import { fake_upload_images, fakeTemplates, } from "../utils/constants.ts"; +import { ERRORS, useError } from "../contexts/ErrorContext.tsx"; interface ResultsTable { index: number; @@ -42,6 +43,7 @@ interface Row { const ReviewTemplate: React.FC = () => { const navigate = useNavigate(); const { clearTemplates } = useFiles(); + const { setError } = useError(); const [submissionIndex, setSubmissionIndex] = useState(0); const [isReviewing, setIsReviewing] = useState(false); const [index, setIndex] = useState(0); @@ -55,6 +57,7 @@ const ReviewTemplate: React.FC = () => { const onDownload = () => { try { + setError(null); if (submissionArray && submissionArray.length > 0) { // Prepare CSV data with column names const csvData = [ @@ -81,8 +84,9 @@ const ReviewTemplate: React.FC = () => { document.body.removeChild(a); // Clean up URL.revokeObjectURL(url); // Revoke URL } - } catch (error) { - console.error(error); + // eslint-disable-next-line @typescript-eslint/no-unused-vars + } catch (e) { + setError(ERRORS.CSV_ERROR); } };