Skip to content

Commit

Permalink
[ML] Fixes to error handling for analytics jobs and file data viz (#6…
Browse files Browse the repository at this point in the history
…0249) (#60354)

* [ML] Fixes to error handling for analytics jobs and file data viz

* [ML] Fix failing tests and address comments from review

* [ML] Add key prop to error messages map

* [ML] Add errors.ts
  • Loading branch information
peteharverson committed Mar 17, 2020
1 parent 3b37ba0 commit 994fb9d
Show file tree
Hide file tree
Showing 9 changed files with 89 additions and 33 deletions.
18 changes: 18 additions & 0 deletions x-pack/plugins/ml/common/types/errors.ts
Original file line number Diff line number Diff line change
@@ -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;
}
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export interface DataFrameAnalyticsStats {
transport_address: string;
};
progress: ProgressSection[];
reason?: string;
failure_reason?: string;
state: DATA_FRAME_TASK_STATE;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
loadEvalData,
Eval,
} from '../../../../common';
import { getTaskStateBadge } from './columns';
import { isCompletedAnalyticsJob } from './common';
import {
isRegressionAnalysis,
Expand Down Expand Up @@ -157,8 +158,15 @@ 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 => {
return { title: s[0].toString(), description: getItemDescription(s[1]) };
items: Object.entries(stateValues).map(([stateKey, stateValue]) => {
const title = stateKey.toString();
if (title === 'state') {
return {
title,
description: getTaskStateBadge(getItemDescription(stateValue)),
};
}
return { title, description: getItemDescription(stateValue) };
}),
position: 'left',
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,25 +6,34 @@

import React, { Fragment, 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) => (
<Fragment key={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" />
</Fragment>
))}
</>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -40,6 +41,10 @@ export interface CreateAnalyticsFormProps {
}

export function getErrorMessage(error: any) {
if (isErrorResponse(error)) {
return `${error.body.error}: ${error.body.message}`;
}

if (typeof error === 'object' && typeof error.message === 'string') {
return error.message;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -177,12 +178,20 @@ export class FileDataVisualizerView extends Component {
});
} catch (error) {
console.error(error);

let serverErrorMsg;
if (isErrorResponse(error) === 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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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',
},
Expand Down

0 comments on commit 994fb9d

Please sign in to comment.