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

Enhance error handling for UI sendRequest service #14971

Merged
merged 5 commits into from
Nov 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
187 changes: 184 additions & 3 deletions frontend/interfaces/errors.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import PropTypes from "prop-types";
import { AxiosError, isAxiosError } from "axios";

export default PropTypes.shape({
http_status: PropTypes.number,
Expand All @@ -11,14 +12,194 @@ export interface IOldApiError {
base: string;
}

export interface IError {
/**
* IFleetApiError is the shape of a Fleet API error. It represents an element of the `errors`
* array in a Fleet API response for failed requests (see `IFleetApiResponseWithErrors`).
*/
export interface IFleetApiError {
name: string;
reason: string;
}

// Response returned by API when there is an error
/**
* IApiError is the shape of a Fleet API response for failed requests.
*
* TODO: Rename to IFleetApiResponseWithErrors
*/
export interface IApiError {
message: string;
errors: IError[];
errors: IFleetApiError[];
uuid?: string;
}

const isFleetApiError = (err: unknown): err is IFleetApiError => {
if (!err || typeof err !== "object" || !("name" in err && "reason" in err)) {
return false;
}
const e = err as Record<"name" | "reason", unknown>;
if (typeof e.name !== "string" || typeof e.reason !== "string") {
return false;
}
return true;
};

interface IRecordWithErrors extends Record<string | number | symbol, unknown> {
errors: unknown[];
}

const isRecordWithErrors = (r: unknown): r is IRecordWithErrors => {
if (!r || typeof r !== "object" || !("errors" in r)) {
return false;
}
const { errors } = r as { errors: unknown };
if (!Array.isArray(errors)) {
return false;
}
return true;
};

interface IRecordWithDataErrors
extends Record<string | number | symbol, unknown> {
data: IRecordWithErrors;
}

const isRecordWithDataErrors = (r: unknown): r is IRecordWithDataErrors => {
if (!r || typeof r !== "object" || !("data" in r)) {
return false;
}
const { data } = r as { data: unknown };
if (!isRecordWithErrors(data)) {
return false;
}
const { errors } = data;
if (!Array.isArray(errors)) {
return false;
}
return true;
};

interface IRecordWithResponseDataErrors
extends Record<string | number | symbol, unknown> {
response: IRecordWithDataErrors;
}

const isRecordWithResponseDataErrors = (
r: unknown
): r is IRecordWithResponseDataErrors => {
if (!r || typeof r !== "object" || !("response" in r)) {
return false;
}
const { response } = r as { response: unknown };
if (!isRecordWithDataErrors(response)) {
return false;
}
return true;
};

interface IFilterFleetErrorBase {
nameEquals?: string;
reasonIncludes?: string;
}

interface IFilterFleetErrorName extends IFilterFleetErrorBase {
nameEquals: string;
reasonIncludes?: never;
}

interface IFilterFleetErrorReason extends IFilterFleetErrorBase {
nameEquals?: never;
reasonIncludes: string;
}

// FilterFleetError is the shape of a filter that can be applied to to filter Fleet
// server errors. It is the union of FilterFleetErrorName and FilterFleetErrorReason,
// which ensures that only one of `nameEquals` or `reasonIncludes` can be specified.
type IFilterFleetError = IFilterFleetErrorName | IFilterFleetErrorReason;

const filterFleetErrorNameEquals = (errs: unknown[], value: string) => {
if (!value || !errs?.length) {
return undefined;
}
return errs?.find((e) => isFleetApiError(e) && e.name === value) as
| IFleetApiError
| undefined;
};

const filterFleetErrorReasonIncludes = (errs: unknown[], value: string) => {
if (!value || !errs?.length) {
return undefined;
}
return errs?.find((e) => isFleetApiError(e) && e.reason?.includes(value)) as
| IFleetApiError
| undefined;
};

const getReasonFromErrors = (errors: unknown[], filter?: IFilterFleetError) => {
if (!errors.length) {
return "";
}

let fleetError: IFleetApiError | undefined;
if (filter?.nameEquals) {
fleetError = filterFleetErrorNameEquals(errors, filter.nameEquals);
} else if (filter?.reasonIncludes) {
fleetError = filterFleetErrorReasonIncludes(errors, filter.reasonIncludes);
} else {
fleetError = isFleetApiError(errors[0]) ? errors[0] : undefined;
}
return fleetError?.reason || "";
};

const getReasonFromRecordWithDataErrors = (
r: IRecordWithDataErrors,
filter?: IFilterFleetError
): string => {
return getReasonFromErrors(r.data.errors, filter);
};

const getReasonFromAxiosError = (
ae: AxiosError,
filter?: IFilterFleetError
): string => {
return isRecordWithDataErrors(ae.response)
? getReasonFromRecordWithDataErrors(ae.response, filter)
: "";
};

/**
* getErrorReason attempts to parse a unknown payload as an `AxiosError` or
* other `Record`-like object with the general shape as follows:
* `{ response: { data: { errors: unknown[] } } }`
*
* It attempts to extract a `reason` from a Fleet API error (i.e. an object
* with `name` and `reason` properties) in the `errors` array, if present.
* Other in values in the payload are generally ignored.
*
* If `filter` is specified, it attempts to find an error that satisfies the filter
* and returns the `reason`, if found. Otherwise, it returns the `reason`
* of the first error, if any.
*
* By default, an empty string is returned as the reason if no error is found.
*/
export const getErrorReason = (
payload: unknown | undefined,
filter?: IFilterFleetError
): string => {
if (isAxiosError(payload)) {
return getReasonFromAxiosError(payload, filter);
}

if (isRecordWithResponseDataErrors(payload)) {
return getReasonFromRecordWithDataErrors(payload.response, filter);
}

if (isRecordWithDataErrors(payload)) {
return getReasonFromRecordWithDataErrors(payload, filter);
}

if (isRecordWithErrors(payload)) {
return getReasonFromErrors(payload.errors, filter);
}

return "";
};
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import React, { useContext } from "react";
import { InjectedRouter } from "react-router";
import { isAxiosError } from "axios";

import PATHS from "router/paths";
import configAPI from "services/entities/config";
import { getErrorReason } from "interfaces/errors";
import { NotificationContext } from "context/notification";
import { AppContext } from "context/app";

Expand Down Expand Up @@ -30,15 +32,23 @@ const useSetWindowsMdm = ({

const turnOnWindowsMdm = async () => {
try {
const updatedConfig = await configAPI.update({
mdm: {
const updatedConfig = await configAPI.updateMDMConfig(
{
windows_enabled_and_configured: enable,
},
});
true
);
setConfig(updatedConfig);
renderFlash("success", successMessage);
} catch {
renderFlash("error", errorMessage);
} catch (e) {
let msg = errorMessage;
if (enable && isAxiosError(e) && e.response?.status === 422) {
msg =
getErrorReason(e, {
nameEquals: "mdm.windows_enabled_and_configured",
}) || msg;
}
renderFlash("error", msg);
} finally {
router.push(PATHS.ADMIN_INTEGRATIONS_MDM);
}
Expand Down
4 changes: 2 additions & 2 deletions frontend/pages/hosts/details/cards/Scripts/Scripts.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import scriptsAPI, {
IHostScript,
IHostScriptsResponse,
} from "services/entities/scripts";
import { IApiError, IError } from "interfaces/errors";
import { IApiError } from "interfaces/errors";
import { NotificationContext } from "context/notification";

import Card from "components/Card";
Expand Down Expand Up @@ -43,7 +43,7 @@ const Scripts = ({
isLoading: isLoadingScriptData,
isError: isErrorScriptData,
refetch: refetchScriptsData,
} = useQuery<IHostScriptsResponse, IError>(
} = useQuery<IHostScriptsResponse, IApiError>(
["scripts", hostId, page],
() => scriptsAPI.getHostScripts(hostId as number, page),
{
Expand Down
11 changes: 7 additions & 4 deletions frontend/pages/packs/EditPackPage/EditPackPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { ITarget, ITargetsAPIResponse } from "interfaces/target";
import { AppContext } from "context/app";
import { NotificationContext } from "context/notification";

import { getError } from "services";
import { getErrorReason } from "interfaces/errors";
import packsAPI from "services/entities/packs";
import queriesAPI from "services/entities/queries";
import scheduledQueriesAPI from "services/entities/scheduled_queries";
Expand Down Expand Up @@ -150,9 +150,12 @@ const EditPacksPage = ({
router.push(PATHS.MANAGE_PACKS);
renderFlash("success", `Successfully updated this pack.`);
})
.catch((response) => {
const error = getError(response);
if (error.includes("Duplicate entry")) {
.catch((e) => {
if (
getErrorReason(e, {
reasonIncludes: "Duplicate entry",
})
) {
renderFlash(
"error",
"Unable to update pack. Pack names must be unique."
Expand Down
6 changes: 3 additions & 3 deletions frontend/pages/packs/ManagePacksPage/ManagePacksPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { useQuery } from "react-query";
import { InjectedRouter } from "react-router/lib/Router";

import { IPack, IStoredPacksResponse } from "interfaces/pack";
import { IError } from "interfaces/errors";
import { IFleetApiError } from "interfaces/errors";
import { AppContext } from "context/app";
import { NotificationContext } from "context/notification";
import packsAPI from "services/entities/packs";
Expand All @@ -29,7 +29,7 @@ const renderTable = (
onDisablePackClick: (selectedTablePackIds: number[]) => void,
onCreatePackClick: React.MouseEventHandler<HTMLButtonElement>,
packs: IPack[] | undefined,
packsError: IError | null,
packsError: IFleetApiError | null,
isLoadingPacks: boolean
): JSX.Element => {
if (packsError) {
Expand Down Expand Up @@ -65,7 +65,7 @@ const ManagePacksPage = ({ router }: IManagePacksPageProps): JSX.Element => {
error: packsError,
isFetching: isLoadingPacks,
refetch: refetchPacks,
} = useQuery<IStoredPacksResponse, IError, IPack[]>(
} = useQuery<IStoredPacksResponse, IFleetApiError, IPack[]>(
"packs",
() => packsAPI.loadAll(),
{
Expand Down
12 changes: 7 additions & 5 deletions frontend/pages/packs/PackComposerPage/PackComposerPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { IQuery } from "interfaces/query";
import { ITargetsAPIResponse } from "interfaces/target";
import { IEditPackFormData } from "interfaces/pack";

import { getError } from "services";
import { getErrorReason } from "interfaces/errors";
import packsAPI from "services/entities/packs";

import PackForm from "components/forms/packs/PackForm";
Expand Down Expand Up @@ -54,10 +54,12 @@ const PackComposerPage = ({ router }: IPackComposerPageProps): JSX.Element => {
"success",
"Pack successfully created. Add queries to your pack."
);
} catch (response) {
const error = getError(response);

if (error.includes("Duplicate entry")) {
} catch (e) {
if (
getErrorReason(e, {
reasonIncludes: "Duplicate entry",
})
) {
renderFlash(
"error",
"Unable to create pack. Pack names must be unique."
Expand Down
42 changes: 39 additions & 3 deletions frontend/services/entities/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import sendRequest from "services";
import endpoints from "utilities/endpoints";
import { IConfig } from "interfaces/config";
import { IConfig, IMdmConfig } from "interfaces/config";
import axios, { AxiosError } from "axios";

export default {
Expand Down Expand Up @@ -35,10 +35,46 @@ export default {

return sendRequest("GET", GLOBAL_ENROLL_SECRETS);
},
update: (formData: any) => {

/**
* update is used to update the app config.
*
* If the request fails and `skipParseError` is `true`, the caller is
* responsible for verifying that the value of the rejected promise is an AxiosError
* and futher parsing of the the error mesage.
*/
update: (formData: any, skipParseError?: boolean) => {
const { CONFIG } = endpoints;

return sendRequest(
"PATCH",
CONFIG,
formData,
undefined,
undefined,
skipParseError
);
},

/**
* updateMDMConfig is a special case of update that is used to update the MDM
* config.
*
* If the request fails and `skipParseError` is `true`, the caller is
* responsible for verifying that the value of the rejected promise is an AxiosError
* and futher parsing of the the error mesage.
*/
updateMDMConfig: (mdm: Partial<IMdmConfig>, skipParseError?: boolean) => {
const { CONFIG } = endpoints;

return sendRequest("PATCH", CONFIG, formData);
return sendRequest(
"PATCH",
CONFIG,
{ mdm },
undefined,
undefined,
skipParseError
);
},

// This API call is made to a specific endpoint that is different than our
Expand Down
Loading
Loading