From 4810991ebf2aaa2381385ae085d399736305ffdd Mon Sep 17 00:00:00 2001 From: Ryan Lewis <93001277+rylew1@users.noreply.github.com> Date: Tue, 16 Apr 2024 07:15:52 -0700 Subject: [PATCH] [Issue #1502]: Setup search error management (#1681) ## Summary Fixes #1502 ## Changes proposed - Add Error boundary page that shows alert - add Error classes (400/500 and generic) are defined in `src/errors.ts` - Update tests --- frontend/src/app/api/BaseApi.ts | 143 +++++++++++---- frontend/src/app/api/SearchOpportunityAPI.ts | 1 + frontend/src/app/search/SearchForm.tsx | 15 +- frontend/src/app/search/error.tsx | 105 +++++++++++ .../components/search/SearchCallToAction.tsx | 2 - .../components/search/SearchResultsList.tsx | 13 +- .../search/error/SearchErrorAlert.tsx | 12 ++ frontend/src/errors.ts | 171 ++++++++++++++++++ frontend/src/hooks/useSearchFormState.ts | 12 +- frontend/tests/api/BaseApi.test.ts | 76 +++++++- .../search/error/SearchErrorAlert.test.tsx | 22 +++ frontend/tests/errors.test.ts | 44 +++++ 12 files changed, 561 insertions(+), 55 deletions(-) create mode 100644 frontend/src/app/search/error.tsx create mode 100644 frontend/src/components/search/error/SearchErrorAlert.tsx create mode 100644 frontend/src/errors.ts create mode 100644 frontend/tests/components/search/error/SearchErrorAlert.test.tsx create mode 100644 frontend/tests/errors.test.ts diff --git a/frontend/src/app/api/BaseApi.ts b/frontend/src/app/api/BaseApi.ts index 42a54e666..284fb7bdb 100644 --- a/frontend/src/app/api/BaseApi.ts +++ b/frontend/src/app/api/BaseApi.ts @@ -4,24 +4,35 @@ // https://nextjs.org/docs/app/building-your-application/rendering/composition-patterns#keeping-server-only-code-out-of-the-client-environment import "server-only"; +import { + ApiRequestError, + BadRequestError, + ForbiddenError, + InternalServerError, + NetworkError, + NotFoundError, + RequestTimeoutError, + ServiceUnavailableError, + UnauthorizedError, + ValidationError, +} from "src/errors"; +import { compact, isEmpty } from "lodash"; + +// TODO (#1682): replace search specific references (since this is a generic API file that any +// future page or different namespace could use) import { SearchAPIResponse } from "../../types/search/searchResponseTypes"; -import { compact } from "lodash"; +import { SearchFetcherProps } from "src/services/search/searchfetcher/SearchFetcher"; export type ApiMethod = "DELETE" | "GET" | "PATCH" | "POST" | "PUT"; export interface JSONRequestBody { [key: string]: unknown; } -// TODO: keep for reference on generic response type - -// export interface ApiResponseBody { -// message: string; -// data: TResponseData; -// status_code: number; - -// errors?: unknown[]; // TODO: define error and warning Issue type -// warnings?: unknown[]; -// } +interface APIResponseError { + field: string; + message: string; + type: string; +} export interface HeadersDict { [header: string]: string; @@ -58,6 +69,8 @@ export default abstract class BaseApi { basePath: string, namespace: string, subPath: string, + + searchInputs: SearchFetcherProps, body?: JSONRequestBody, options: { additionalHeaders?: HeadersDict; @@ -78,11 +91,15 @@ export default abstract class BaseApi { }; headers["Content-Type"] = "application/json"; - const response = await this.sendRequest(url, { - body: method === "GET" || !body ? null : createRequestBody(body), - headers, - method, - }); + const response = await this.sendRequest( + url, + { + body: method === "GET" || !body ? null : createRequestBody(body), + headers, + method, + }, + searchInputs, + ); return response; } @@ -90,33 +107,26 @@ export default abstract class BaseApi { /** * Send a request and handle the response */ - private async sendRequest(url: string, fetchOptions: RequestInit) { + private async sendRequest( + url: string, + fetchOptions: RequestInit, + searchInputs: SearchFetcherProps, + ) { let response: Response; let responseBody: SearchAPIResponse; try { response = await fetch(url, fetchOptions); responseBody = (await response.json()) as SearchAPIResponse; } catch (error) { - console.log("Network Error encountered => ", error); - throw new Error("Network request failed"); - // TODO: Error management - // throw fetchErrorToNetworkError(error); + // API most likely down, but also possibly an error setting up or sending a request + // or parsing the response. + throw fetchErrorToNetworkError(error, searchInputs); } - const { data, message, pagination_info, status_code, errors, warnings } = + const { data, message, pagination_info, status_code, warnings } = responseBody; if (!response.ok) { - console.log( - "Not OK Response => ", - response, - errors, - this.namespace, - data, - ); - - throw new Error("Not OK response received"); - // TODO: Error management - // handleNotOkResponse(response, errors, this.namespace, data); + handleNotOkResponse(responseBody, message, status_code, searchInputs); } return { @@ -174,3 +184,70 @@ function createRequestBody(payload?: JSONRequestBody): XMLHttpRequestBodyInit { return JSON.stringify(payload); } + +/** + * Handle request errors + */ +export function fetchErrorToNetworkError( + error: unknown, + searchInputs: SearchFetcherProps, +) { + // Request failed to send or something failed while parsing the response + // Log the JS error to support troubleshooting + console.error(error); + return new NetworkError(error, searchInputs); +} + +function handleNotOkResponse( + response: SearchAPIResponse, + message: string, + status_code: number, + searchInputs: SearchFetcherProps, +) { + const { errors } = response; + if (isEmpty(errors)) { + // No detailed errors provided, throw generic error based on status code + throwError(message, status_code, searchInputs); + } else { + if (errors) { + const firstError = errors[0] as APIResponseError; + throwError(message, status_code, searchInputs, firstError); + } + } +} + +const throwError = ( + message: string, + status_code: number, + searchInputs: SearchFetcherProps, + firstError?: APIResponseError, +) => { + // Include just firstError for now, we can expand this + // If we need ValidationErrors to be more expanded + const error = firstError ? { message, firstError } : { message }; + switch (status_code) { + case 400: + throw new BadRequestError(error, searchInputs); + case 401: + throw new UnauthorizedError(error, searchInputs); + case 403: + throw new ForbiddenError(error, searchInputs); + case 404: + throw new NotFoundError(error, searchInputs); + case 422: + throw new ValidationError(error, searchInputs); + case 408: + throw new RequestTimeoutError(error, searchInputs); + case 500: + throw new InternalServerError(error, searchInputs); + case 503: + throw new ServiceUnavailableError(error, searchInputs); + default: + throw new ApiRequestError( + error, + searchInputs, + "APIRequestError", + status_code, + ); + } +}; diff --git a/frontend/src/app/api/SearchOpportunityAPI.ts b/frontend/src/app/api/SearchOpportunityAPI.ts index 23632f5d7..4e4f321b3 100644 --- a/frontend/src/app/api/SearchOpportunityAPI.ts +++ b/frontend/src/app/api/SearchOpportunityAPI.ts @@ -49,6 +49,7 @@ export default class SearchOpportunityAPI extends BaseApi { this.basePath, this.namespace, subPath, + searchInputs, requestBody, ); diff --git a/frontend/src/app/search/SearchForm.tsx b/frontend/src/app/search/SearchForm.tsx index c3c946818..981c98941 100644 --- a/frontend/src/app/search/SearchForm.tsx +++ b/frontend/src/app/search/SearchForm.tsx @@ -78,13 +78,15 @@ export function SearchForm({
- {searchResults.data.length >= 1 ? ( + {searchResults?.data.length >= 1 ? ( - {searchResults.data.length >= 1 ? ( + {searchResults?.data?.length >= 1 ? ( diff --git a/frontend/src/app/search/error.tsx b/frontend/src/app/search/error.tsx new file mode 100644 index 000000000..01ea3cf2a --- /dev/null +++ b/frontend/src/app/search/error.tsx @@ -0,0 +1,105 @@ +"use client"; // Error components must be Client Components + +import { + PaginationInfo, + SearchAPIResponse, +} from "src/types/search/searchResponseTypes"; + +import PageSEO from "src/components/PageSEO"; +import SearchCallToAction from "src/components/search/SearchCallToAction"; +import { SearchFetcherProps } from "src/services/search/searchfetcher/SearchFetcher"; +import { SearchForm } from "src/app/search/SearchForm"; +import { useEffect } from "react"; + +interface ErrorProps { + // Next's error boundary also includes a reset function as a prop for retries, + // but it was not needed as users can retry with new inputs in the normal page flow. + error: Error & { digest?: string }; +} + +export interface ParsedError { + message: string; + searchInputs: SearchFetcherProps; + status: number; + type: string; +} + +export default function Error({ error }: ErrorProps) { + // The error message is passed as an object that's been stringified. + // Parse it here. + const parsedErrorData = JSON.parse(error.message) as ParsedError; + + const pagination_info = getErrorPaginationInfo(); + const initialSearchResults: SearchAPIResponse = getErrorInitialSearchResults( + parsedErrorData, + pagination_info, + ); + + // The error message search inputs had to be converted to arrays in order to be stringified, + // convert those back to sets as we do in non-error flow. + const convertedSearchParams = convertSearchInputArraysToSets( + parsedErrorData.searchInputs, + ); + + useEffect(() => { + console.error(error); + }, [error]); + + return ( + <> + + + + + ); +} + +/* + * Generate empty response data to render the full page on an error + * which otherwise may not have any data. + */ +function getErrorInitialSearchResults( + parsedError: ParsedError, + pagination_info: PaginationInfo, +) { + return { + errors: [{ ...parsedError }], + data: [], + pagination_info, + status_code: parsedError.status, + message: parsedError.message, + }; +} + +// There will be no pagination shown on an error +// so the values here just need to be valid for the page to +// load without error +function getErrorPaginationInfo() { + return { + order_by: "opportunity_id", + page_offset: 0, + page_size: 25, + sort_direction: "ascending", + total_pages: 1, + total_records: 0, + }; +} + +function convertSearchInputArraysToSets( + searchInputs: SearchFetcherProps, +): SearchFetcherProps { + return { + ...searchInputs, + status: new Set(searchInputs.status || []), + fundingInstrument: new Set(searchInputs.fundingInstrument || []), + eligibility: new Set(searchInputs.eligibility || []), + agency: new Set(searchInputs.agency || []), + category: new Set(searchInputs.category || []), + }; +} diff --git a/frontend/src/components/search/SearchCallToAction.tsx b/frontend/src/components/search/SearchCallToAction.tsx index 8eff691aa..812433944 100644 --- a/frontend/src/components/search/SearchCallToAction.tsx +++ b/frontend/src/components/search/SearchCallToAction.tsx @@ -1,5 +1,3 @@ -import "server-only"; - import Breadcrumbs from "../../components/Breadcrumbs"; import { GridContainer } from "@trussworks/react-uswds"; import React from "react"; diff --git a/frontend/src/components/search/SearchResultsList.tsx b/frontend/src/components/search/SearchResultsList.tsx index cb3e91893..ff64b19e8 100644 --- a/frontend/src/components/search/SearchResultsList.tsx +++ b/frontend/src/components/search/SearchResultsList.tsx @@ -1,18 +1,21 @@ "use client"; import Loading from "../../app/search/loading"; +import SearchErrorAlert from "src/components/search/error/SearchErrorAlert"; import { SearchResponseData } from "../../types/search/searchResponseTypes"; -import { useFormStatus } from "react-dom"; import SearchResultsListItem from "./SearchResultsListItem"; +import { useFormStatus } from "react-dom"; interface SearchResultsListProps { searchResults: SearchResponseData; maxPaginationError: boolean; + errors?: unknown[] | null | undefined; // If passed in, there's been an issue with the fetch call } const SearchResultsList: React.FC = ({ searchResults, maxPaginationError, + errors, }) => { const { pending } = useFormStatus(); @@ -20,7 +23,11 @@ const SearchResultsList: React.FC = ({ return ; } - if (searchResults.length === 0) { + if (errors) { + return ; + } + + if (searchResults?.length === 0) { return (

Your search did not return any results.

@@ -45,7 +52,7 @@ const SearchResultsList: React.FC = ({ } )} - {searchResults.map((opportunity) => ( + {searchResults?.map((opportunity) => (
    • diff --git a/frontend/src/components/search/error/SearchErrorAlert.tsx b/frontend/src/components/search/error/SearchErrorAlert.tsx new file mode 100644 index 000000000..e03f3a053 --- /dev/null +++ b/frontend/src/components/search/error/SearchErrorAlert.tsx @@ -0,0 +1,12 @@ +const SearchErrorAlert = () => ( +
    +
    +

    We're sorry.

    +

    + There seems to have been an error. Please try your search again. +

    +
    +
    +); + +export default SearchErrorAlert; diff --git a/frontend/src/errors.ts b/frontend/src/errors.ts new file mode 100644 index 000000000..c1b1b8805 --- /dev/null +++ b/frontend/src/errors.ts @@ -0,0 +1,171 @@ +/** + * @file Custom Error classes. Useful as a way to see all potential errors that our system may throw/catch + */ + +import { SearchFetcherProps } from "src/services/search/searchfetcher/SearchFetcher"; + +/** + * A fetch request failed due to a network error. The error wasn't the fault of the user, + * and an issue was encountered while setting up or sending a request, or parsing the response. + * Examples of a NetworkError could be the user's device lost internet connection, a CORS issue, + * or a malformed request. + */ + +export class NetworkError extends Error { + constructor(error: unknown, searchInputs: SearchFetcherProps) { + const serializedSearchInputs = convertSearchInputSetsToArrays(searchInputs); + + const serializedData = JSON.stringify({ + type: "NetworkError", + searchInputs: serializedSearchInputs, + message: error instanceof Error ? error.message : "Unknown Error", + status: 500, + }); + super(serializedData); + } +} + +// Used as a base class for all !response.ok errors +export class BaseFrontendError extends Error { + constructor( + error: unknown, + searchInputs: SearchFetcherProps, + type: string, + status?: number, + ) { + const serializedSearchInputs = convertSearchInputSetsToArrays(searchInputs); + + const serializedData = JSON.stringify({ + type: type || "BaseFrontendError", + searchInputs: serializedSearchInputs, + message: error instanceof Error ? error.message : "Unknown Error", + status, + }); + + super(serializedData); + + if (typeof Error.captureStackTrace === "function") { + Error.captureStackTrace(this, this.constructor); + } + } +} + +/***************************************************/ +/* 400 Errors +/***************************************************/ + +/** + * An API response returned a status code greater than 400 + */ +export class ApiRequestError extends BaseFrontendError { + constructor( + error: unknown, + searchInputs: SearchFetcherProps, + type: string, + status: number, + ) { + super(error, searchInputs, type || "APIRequestError", status || 400); + } +} + +/** + * An API response returned a 400 status code and its JSON body didn't include any `errors` + */ +export class BadRequestError extends ApiRequestError { + constructor(error: unknown, searchInputs: SearchFetcherProps) { + super(error, searchInputs, "BadRequestError", 400); + } +} + +/** + * An API response returned a 401 status code + */ +export class UnauthorizedError extends ApiRequestError { + constructor(error: unknown, searchInputs: SearchFetcherProps) { + super(error, searchInputs, "UnauthorizedError", 401); + } +} + +/** + * An API response returned a 403 status code, indicating an issue with the authorization of the request. + * Examples of a ForbiddenError could be the user's browser prevented the session cookie from + * being created, or the user hasn't consented to the data sharing agreement. + */ +export class ForbiddenError extends ApiRequestError { + constructor(error: unknown, searchInputs: SearchFetcherProps) { + super(error, searchInputs, "ForbiddenError", 403); + } +} + +/** + * A fetch request failed due to a 404 error + */ +export class NotFoundError extends ApiRequestError { + constructor(error: unknown, searchInputs: SearchFetcherProps) { + super(error, searchInputs, "NotFoundError", 404); + } +} + +/** + * An API response returned a 408 status code + */ +export class RequestTimeoutError extends ApiRequestError { + constructor(error: unknown, searchInputs: SearchFetcherProps) { + super(error, searchInputs, "RequestTimeoutError", 408); + } +} + +/** + * An API response returned a 422 status code + */ +export class ValidationError extends ApiRequestError { + constructor(error: unknown, searchInputs: SearchFetcherProps) { + super(error, searchInputs, "ValidationError", 422); + } +} + +/***************************************************/ +/* 500 Errors +/***************************************************/ + +/** + * An API response returned a 500 status code + */ +export class InternalServerError extends ApiRequestError { + constructor(error: unknown, searchInputs: SearchFetcherProps) { + super(error, searchInputs, "InternalServerError", 500); + } +} + +/** + * An API response returned a 503 status code + */ +export class ServiceUnavailableError extends ApiRequestError { + constructor(error: unknown, searchInputs: SearchFetcherProps) { + super(error, searchInputs, "ServiceUnavailableError", 503); + } +} + +type SearchInputsSimple = { + [key: string]: string[] | string | number | null | undefined; +}; + +function convertSearchInputSetsToArrays( + searchInputs: SearchFetcherProps, +): SearchInputsSimple { + return { + ...searchInputs, + status: searchInputs.status ? Array.from(searchInputs.status) : [], + fundingInstrument: searchInputs.fundingInstrument + ? Array.from(searchInputs.fundingInstrument) + : [], + eligibility: searchInputs.eligibility + ? Array.from(searchInputs.eligibility) + : [], + agency: searchInputs.agency ? Array.from(searchInputs.agency) : [], + category: searchInputs.category ? Array.from(searchInputs.category) : [], + query: searchInputs.query, + sortby: searchInputs.sortby, + page: searchInputs.page, + }; +} diff --git a/frontend/src/hooks/useSearchFormState.ts b/frontend/src/hooks/useSearchFormState.ts index ef7812c4a..c3bf035f1 100644 --- a/frontend/src/hooks/useSearchFormState.ts +++ b/frontend/src/hooks/useSearchFormState.ts @@ -3,7 +3,6 @@ import { useRef, useState } from "react"; import { SearchAPIResponse } from "../types/search/searchResponseTypes"; -import { SearchFetcherActionType } from "../types/search/searchRequestTypes"; import { SearchFetcherProps } from "../services/search/searchfetcher/SearchFetcher"; import { updateResults } from "../app/search/actions"; import { useFormState } from "react-dom"; @@ -75,13 +74,9 @@ export function useSearchFormState( // TODO (Issue #1517): move this to server-side calculation? const maxPaginationError = - searchResults.pagination_info.total_pages > 0 && - searchResults.pagination_info.page_offset > - searchResults.pagination_info.total_pages; - - const resetPagination = - searchResults.actionType === SearchFetcherActionType.Update && - searchResults.fieldChanged !== "pagination"; + searchResults?.pagination_info?.total_pages > 0 && + searchResults?.pagination_info?.page_offset > + searchResults?.pagination_info?.total_pages; return { searchResults, @@ -98,7 +93,6 @@ export function useSearchFormState( agencyQueryParams, categoryQueryParams, fieldChangedRef, - resetPagination, page, setPage, handlePageChange, diff --git a/frontend/tests/api/BaseApi.test.ts b/frontend/tests/api/BaseApi.test.ts index 682ec1f9d..cfd8b277f 100644 --- a/frontend/tests/api/BaseApi.test.ts +++ b/frontend/tests/api/BaseApi.test.ts @@ -1,6 +1,7 @@ import "server-only"; import BaseApi, { ApiMethod, JSONRequestBody } from "../../src/app/api/BaseApi"; +import { NetworkError, UnauthorizedError } from "src/errors"; // Define a concrete implementation of BaseApi for testing class TestApi extends BaseApi { @@ -13,6 +14,17 @@ class TestApi extends BaseApi { } } +const searchInputs = { + status: new Set(["active"]), + fundingInstrument: new Set(["grant"]), + eligibility: new Set(["public"]), + agency: new Set(["NASA"]), + category: new Set(["science"]), + query: "space exploration", + sortby: "date", + page: 1, +}; + describe("BaseApi", () => { let testApi: TestApi; @@ -32,7 +44,7 @@ describe("BaseApi", () => { const namespace = "mynamespace"; const subPath = "myendpointendpoint"; - await testApi.request(method, basePath, namespace, subPath); + await testApi.request(method, basePath, namespace, subPath, searchInputs); const expectedHeaders = { "Content-Type": "application/json", @@ -61,7 +73,14 @@ describe("BaseApi", () => { }, }; - await testApi.request(method, basePath, namespace, subPath, body); + await testApi.request( + method, + basePath, + namespace, + subPath, + searchInputs, + body, + ); const expectedHeaders = { "Content-Type": "application/json", @@ -76,4 +95,57 @@ describe("BaseApi", () => { }), ); }); + + describe("Not OK response", () => { + let testApi: TestApi; + + beforeEach(() => { + testApi = new TestApi(); + global.fetch = jest.fn().mockResolvedValue({ + json: () => + Promise.resolve({ + data: {}, + errors: [], + message: + "The server could not verify that you are authorized to access the URL requested", + status_code: 401, + }), + ok: false, + status: 401, + }); + }); + + it("throws an UnauthorizedError for a 401 response", async () => { + const method = "GET"; + const basePath = "http://mydomain:8080"; + const namespace = "mynamespace"; + const subPath = "endpoint"; + + await expect( + testApi.request(method, basePath, namespace, subPath, searchInputs), + ).rejects.toThrow(UnauthorizedError); + }); + }); + + describe("API is down", () => { + let testApi: TestApi; + + beforeEach(() => { + testApi = new TestApi(); + global.fetch = jest.fn().mockImplementation(() => { + throw new Error("Network failure"); + }); + }); + + it("throws a NetworkError when fetch fails", async () => { + const method = "GET"; + const basePath = "http://mydomain:8080"; + const namespace = "mynamespace"; + const subPath = "endpoint"; + + await expect( + testApi.request(method, basePath, namespace, subPath, searchInputs), + ).rejects.toThrow(NetworkError); + }); + }); }); diff --git a/frontend/tests/components/search/error/SearchErrorAlert.test.tsx b/frontend/tests/components/search/error/SearchErrorAlert.test.tsx new file mode 100644 index 000000000..ba5082c81 --- /dev/null +++ b/frontend/tests/components/search/error/SearchErrorAlert.test.tsx @@ -0,0 +1,22 @@ +import { render, screen } from "@testing-library/react"; + +import SearchErrorAlert from "src/components/search/error/SearchErrorAlert"; +import { axe } from "jest-axe"; + +describe("SearchErrorAlert", () => { + it("should display the error message", () => { + render(); + expect(screen.getByText("We're sorry.")).toBeInTheDocument(); + expect( + screen.getByText( + "There seems to have been an error. Please try your search again.", + ), + ).toBeInTheDocument(); + }); + + it("should not have any accessibility violations", async () => { + const { container } = render(); + const results = await axe(container); + expect(results).toHaveNoViolations(); + }); +}); diff --git a/frontend/tests/errors.test.ts b/frontend/tests/errors.test.ts new file mode 100644 index 000000000..31db3364b --- /dev/null +++ b/frontend/tests/errors.test.ts @@ -0,0 +1,44 @@ +import { BadRequestError } from "src/errors"; +import { ParsedError } from "src/app/search/error"; +import { SearchFetcherProps } from "src/services/search/searchfetcher/SearchFetcher"; + +describe("BadRequestError", () => { + const dummySearchInputs: SearchFetcherProps = { + status: new Set(["active"]), + fundingInstrument: new Set(["grant"]), + eligibility: new Set(["public"]), + agency: new Set(["NASA"]), + category: new Set(["science"]), + query: "space exploration", + sortby: "date", + page: 1, + }; + + it("serializes search inputs and error message correctly", () => { + const error = new BadRequestError( + new Error("Test Error"), + dummySearchInputs, + ); + const errorData = JSON.parse(error.message) as ParsedError; + + expect(errorData.type).toEqual("BadRequestError"); + expect(errorData.status).toEqual(400); + expect(errorData.message).toEqual("Test Error"); + expect(errorData.searchInputs.status).toContain("active"); + expect(errorData.searchInputs.fundingInstrument).toContain("grant"); + }); + + it("handles non-Error inputs correctly", () => { + const error = new BadRequestError("Some string error", dummySearchInputs); + const errorData = JSON.parse(error.message) as ParsedError; + + expect(errorData.message).toEqual("Unknown Error"); + }); + + it("sets a default message when error is not an instance of Error", () => { + const error = new BadRequestError(null, dummySearchInputs); + const errorData = JSON.parse(error.message) as ParsedError; + + expect(errorData.message).toEqual("Unknown Error"); + }); +});