Enhance error handling for UI sendRequest service (#14971)

frontend/interfaces/errors.ts

@@ -1,4 +1,5 @@
 import PropTypes from "prop-types";
+import { AxiosError, isAxiosError } from "axios";
 
 export default PropTypes.shape({
   http_status: PropTypes.number,
@@ -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 "";
+};

frontend/pages/admin/IntegrationsPage/cards/MdmSettings/WindowsMdmPage/WindowsMdmPage.tsx

@@ -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";
 
@@ -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);
     }

frontend/pages/hosts/details/cards/Scripts/Scripts.tsx

@@ -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";
@@ -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),
     {

frontend/pages/packs/EditPackPage/EditPackPage.tsx

@@ -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";
@@ -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."

frontend/pages/packs/ManagePacksPage/ManagePacksPage.tsx

@@ -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";
@@ -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) {
@@ -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(),
     {

frontend/pages/packs/PackComposerPage/PackComposerPage.tsx

@@ -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";
@@ -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."

frontend/services/entities/config.ts

@@ -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 {
@@ -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);
+    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,
+      { mdm },
+      undefined,
+      undefined,
+      skipParseError
+    );
   },
 
   // This API call is made to a specific endpoint that is different than our

frontend/services/entities/queries.ts

@@ -1,8 +1,8 @@
 /* eslint-disable  @typescript-eslint/explicit-module-boundary-types */
-import sendRequest, { getError } from "services";
+import sendRequest from "services";
 import endpoints from "utilities/endpoints";
+import { getErrorReason } from "interfaces/errors";
 import { ISelectedTargetsForApi } from "interfaces/target";
-import { AxiosResponse } from "axios";
 import {
   ICreateQueryRequestBody,
   IModifyQueryRequestBody,
@@ -70,8 +70,10 @@ export default {
           total: 0,
         },
       });
-    } catch (response) {
-      throw new Error(getError(response as AxiosResponse));
+    } catch (e) {
+      throw new Error(
+        getErrorReason(e) || `run query: parse server error ${e}`
+      );
     }
   },
   update: (id: number, updateParams: IModifyQueryRequestBody) => {

frontend/services/index.ts

@@ -1,18 +1,14 @@
-import axios, {
-  AxiosError,
-  AxiosResponse,
-  ResponseType as AxiosResponseType,
-} from "axios";
+import axios, { isAxiosError, ResponseType as AxiosResponseType } from "axios";
+import URL_PREFIX from "router/url_prefix";
 import { authToken } from "utilities/local";
 
-import URL_PREFIX from "router/url_prefix";
-
-const sendRequest = async (
+export const sendRequest = async (
   method: "GET" | "POST" | "PATCH" | "DELETE" | "HEAD",
   path: string,
   data?: unknown,
   responseType: AxiosResponseType = "json",
-  timeout?: number
+  timeout?: number,
+  skipParseError?: boolean
 ) => {
   const { origin } = global.window.location;
 
@@ -33,20 +29,17 @@ const sendRequest = async (
 
     return response.data;
   } catch (error) {
-    const axiosError = error as AxiosError;
+    if (skipParseError) {
+      return Promise.reject(error);
+    }
+    let reason: unknown | undefined;
+    if (isAxiosError(error)) {
+      reason = error.response || error.message || error.code;
+    }
     return Promise.reject(
-      axiosError.response ||
-        axiosError.message ||
-        axiosError.code ||
-        "unknown axios error"
+      reason || `send request: parse server error: ${error}`
     );
   }
 };
 
-// return the first error
-export const getError = (response: unknown): string => {
-  const r = response as AxiosResponse;
-  return r.data?.errors?.[0]?.reason || ""; // TODO: check if any callers rely on empty return value
-};
-
 export default sendRequest;

frontend/utilities/format_error_response/index.ts

@@ -1,7 +1,7 @@
 import { get, join } from "lodash";
-import { IError, IOldApiError } from "interfaces/errors";
+import { IFleetApiError } from "interfaces/errors";
 
-const formatServerErrors = (errors: IError[]) => {
+const formatServerErrors = (errors: IFleetApiError[]) => {
   if (!errors || !errors.length) {
     return {};
   }