Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Issue #3152] properly error on client for max search term length #3554

Merged
merged 12 commits into from
Jan 22, 2025
17 changes: 14 additions & 3 deletions frontend/src/app/[locale]/search/error.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
"use client";

import QueryProvider from "src/app/[locale]/search/QueryProvider";
import { FrontendErrorDetails } from "src/types/apiResponseTypes";
import { ServerSideSearchParams } from "src/types/searchRequestURLTypes";
import { Breakpoints, ErrorProps } from "src/types/uiTypes";
import { convertSearchParamsToProperTypes } from "src/utils/search/convertSearchParamsToProperTypes";

import { useTranslations } from "next-intl";
import { useEffect } from "react";
import { Alert } from "@trussworks/react-uswds";

import ContentDisplayToggle from "src/components/ContentDisplayToggle";
import SearchBar from "src/components/search/SearchBar";
Expand All @@ -18,6 +20,7 @@ export interface ParsedError {
searchInputs: ServerSideSearchParams;
status: number;
type: string;
details?: FrontendErrorDetails;
}

function isValidJSON(str: string) {
Expand Down Expand Up @@ -73,6 +76,16 @@ export default function SearchError({ error }: ErrorProps) {
console.error(error);
}, [error]);

// note that the validation error will contain untranslated strings
const ErrorAlert =
parsedErrorData.details && parsedErrorData.type === "ValidationError" ? (
<Alert type="error" heading={t("validationError")} headingLevel="h4">
{`Error in ${parsedErrorData.details.field || "a search field"}: ${parsedErrorData.details.message || "adjust your search and try again"}`}
Copy link
Collaborator

@acouch acouch Jan 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we do a follow-up to translate this, ie the "adjust your search and try again"?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can make a ticket, but I'm not exactly sure what the scope would need to be. To get this to work as a fully translated UI we'd need to handle translating stuff that's coming from the API, including error messages and our field names. That seems to open up a lot of questions, and since it doesn't seem like we're prioritizing translation right now I don't know when the right time for that conversation is

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant the a search field part.

</Alert>
) : (
<ServerErrorAlert callToAction={t("generic_error_cta")} />
);

return (
<QueryProvider>
<div className="grid-container">
Expand All @@ -95,9 +108,7 @@ export default function SearchError({ error }: ErrorProps) {
/>
</ContentDisplayToggle>
</div>
<div className="tablet:grid-col-8">
<ServerErrorAlert callToAction={t("generic_error_cta")} />
</div>
<div className="tablet:grid-col-8">{ErrorAlert}</div>
</div>
</div>
</QueryProvider>
Expand Down
24 changes: 20 additions & 4 deletions frontend/src/components/search/SearchBar.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
"use client";

import clsx from "clsx";
import { QueryContext } from "src/app/[locale]/search/QueryProvider";
import { useSearchParamUpdater } from "src/hooks/useSearchParamUpdater";

import { useTranslations } from "next-intl";
import { useContext, useEffect, useRef } from "react";
import { Icon } from "@trussworks/react-uswds";
import { useContext, useEffect, useRef, useState } from "react";
import { ErrorMessage, Icon } from "@trussworks/react-uswds";

interface SearchBarProps {
query: string | null | undefined;
Expand All @@ -16,8 +17,16 @@ export default function SearchBar({ query }: SearchBarProps) {
const { queryTerm, updateQueryTerm } = useContext(QueryContext);
const { updateQueryParams, searchParams } = useSearchParamUpdater();
const t = useTranslations("Search");
const [validationError, setValidationError] = useState<string>();

const handleSubmit = () => {
if (queryTerm && queryTerm.length > 99) {
setValidationError(t("tooLongError"));
return;
}
if (validationError) {
setValidationError(undefined);
}
updateQueryParams("", "query", queryTerm, false);
};

Expand All @@ -38,7 +47,11 @@ export default function SearchBar({ query }: SearchBarProps) {
}, [searchParams, updateQueryParams]);

return (
<div className="margin-top-5 margin-bottom-2">
<div
className={clsx("margin-top-5", "margin-bottom-2", {
"usa-form-group--error": !!validationError,
})}
>
<label
htmlFor="query"
className="font-sans-lg display-block margin-bottom-2"
Expand All @@ -50,10 +63,13 @@ export default function SearchBar({ query }: SearchBarProps) {
),
})}
</label>
{validationError && <ErrorMessage>{validationError}</ErrorMessage>}
<div className="usa-search usa-search--big" role="search">
<input
ref={inputRef}
className="usa-input maxw-none"
className={clsx("usa-input", "maxw-none", {
"usa-input--error": !!validationError,
})}
id="query"
type="search"
name="query"
Expand Down
50 changes: 27 additions & 23 deletions frontend/src/errors.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
/**
* @file Custom Error classes. Useful as a way to see all potential errors that our system may throw/catch
* Note that the errors defined here rely on stringifying JSON data into the Error's message parameter
* That data will need to be parsed back out into JSON when reading the error
*/

import { FrontendErrorDetails } from "src/types/apiResponseTypes";
import { QueryParamData } from "src/types/search/searchRequestTypes";

export const parseErrorStatus = (error: ApiRequestError): number => {
Expand Down Expand Up @@ -41,11 +44,11 @@ export class NetworkError extends Error {
// Used as a base class for all !response.ok errors
export class BaseFrontendError extends Error {
constructor(
error: unknown,
message: string,
type = "BaseFrontendError",
status?: number,
searchInputs?: QueryParamData,
details?: FrontendErrorDetails,
) {
const { searchInputs, status, ...additionalDetails } = details || {};
// Sets cannot be properly serialized so convert to arrays first
const serializedSearchInputs = searchInputs
? convertSearchInputSetsToArrays(searchInputs)
Expand All @@ -54,8 +57,9 @@ export class BaseFrontendError extends Error {
const serializedData = JSON.stringify({
type,
searchInputs: serializedSearchInputs,
message: error instanceof Error ? error.message : "Unknown Error",
message: message || "Unknown Error",
status,
details: additionalDetails,
});

super(serializedData);
Expand All @@ -75,30 +79,30 @@ export class BaseFrontendError extends Error {
*/
export class ApiRequestError extends BaseFrontendError {
constructor(
error: unknown,
message: string,
type = "APIRequestError",
status = 400,
searchInputs?: QueryParamData,
details?: FrontendErrorDetails,
) {
super(error, type, status, searchInputs);
super(message, type, { status, ...details });
}
}

/**
* 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?: QueryParamData) {
super(error, "BadRequestError", 400, searchInputs);
constructor(message: string, details?: FrontendErrorDetails) {
super(message, "BadRequestError", 400, details);
}
}

/**
* An API response returned a 401 status code
*/
export class UnauthorizedError extends ApiRequestError {
constructor(error: unknown, searchInputs?: QueryParamData) {
super(error, "UnauthorizedError", 401, searchInputs);
constructor(message: string, details?: FrontendErrorDetails) {
super(message, "UnauthorizedError", 401, details);
}
}

Expand All @@ -108,35 +112,35 @@ export class UnauthorizedError extends ApiRequestError {
* being created, or the user hasn't consented to the data sharing agreement.
*/
export class ForbiddenError extends ApiRequestError {
constructor(error: unknown, searchInputs?: QueryParamData) {
super(error, "ForbiddenError", 403, searchInputs);
constructor(message: string, details?: FrontendErrorDetails) {
super(message, "ForbiddenError", 403, details);
}
}

/**
* A fetch request failed due to a 404 error
*/
export class NotFoundError extends ApiRequestError {
constructor(error: unknown, searchInputs?: QueryParamData) {
super(error, "NotFoundError", 404, searchInputs);
constructor(message: string, details?: FrontendErrorDetails) {
super(message, "NotFoundError", 404, details);
}
}

/**
* An API response returned a 408 status code
*/
export class RequestTimeoutError extends ApiRequestError {
constructor(error: unknown, searchInputs?: QueryParamData) {
super(error, "RequestTimeoutError", 408, searchInputs);
constructor(message: string, details?: FrontendErrorDetails) {
super(message, "RequestTimeoutError", 408, details);
}
}

/**
* An API response returned a 422 status code
*/
export class ValidationError extends ApiRequestError {
constructor(error: unknown, searchInputs?: QueryParamData) {
super(error, "ValidationError", 422, searchInputs);
constructor(message: string, details?: FrontendErrorDetails) {
super(message, "ValidationError", 422, details);
}
}

Expand All @@ -148,17 +152,17 @@ export class ValidationError extends ApiRequestError {
* An API response returned a 500 status code
*/
export class InternalServerError extends ApiRequestError {
constructor(error: unknown, searchInputs?: QueryParamData) {
super(error, "InternalServerError", 500, searchInputs);
constructor(message: string, details?: FrontendErrorDetails) {
super(message, "InternalServerError", 500, details);
}
}

/**
* An API response returned a 503 status code
*/
export class ServiceUnavailableError extends ApiRequestError {
constructor(error: unknown, searchInputs?: QueryParamData) {
super(error, "ServiceUnavailableError", 503, searchInputs);
constructor(message: string, details?: FrontendErrorDetails) {
super(message, "ServiceUnavailableError", 503, details);
}
}

Expand Down
2 changes: 2 additions & 0 deletions frontend/src/i18n/messages/en/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -619,6 +619,8 @@ export const messages = {
hideFilters: "Hide Filters",
},
generic_error_cta: "Please try your search again.",
validationError: "Search Validation Error",
tooLongError: "Search terms must be no longer than 100 characters.",
},
Maintenance: {
heading: "Simpler.Grants.gov Is Currently Undergoing Maintenance",
Expand Down
33 changes: 19 additions & 14 deletions frontend/src/services/fetch/fetcherHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ export function getDefaultHeaders(): HeadersDict {

/**
* Send a request and handle the response
* @param queryParamData: note that this is only used in error handling in order to help restore original page state
*/
export async function sendRequest<ResponseType extends APIResponse>(
url: string,
Expand Down Expand Up @@ -126,6 +127,9 @@ function fetchErrorToNetworkError(
: new NetworkError(error);
}

// note that this will pass along filter inputs in order to maintain the state
// of the page when relaying an error, but anything passed in the body of the request,
// such as keyword search query will not be included
function handleNotOkResponse(
response: APIResponse,
url: string,
Expand All @@ -143,7 +147,7 @@ function handleNotOkResponse(
}
}

const throwError = (
export const throwError = (
response: APIResponse,
url: string,
searchInputs?: QueryParamData,
Expand All @@ -155,32 +159,33 @@ const throwError = (
searchInputs,
);

// Include just firstError for now, we can expand this
// If we need ValidationErrors to be more expanded
const error = firstError ? { message, firstError } : { message };
const details = {
searchInputs,
...(firstError || {}),
};
switch (status_code) {
case 400:
throw new BadRequestError(error, searchInputs);
throw new BadRequestError(message, details);
case 401:
throw new UnauthorizedError(error, searchInputs);
throw new UnauthorizedError(message, details);
case 403:
throw new ForbiddenError(error, searchInputs);
throw new ForbiddenError(message, details);
case 404:
throw new NotFoundError(error, searchInputs);
throw new NotFoundError(message, details);
case 422:
throw new ValidationError(error, searchInputs);
throw new ValidationError(message, details);
case 408:
throw new RequestTimeoutError(error, searchInputs);
throw new RequestTimeoutError(message, details);
case 500:
throw new InternalServerError(error, searchInputs);
throw new InternalServerError(message, details);
case 503:
throw new ServiceUnavailableError(error, searchInputs);
throw new ServiceUnavailableError(message, details);
default:
throw new ApiRequestError(
error,
message,
"APIRequestError",
status_code,
searchInputs,
details,
);
}
};
9 changes: 7 additions & 2 deletions frontend/src/services/fetch/fetchers/clientUserFetcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,15 @@ export const userFetcher: UserFetcher = async (url) => {
try {
response = await fetch(url, { cache: "no-store" });
} catch (e) {
const error = e as Error;
console.error("User session fetch network error", e);
throw new ApiRequestError(0); // Network error
throw new ApiRequestError(error.message, "NetworkError", 0); // Network error
}
if (response.status === 204) return undefined;
if (response.ok) return (await response.json()) as UserSession;
throw new ApiRequestError(response.status);
throw new ApiRequestError(
"Unknown error fetching user",
undefined,
response.status,
);
};
2 changes: 1 addition & 1 deletion frontend/src/services/fetch/fetchers/fetchers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export function requesterForEndpoint<ResponseType extends APIResponse>({
return async function (
options: {
subPath?: string;
queryParamData?: QueryParamData;
queryParamData?: QueryParamData; // only used for error handling purposes
body?: JSONRequestBody;
additionalHeaders?: HeadersDict;
} = {},
Expand Down
5 changes: 4 additions & 1 deletion frontend/src/services/fetch/fetchers/searchFetcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,10 @@ export const searchForOpportunities = async (searchInputs: QueryParamData) => {
requestBody.query = query;
}

const response = await fetchOpportunitySearch({ body: requestBody });
const response = await fetchOpportunitySearch({
body: requestBody,
queryParamData: searchInputs,
});

response.actionType = searchInputs.actionType;
response.fieldChanged = searchInputs.fieldChanged;
Expand Down
10 changes: 10 additions & 0 deletions frontend/src/types/apiResponseTypes.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { QueryParamData } from "src/types/search/searchRequestTypes";

export interface PaginationInfo {
order_by: string;
page_offset: number;
Expand All @@ -15,3 +17,11 @@ export interface APIResponse {
warnings?: unknown[] | null | undefined;
errors?: unknown[] | null | undefined;
}

export interface FrontendErrorDetails {
status?: number;
searchInputs?: QueryParamData;
field?: string;
message?: string;
type?: string;
}
Loading