From 7d4eb54db47449c42e35a8747ea549dcf1952202 Mon Sep 17 00:00:00 2001
From: Pete Harverson <pete@elastic.co>
Date: Mon, 16 Mar 2020 12:05:00 +0000
Subject: [PATCH 1/4] [ML] Fixes to error handling for analytics jobs and file
 data viz

---
 .../components/analytics_list/columns.tsx     |  8 ++--
 .../components/analytics_list/common.ts       |  2 +-
 .../analytics_list/expanded_row.tsx           |  7 ++-
 .../create_analytics_form/messages.tsx        | 43 +++++++++++--------
 .../use_create_analytics_form.ts              | 21 +++++++--
 .../file_datavisualizer_view.js               | 12 +++++-
 6 files changed, 65 insertions(+), 28 deletions(-)

diff --git a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/analytics_list/columns.tsx b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/analytics_list/columns.tsx
index 00cd9e3f1e0dd..2ab8cb4a78d86 100644
--- a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/analytics_list/columns.tsx
+++ b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/analytics_list/columns.tsx
@@ -43,13 +43,13 @@ enum TASK_STATE_COLOR {
 
 export const getTaskStateBadge = (
   state: DataFrameAnalyticsStats['state'],
-  reason?: DataFrameAnalyticsStats['reason']
+  failureReason?: DataFrameAnalyticsStats['failure_reason']
 ) => {
   const color = TASK_STATE_COLOR[state];
 
-  if (isDataFrameAnalyticsFailed(state) && reason !== undefined) {
+  if (isDataFrameAnalyticsFailed(state) && failureReason !== undefined) {
     return (
-      <EuiToolTip content={reason}>
+      <EuiToolTip content={failureReason}>
         <EuiBadge className="mlTaskStateBadge" color={color}>
           {state}
         </EuiBadge>
@@ -229,7 +229,7 @@ export const getColumns = (
       sortable: (item: DataFrameAnalyticsListRow) => item.stats.state,
       truncateText: true,
       render(item: DataFrameAnalyticsListRow) {
-        return getTaskStateBadge(item.stats.state, item.stats.reason);
+        return getTaskStateBadge(item.stats.state, item.stats.failure_reason);
       },
       width: '100px',
       'data-test-subj': 'mlAnalyticsTableColumnStatus',
diff --git a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/analytics_list/common.ts b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/analytics_list/common.ts
index ff7da8d67852f..2c3ded52eba9b 100644
--- a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/analytics_list/common.ts
+++ b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/analytics_list/common.ts
@@ -50,7 +50,7 @@ export interface DataFrameAnalyticsStats {
     transport_address: string;
   };
   progress: ProgressSection[];
-  reason?: string;
+  failure_reason?: string;
   state: DATA_FRAME_TASK_STATE;
 }
 
diff --git a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/analytics_list/expanded_row.tsx b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/analytics_list/expanded_row.tsx
index 8772be698bf58..ab1946a6b25e3 100644
--- a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/analytics_list/expanded_row.tsx
+++ b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/analytics_list/expanded_row.tsx
@@ -24,6 +24,7 @@ import {
   loadEvalData,
   Eval,
 } from '../../../../common';
+import { getTaskStateBadge } from './columns';
 import { isCompletedAnalyticsJob } from './common';
 import {
   isRegressionAnalysis,
@@ -158,7 +159,11 @@ export const ExpandedRow: FC<Props> = ({ item }) => {
       defaultMessage: 'State',
     }),
     items: Object.entries(stateValues).map(s => {
-      return { title: s[0].toString(), description: getItemDescription(s[1]) };
+      if (s[0].toString() === 'state') {
+        return { title: s[0].toString(), description: getTaskStateBadge(getItemDescription(s[1])) };
+      } else {
+        return { title: s[0].toString(), description: getItemDescription(s[1]) };
+      }
     }),
     position: 'left',
   };
diff --git a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/create_analytics_form/messages.tsx b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/create_analytics_form/messages.tsx
index f228d8fe90097..65691c3e49486 100644
--- a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/create_analytics_form/messages.tsx
+++ b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/create_analytics_form/messages.tsx
@@ -4,27 +4,36 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import React, { Fragment, FC } from 'react';
+import React, { FC } from 'react';
 
-import { EuiCallOut, EuiSpacer } from '@elastic/eui';
+import { EuiCallOut, EuiCodeBlock, EuiSpacer } from '@elastic/eui';
 
 import { FormMessage } from '../../hooks/use_create_analytics_form/state'; // State
 
 interface Props {
-  messages: any; // TODO: fix --> something like State['requestMessages'];
+  messages: FormMessage[];
 }
 
-export const Messages: FC<Props> = ({ messages }) =>
-  messages.map((requestMessage: FormMessage, i: number) => (
-    <Fragment key={i}>
-      <EuiCallOut
-        title={requestMessage.message}
-        color={requestMessage.error !== undefined ? 'danger' : 'primary'}
-        iconType={requestMessage.error !== undefined ? 'alert' : 'checkInCircleFilled'}
-        size="s"
-      >
-        {requestMessage.error !== undefined ? <p>{requestMessage.error}</p> : null}
-      </EuiCallOut>
-      <EuiSpacer size="s" />
-    </Fragment>
-  ));
+export const Messages: FC<Props> = ({ messages }) => {
+  return (
+    <>
+      {messages.map((requestMessage, i) => (
+        <>
+          <EuiCallOut
+            title={requestMessage.message}
+            color={requestMessage.error !== undefined ? 'danger' : 'primary'}
+            iconType={requestMessage.error !== undefined ? 'alert' : 'checkInCircleFilled'}
+            size="s"
+          >
+            {requestMessage.error !== undefined && (
+              <EuiCodeBlock language="json" fontSize="s" paddingSize="s" isCopyable>
+                {requestMessage.error}
+              </EuiCodeBlock>
+            )}
+          </EuiCallOut>
+          <EuiSpacer size="s" />
+        </>
+      ))}
+    </>
+  );
+};
diff --git a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/use_create_analytics_form.ts b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/use_create_analytics_form.ts
index 74161d7c48c24..1bf3a03796fd3 100644
--- a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/use_create_analytics_form.ts
+++ b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/use_create_analytics_form.ts
@@ -39,12 +39,25 @@ export interface CreateAnalyticsFormProps {
   state: State;
 }
 
+interface ErrorResponse {
+  body: {
+    statusCode: number;
+    error: string;
+    message: string;
+  };
+  name: string;
+}
+
+const isErrorResponse = (arg: any): arg is ErrorResponse => {
+  return arg?.body?.error !== undefined && arg?.body?.message !== undefined;
+};
+
 export function getErrorMessage(error: any) {
-  if (typeof error === 'object' && typeof error.message === 'string') {
-    return error.message;
+  if (isErrorResponse(error)) {
+    return `${error.body.error}: ${error.body.message}`;
+  } else {
+    return JSON.stringify(error, null, 2);
   }
-
-  return JSON.stringify(error);
 }
 
 export const useCreateAnalyticsForm = (): CreateAnalyticsFormProps => {
diff --git a/x-pack/plugins/ml/public/application/datavisualizer/file_based/components/file_datavisualizer_view/file_datavisualizer_view.js b/x-pack/plugins/ml/public/application/datavisualizer/file_based/components/file_datavisualizer_view/file_datavisualizer_view.js
index a4300de5abbbb..c5b0f0b4691b7 100644
--- a/x-pack/plugins/ml/public/application/datavisualizer/file_based/components/file_datavisualizer_view/file_datavisualizer_view.js
+++ b/x-pack/plugins/ml/public/application/datavisualizer/file_based/components/file_datavisualizer_view/file_datavisualizer_view.js
@@ -177,12 +177,22 @@ export class FileDataVisualizerView extends Component {
       });
     } catch (error) {
       console.error(error);
+
+      let serverErrorMsg;
+      const isErrorResponse =
+        error?.body?.error !== undefined && error?.body?.message !== undefined;
+      if (isErrorResponse === true) {
+        serverErrorMsg = `${error.body.error}: ${error.body.message}`;
+      } else {
+        serverErrorMsg = JSON.stringify(error, null, 2);
+      }
+
       this.setState({
         results: undefined,
         loaded: false,
         loading: false,
         fileCouldNotBeRead: true,
-        serverErrorMessage: error.message,
+        serverErrorMessage: serverErrorMsg,
       });
 
       // as long as the previous overrides are different to the current overrides,

From d8926e25a3a4e1c7e3dd20a6342c15cba035bfc5 Mon Sep 17 00:00:00 2001
From: Pete Harverson <pete@elastic.co>
Date: Mon, 16 Mar 2020 17:17:34 +0000
Subject: [PATCH 2/4] [ML] Fix failing tests and address comments from review

---
 .../analytics_list/expanded_row.tsx           | 12 ++++++----
 .../use_create_analytics_form.test.tsx        | 23 ++++++++++++-------
 .../use_create_analytics_form.ts              | 22 ++++++------------
 .../file_datavisualizer_view.js               |  5 ++--
 .../components/analytics_panel/table.tsx      |  2 +-
 5 files changed, 32 insertions(+), 32 deletions(-)

diff --git a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/analytics_list/expanded_row.tsx b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/analytics_list/expanded_row.tsx
index ab1946a6b25e3..27bafc60ce943 100644
--- a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/analytics_list/expanded_row.tsx
+++ b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/analytics_list/expanded_row.tsx
@@ -158,12 +158,14 @@ export const ExpandedRow: FC<Props> = ({ item }) => {
     title: i18n.translate('xpack.ml.dataframe.analyticsList.expandedRow.tabs.jobSettings.state', {
       defaultMessage: 'State',
     }),
-    items: Object.entries(stateValues).map(s => {
-      if (s[0].toString() === 'state') {
-        return { title: s[0].toString(), description: getTaskStateBadge(getItemDescription(s[1])) };
-      } else {
-        return { title: s[0].toString(), description: getItemDescription(s[1]) };
+    items: Object.entries(stateValues).map(([stateKey, stateValue]) => {
+      if (stateKey.toString() === 'state') {
+        return {
+          title: stateKey.toString(),
+          description: getTaskStateBadge(getItemDescription(stateValue)),
+        };
       }
+      return { title: stateKey.toString(), description: getItemDescription(stateValue) };
     }),
     position: 'left',
   };
diff --git a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/use_create_analytics_form.test.tsx b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/use_create_analytics_form.test.tsx
index 2bdcc28e31fff..1a248f8559ffa 100644
--- a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/use_create_analytics_form.test.tsx
+++ b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/use_create_analytics_form.test.tsx
@@ -22,16 +22,23 @@ const getMountedHook = () =>
 
 describe('getErrorMessage()', () => {
   test('verify error message response formats', () => {
-    const errorMessage = getErrorMessage(new Error('the-error-message'));
-    expect(errorMessage).toBe('the-error-message');
+    const customError1 = {
+      body: { statusCode: 403, error: 'Forbidden', message: 'the-error-message' },
+    };
+    const errorMessage1 = getErrorMessage(customError1);
+    expect(errorMessage1).toBe('Forbidden: the-error-message');
 
-    const customError1 = { customErrorMessage: 'the-error-message' };
-    const errorMessageMessage1 = getErrorMessage(customError1);
-    expect(errorMessageMessage1).toBe('{"customErrorMessage":"the-error-message"}');
+    const customError2 = new Error('the-error-message');
+    const errorMessage2 = getErrorMessage(customError2);
+    expect(errorMessage2).toBe('the-error-message');
 
-    const customError2 = { message: 'the-error-message' };
-    const errorMessageMessage2 = getErrorMessage(customError2);
-    expect(errorMessageMessage2).toBe('the-error-message');
+    const customError3 = { customErrorMessage: 'the-error-message' };
+    const errorMessage3 = getErrorMessage(customError3);
+    expect(errorMessage3).toBe('{"customErrorMessage":"the-error-message"}');
+
+    const customError4 = { message: 'the-error-message' };
+    const errorMessage4 = getErrorMessage(customError4);
+    expect(errorMessage4).toBe('the-error-message');
   });
 });
 
diff --git a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/use_create_analytics_form.ts b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/use_create_analytics_form.ts
index 1bf3a03796fd3..3b6b8538c2ffd 100644
--- a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/use_create_analytics_form.ts
+++ b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/use_create_analytics_form.ts
@@ -9,6 +9,7 @@ import { useReducer } from 'react';
 import { i18n } from '@kbn/i18n';
 
 import { SimpleSavedObject } from 'kibana/public';
+import { isErrorResponse } from '../../../../../../../common/types/errors';
 import { ml } from '../../../../../services/ml_api_service';
 import { useMlContext } from '../../../../../contexts/ml';
 
@@ -39,25 +40,16 @@ export interface CreateAnalyticsFormProps {
   state: State;
 }
 
-interface ErrorResponse {
-  body: {
-    statusCode: number;
-    error: string;
-    message: string;
-  };
-  name: string;
-}
-
-const isErrorResponse = (arg: any): arg is ErrorResponse => {
-  return arg?.body?.error !== undefined && arg?.body?.message !== undefined;
-};
-
 export function getErrorMessage(error: any) {
   if (isErrorResponse(error)) {
     return `${error.body.error}: ${error.body.message}`;
-  } else {
-    return JSON.stringify(error, null, 2);
   }
+
+  if (typeof error === 'object' && typeof error.message === 'string') {
+    return error.message;
+  }
+
+  return JSON.stringify(error);
 }
 
 export const useCreateAnalyticsForm = (): CreateAnalyticsFormProps => {
diff --git a/x-pack/plugins/ml/public/application/datavisualizer/file_based/components/file_datavisualizer_view/file_datavisualizer_view.js b/x-pack/plugins/ml/public/application/datavisualizer/file_based/components/file_datavisualizer_view/file_datavisualizer_view.js
index c5b0f0b4691b7..12e5a14b51871 100644
--- a/x-pack/plugins/ml/public/application/datavisualizer/file_based/components/file_datavisualizer_view/file_datavisualizer_view.js
+++ b/x-pack/plugins/ml/public/application/datavisualizer/file_based/components/file_datavisualizer_view/file_datavisualizer_view.js
@@ -19,6 +19,7 @@ import { FileCouldNotBeRead, FileTooLarge } from './file_error_callouts';
 import { EditFlyout } from '../edit_flyout';
 import { ImportView } from '../import_view';
 import { MAX_BYTES } from '../../../../../../common/constants/file_datavisualizer';
+import { isErrorResponse } from '../../../../../../common/types/errors';
 import {
   readFile,
   createUrlOverrides,
@@ -179,9 +180,7 @@ export class FileDataVisualizerView extends Component {
       console.error(error);
 
       let serverErrorMsg;
-      const isErrorResponse =
-        error?.body?.error !== undefined && error?.body?.message !== undefined;
-      if (isErrorResponse === true) {
+      if (isErrorResponse(error) === true) {
         serverErrorMsg = `${error.body.error}: ${error.body.message}`;
       } else {
         serverErrorMsg = JSON.stringify(error, null, 2);
diff --git a/x-pack/plugins/ml/public/application/overview/components/analytics_panel/table.tsx b/x-pack/plugins/ml/public/application/overview/components/analytics_panel/table.tsx
index ff81f0e87aca8..c3b8e8dd4e27f 100644
--- a/x-pack/plugins/ml/public/application/overview/components/analytics_panel/table.tsx
+++ b/x-pack/plugins/ml/public/application/overview/components/analytics_panel/table.tsx
@@ -61,7 +61,7 @@ export const AnalyticsTable: FC<Props> = ({ items }) => {
       sortable: (item: DataFrameAnalyticsListRow) => item.stats.state,
       truncateText: true,
       render(item: DataFrameAnalyticsListRow) {
-        return getTaskStateBadge(item.stats.state, item.stats.reason);
+        return getTaskStateBadge(item.stats.state, item.stats.failure_reason);
       },
       width: '100px',
     },

From d3df262ad58866dd612cd3621502d911329d582c Mon Sep 17 00:00:00 2001
From: Pete Harverson <pete@elastic.co>
Date: Mon, 16 Mar 2020 17:36:27 +0000
Subject: [PATCH 3/4] [ML] Add key prop to error messages map

---
 .../components/analytics_list/expanded_row.tsx             | 7 ++++---
 .../components/create_analytics_form/messages.tsx          | 6 +++---
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/analytics_list/expanded_row.tsx b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/analytics_list/expanded_row.tsx
index 27bafc60ce943..43ef6b36c3972 100644
--- a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/analytics_list/expanded_row.tsx
+++ b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/analytics_list/expanded_row.tsx
@@ -159,13 +159,14 @@ export const ExpandedRow: FC<Props> = ({ item }) => {
       defaultMessage: 'State',
     }),
     items: Object.entries(stateValues).map(([stateKey, stateValue]) => {
-      if (stateKey.toString() === 'state') {
+      const title = stateKey.toString();
+      if (title === 'state') {
         return {
-          title: stateKey.toString(),
+          title,
           description: getTaskStateBadge(getItemDescription(stateValue)),
         };
       }
-      return { title: stateKey.toString(), description: getItemDescription(stateValue) };
+      return { title, description: getItemDescription(stateValue) };
     }),
     position: 'left',
   };
diff --git a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/create_analytics_form/messages.tsx b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/create_analytics_form/messages.tsx
index 65691c3e49486..68a9a264ddf78 100644
--- a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/create_analytics_form/messages.tsx
+++ b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/create_analytics_form/messages.tsx
@@ -4,7 +4,7 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import React, { FC } from 'react';
+import React, { Fragment, FC } from 'react';
 
 import { EuiCallOut, EuiCodeBlock, EuiSpacer } from '@elastic/eui';
 
@@ -18,7 +18,7 @@ export const Messages: FC<Props> = ({ messages }) => {
   return (
     <>
       {messages.map((requestMessage, i) => (
-        <>
+        <Fragment key={i}>
           <EuiCallOut
             title={requestMessage.message}
             color={requestMessage.error !== undefined ? 'danger' : 'primary'}
@@ -32,7 +32,7 @@ export const Messages: FC<Props> = ({ messages }) => {
             )}
           </EuiCallOut>
           <EuiSpacer size="s" />
-        </>
+        </Fragment>
       ))}
     </>
   );

From 995f03ab83da4711039ca4de2b7c74afbe36e219 Mon Sep 17 00:00:00 2001
From: Pete Harverson <pete@elastic.co>
Date: Mon, 16 Mar 2020 19:44:14 +0000
Subject: [PATCH 4/4] [ML] Add errors.ts

---
 x-pack/plugins/ml/common/types/errors.ts | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
 create mode 100644 x-pack/plugins/ml/common/types/errors.ts

diff --git a/x-pack/plugins/ml/common/types/errors.ts b/x-pack/plugins/ml/common/types/errors.ts
new file mode 100644
index 0000000000000..63e222490082b
--- /dev/null
+++ b/x-pack/plugins/ml/common/types/errors.ts
@@ -0,0 +1,18 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+
+export interface ErrorResponse {
+  body: {
+    statusCode: number;
+    error: string;
+    message: string;
+  };
+  name: string;
+}
+
+export function isErrorResponse(arg: any): arg is ErrorResponse {
+  return arg?.body?.error !== undefined && arg?.body?.message !== undefined;
+}