From 122df101f239afb1a9b7fdbd155a15dced810e1c Mon Sep 17 00:00:00 2001 From: James Gowdy Date: Fri, 4 Sep 2020 09:00:45 +0100 Subject: [PATCH 01/17] [ML] Improving client side error handling --- x-pack/plugins/ml/common/util/errors.ts | 24 +++--- .../rule_editor/rule_editor_flyout.js | 9 ++- .../components/rule_editor/utils.js | 27 +++---- .../validate_job/validate_job_view.d.ts | 2 +- .../validate_job/validate_job_view.js | 61 ++++++++++----- .../data_frame_analytics/common/analytics.ts | 4 +- .../common/get_index_data.ts | 4 +- .../common/use_results_view_config.ts | 6 +- .../hooks/use_index_data.ts | 4 +- .../use_create_analytics_form.test.tsx | 12 +-- .../use_create_analytics_form.ts | 12 +-- .../components/edit_job_flyout/edit_utils.js | 29 +++---- .../jobs/jobs_list/components/utils.js | 14 ++-- .../post_save_options/post_save_options.tsx | 4 +- .../pages/components/summary_step/summary.tsx | 6 +- .../components/validation_step/validation.tsx | 4 +- .../application/services/job_service.js | 77 ------------------- .../calendars/list/delete_calendars.js | 4 +- 18 files changed, 116 insertions(+), 187 deletions(-) diff --git a/x-pack/plugins/ml/common/util/errors.ts b/x-pack/plugins/ml/common/util/errors.ts index 6c5fa7bd75daf..2a408b1cb665a 100644 --- a/x-pack/plugins/ml/common/util/errors.ts +++ b/x-pack/plugins/ml/common/util/errors.ts @@ -5,22 +5,8 @@ */ import { ResponseError, ResponseHeaders } from 'kibana/server'; -import { isErrorResponse } from '../types/errors'; - -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; - } - - return JSON.stringify(error); -} // Adding temporary types until Kibana ResponseError is updated - export interface BoomResponse { data: any; isBoom: boolean; @@ -135,9 +121,17 @@ export const extractErrorProperties = ( typeof error.body.attributes === 'object' && error.body.attributes.body?.status !== undefined ) { - statusCode = error.body.attributes.body?.status; + statusCode = error.body.attributes.body.status; + + if (typeof error.body.attributes.body.error?.reason === 'string') { + return { + message: error.body.attributes.body.error.reason, + statusCode, + }; + } } + // I DON'T THINK THESE ONES BELOW WILL BE HIT if (typeof error.body.message === 'string') { return { message: error.body.message, diff --git a/x-pack/plugins/ml/public/application/components/rule_editor/rule_editor_flyout.js b/x-pack/plugins/ml/public/application/components/rule_editor/rule_editor_flyout.js index eed57aaf1b491..5b0224c8e9f0a 100644 --- a/x-pack/plugins/ml/public/application/components/rule_editor/rule_editor_flyout.js +++ b/x-pack/plugins/ml/public/application/components/rule_editor/rule_editor_flyout.js @@ -10,6 +10,8 @@ import PropTypes from 'prop-types'; import React, { Component } from 'react'; +import { i18n } from '@kbn/i18n'; +import { FormattedMessage } from '@kbn/i18n/react'; import { EuiButton, @@ -51,8 +53,7 @@ import { getPartitioningFieldNames } from '../../../../common/util/job_utils'; import { withKibana } from '../../../../../../../src/plugins/kibana_react/public'; import { mlJobService } from '../../services/job_service'; import { ml } from '../../services/ml_api_service'; -import { i18n } from '@kbn/i18n'; -import { FormattedMessage } from '@kbn/i18n/react'; +import { extractErrorMessage } from '../../../../common/util/errors'; class RuleEditorFlyoutUI extends Component { static propTypes = { @@ -431,8 +432,8 @@ class RuleEditorFlyoutUI extends Component { values: { jobId }, } ); - if (error.message) { - errorMessage += ` : ${error.message}`; + if (error.error) { + errorMessage += ` : ${extractErrorMessage(error.error)}`; } toasts.addDanger(errorMessage); }); diff --git a/x-pack/plugins/ml/public/application/components/rule_editor/utils.js b/x-pack/plugins/ml/public/application/components/rule_editor/utils.js index a697eed00fd66..d5072ae699d61 100644 --- a/x-pack/plugins/ml/public/application/components/rule_editor/utils.js +++ b/x-pack/plugins/ml/public/application/components/rule_editor/utils.js @@ -146,22 +146,17 @@ export function updateJobRules(job, detectorIndex, rules) { } return new Promise((resolve, reject) => { - mlJobService - .updateJob(jobId, jobData) - .then((resp) => { - if (resp.success) { - // Refresh the job data in the job service before resolving. - mlJobService - .refreshJob(jobId) - .then(() => { - resolve({ success: true }); - }) - .catch((refreshResp) => { - reject(refreshResp); - }); - } else { - reject(resp); - } + ml.updateJob(jobId, jobData) + .then(() => { + // Refresh the job data in the job service before resolving. + mlJobService + .refreshJob(jobId) + .then(() => { + resolve({ success: true }); + }) + .catch((refreshResp) => { + reject(refreshResp); + }); }) .catch((resp) => { reject(resp); diff --git a/x-pack/plugins/ml/public/application/components/validate_job/validate_job_view.d.ts b/x-pack/plugins/ml/public/application/components/validate_job/validate_job_view.d.ts index 43e0a5f3eac78..35e4e189b4326 100644 --- a/x-pack/plugins/ml/public/application/components/validate_job/validate_job_view.d.ts +++ b/x-pack/plugins/ml/public/application/components/validate_job/validate_job_view.d.ts @@ -8,7 +8,7 @@ import { FC } from 'react'; declare const ValidateJob: FC<{ getJobConfig: any; getDuration: any; - mlJobService: any; + ml: any; embedded?: boolean; setIsValid?: (valid: boolean) => void; idFilterList?: string[]; diff --git a/x-pack/plugins/ml/public/application/components/validate_job/validate_job_view.js b/x-pack/plugins/ml/public/application/components/validate_job/validate_job_view.js index dde6925631d3e..0c079bc11cffc 100644 --- a/x-pack/plugins/ml/public/application/components/validate_job/validate_job_view.js +++ b/x-pack/plugins/ml/public/application/components/validate_job/validate_job_view.js @@ -32,6 +32,8 @@ import { getDocLinks } from '../../util/dependency_cache'; import { VALIDATION_STATUS } from '../../../../common/constants/validation'; import { getMostSevereMessageStatus } from '../../../../common/util/validation_utils'; +import { toastNotificationServiceProvider } from '../../services/toast_notification_service'; +import { withKibana } from '../../../../../../../src/plugins/kibana_react/public'; const defaultIconType = 'questionInCircle'; const getDefaultState = () => ({ @@ -182,7 +184,7 @@ Modal.propType = { title: PropTypes.string, }; -export class ValidateJob extends Component { +export class ValidateJobUI extends Component { constructor(props) { super(props); this.state = getDefaultState(); @@ -209,25 +211,40 @@ export class ValidateJob extends Component { if (typeof job === 'object') { let shouldShowLoadingIndicator = true; - this.props.mlJobService.validateJob({ duration, fields, job }).then((data) => { - shouldShowLoadingIndicator = false; - this.setState({ - ...this.state, - ui: { - ...this.state.ui, - iconType: statusToEuiIconType(getMostSevereMessageStatus(data.messages)), - isLoading: false, - isModalVisible: true, - }, - data, - title: job.job_id, - }); - if (typeof this.props.setIsValid === 'function') { - this.props.setIsValid( - data.messages.some((m) => m.status === VALIDATION_STATUS.ERROR) === false + this.props.ml + .validateJob({ duration, fields, job }) + .then((messages) => { + shouldShowLoadingIndicator = false; + this.setState({ + ...this.state, + ui: { + ...this.state.ui, + iconType: statusToEuiIconType(getMostSevereMessageStatus(messages)), + isLoading: false, + isModalVisible: true, + }, + data: { + messages, + success: true, + }, + title: job.job_id, + }); + if (typeof this.props.setIsValid === 'function') { + this.props.setIsValid( + messages.some((m) => m.status === VALIDATION_STATUS.ERROR) === false + ); + } + }) + .catch((error) => { + const { toasts } = this.props.kibana.services.notifications; + const toastNotificationService = toastNotificationServiceProvider(toasts); + toastNotificationService.displayErrorToast( + error, + i18n.translate('xpack.ml.jobService.validateJobErrorTitle', { + defaultMessage: 'Job Validation Error', + }) ); - } - }); + }); // wait for 250ms before triggering the loading indicator // to avoid flickering when there's a loading time below @@ -335,15 +352,17 @@ export class ValidateJob extends Component { ); } } -ValidateJob.propTypes = { +ValidateJobUI.propTypes = { fields: PropTypes.object, fill: PropTypes.bool, getDuration: PropTypes.func, getJobConfig: PropTypes.func.isRequired, isCurrentJobConfig: PropTypes.bool, isDisabled: PropTypes.bool, - mlJobService: PropTypes.object.isRequired, + ml: PropTypes.object.isRequired, embedded: PropTypes.bool, setIsValid: PropTypes.func, idFilterList: PropTypes.array, }; + +export const ValidateJob = withKibana(ValidateJobUI); diff --git a/x-pack/plugins/ml/public/application/data_frame_analytics/common/analytics.ts b/x-pack/plugins/ml/public/application/data_frame_analytics/common/analytics.ts index 8ad861e616b7a..2096eda45311c 100644 --- a/x-pack/plugins/ml/public/application/data_frame_analytics/common/analytics.ts +++ b/x-pack/plugins/ml/public/application/data_frame_analytics/common/analytics.ts @@ -10,7 +10,7 @@ import { distinctUntilChanged, filter } from 'rxjs/operators'; import { cloneDeep } from 'lodash'; import { ml } from '../../services/ml_api_service'; import { Dictionary } from '../../../../common/types/common'; -import { getErrorMessage } from '../../../../common/util/errors'; +import { extractErrorMessage } from '../../../../common/util/errors'; import { SavedSearchQuery } from '../../contexts/ml'; import { AnalysisConfig, @@ -549,7 +549,7 @@ export const loadEvalData = async ({ results.eval = evalResult; return results; } catch (e) { - results.error = getErrorMessage(e); + results.error = extractErrorMessage(e); return results; } }; diff --git a/x-pack/plugins/ml/public/application/data_frame_analytics/common/get_index_data.ts b/x-pack/plugins/ml/public/application/data_frame_analytics/common/get_index_data.ts index c162cb2754c10..53c0f02fd9a80 100644 --- a/x-pack/plugins/ml/public/application/data_frame_analytics/common/get_index_data.ts +++ b/x-pack/plugins/ml/public/application/data_frame_analytics/common/get_index_data.ts @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import { getErrorMessage } from '../../../../common/util/errors'; +import { extractErrorMessage } from '../../../../common/util/errors'; import { EsSorting, SearchResponse7, UseDataGridReturnType } from '../../components/data_grid'; import { ml } from '../../services/ml_api_service'; @@ -62,7 +62,7 @@ export const getIndexData = async ( setTableItems(docs); setStatus(INDEX_STATUS.LOADED); } catch (e) { - setErrorMessage(getErrorMessage(e)); + setErrorMessage(extractErrorMessage(e)); setStatus(INDEX_STATUS.ERROR); } } diff --git a/x-pack/plugins/ml/public/application/data_frame_analytics/common/use_results_view_config.ts b/x-pack/plugins/ml/public/application/data_frame_analytics/common/use_results_view_config.ts index fde1b26106508..b0e73edff7476 100644 --- a/x-pack/plugins/ml/public/application/data_frame_analytics/common/use_results_view_config.ts +++ b/x-pack/plugins/ml/public/application/data_frame_analytics/common/use_results_view_config.ts @@ -8,7 +8,7 @@ import { useEffect, useState } from 'react'; import { IndexPattern } from '../../../../../../../src/plugins/data/public'; -import { getErrorMessage } from '../../../../common/util/errors'; +import { extractErrorMessage } from '../../../../common/util/errors'; import { getIndexPatternIdFromName } from '../../util/index_utils'; import { ml } from '../../services/ml_api_service'; @@ -83,12 +83,12 @@ export const useResultsViewConfig = (jobId: string) => { setIsLoadingJobConfig(false); } } catch (e) { - setJobCapsServiceErrorMessage(getErrorMessage(e)); + setJobCapsServiceErrorMessage(extractErrorMessage(e)); setIsLoadingJobConfig(false); } } } catch (e) { - setJobConfigErrorMessage(getErrorMessage(e)); + setJobConfigErrorMessage(extractErrorMessage(e)); setIsLoadingJobConfig(false); } })(); diff --git a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_creation/hooks/use_index_data.ts b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_creation/hooks/use_index_data.ts index eab5165a42137..ea958c8c4a3a3 100644 --- a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_creation/hooks/use_index_data.ts +++ b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_creation/hooks/use_index_data.ts @@ -25,7 +25,7 @@ import { SearchResponse7, UseIndexDataReturnType, } from '../../../../components/data_grid'; -import { getErrorMessage } from '../../../../../../common/util/errors'; +import { extractErrorMessage } from '../../../../../../common/util/errors'; import { INDEX_STATUS } from '../../../common/analytics'; import { ml } from '../../../../services/ml_api_service'; @@ -94,7 +94,7 @@ export const useIndexData = ( setTableItems(docs); setStatus(INDEX_STATUS.LOADED); } catch (e) { - setErrorMessage(getErrorMessage(e)); + setErrorMessage(extractErrorMessage(e)); setStatus(INDEX_STATUS.ERROR); } }; 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 ac1c710e1d106..3fe0f9e92cb30 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 @@ -11,7 +11,7 @@ import { MlContext } from '../../../../../contexts/ml'; import { kibanaContextValueMock } from '../../../../../contexts/ml/__mocks__/kibana_context_value'; import { useCreateAnalyticsForm } from './use_create_analytics_form'; -import { getErrorMessage } from '../../../../../../../common/util/errors'; +import { extractErrorMessage } from '../../../../../../../common/util/errors'; const getMountedHook = () => mountHook( @@ -21,24 +21,24 @@ const getMountedHook = () => ) ); -describe('getErrorMessage()', () => { +describe('extractErrorMessage()', () => { test('verify error message response formats', () => { const customError1 = { body: { statusCode: 403, error: 'Forbidden', message: 'the-error-message' }, }; - const errorMessage1 = getErrorMessage(customError1); + const errorMessage1 = extractErrorMessage(customError1); expect(errorMessage1).toBe('Forbidden: the-error-message'); const customError2 = new Error('the-error-message'); - const errorMessage2 = getErrorMessage(customError2); + const errorMessage2 = extractErrorMessage(customError2); expect(errorMessage2).toBe('the-error-message'); const customError3 = { customErrorMessage: 'the-error-message' }; - const errorMessage3 = getErrorMessage(customError3); + const errorMessage3 = extractErrorMessage(customError3); expect(errorMessage3).toBe('{"customErrorMessage":"the-error-message"}'); const customError4 = { message: 'the-error-message' }; - const errorMessage4 = getErrorMessage(customError4); + const errorMessage4 = extractErrorMessage(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 9612b9213d120..161dde51df43e 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 @@ -8,7 +8,7 @@ import { useReducer } from 'react'; import { i18n } from '@kbn/i18n'; -import { getErrorMessage } from '../../../../../../../common/util/errors'; +import { extractErrorMessage } from '../../../../../../../common/util/errors'; import { DeepReadonly } from '../../../../../../../common/types/common'; import { ml } from '../../../../../services/ml_api_service'; import { useMlContext } from '../../../../../contexts/ml'; @@ -115,7 +115,7 @@ export const useCreateAnalyticsForm = (): CreateAnalyticsFormProps => { refresh(); } catch (e) { addRequestMessage({ - error: getErrorMessage(e), + error: extractErrorMessage(e), message: i18n.translate( 'xpack.ml.dataframe.analytics.create.errorCreatingDataFrameAnalyticsJob', { @@ -178,7 +178,7 @@ export const useCreateAnalyticsForm = (): CreateAnalyticsFormProps => { }); } catch (e) { addRequestMessage({ - error: getErrorMessage(e), + error: extractErrorMessage(e), message: i18n.translate( 'xpack.ml.dataframe.analytics.create.createIndexPatternErrorMessage', { @@ -199,7 +199,7 @@ export const useCreateAnalyticsForm = (): CreateAnalyticsFormProps => { ); } catch (e) { addRequestMessage({ - error: getErrorMessage(e), + error: extractErrorMessage(e), message: i18n.translate( 'xpack.ml.dataframe.analytics.create.errorGettingDataFrameAnalyticsList', { @@ -225,7 +225,7 @@ export const useCreateAnalyticsForm = (): CreateAnalyticsFormProps => { }); } catch (e) { addRequestMessage({ - error: getErrorMessage(e), + error: extractErrorMessage(e), message: i18n.translate( 'xpack.ml.dataframe.analytics.create.errorGettingIndexPatternTitles', { @@ -260,7 +260,7 @@ export const useCreateAnalyticsForm = (): CreateAnalyticsFormProps => { refresh(); } catch (e) { addRequestMessage({ - error: getErrorMessage(e), + error: extractErrorMessage(e), message: i18n.translate( 'xpack.ml.dataframe.analytics.create.errorStartingDataFrameAnalyticsJob', { diff --git a/x-pack/plugins/ml/public/application/jobs/jobs_list/components/edit_job_flyout/edit_utils.js b/x-pack/plugins/ml/public/application/jobs/jobs_list/components/edit_job_flyout/edit_utils.js index 5030c48a4e367..adcc576c5e356 100644 --- a/x-pack/plugins/ml/public/application/jobs/jobs_list/components/edit_job_flyout/edit_utils.js +++ b/x-pack/plugins/ml/public/application/jobs/jobs_list/components/edit_job_flyout/edit_utils.js @@ -6,9 +6,9 @@ import { difference } from 'lodash'; import { getNewJobLimits } from '../../../../services/ml_server_info'; -import { mlJobService } from '../../../../services/job_service'; import { processCreatedBy } from '../../../../../../common/util/job_utils'; import { getSavedObjectsClient } from '../../../../util/dependency_cache'; +import { ml } from '../../../../services/ml_api_service'; export function saveJob(job, newJobData, finish) { return new Promise((resolve, reject) => { @@ -41,14 +41,9 @@ export function saveJob(job, newJobData, finish) { // if anything has changed, post the changes if (Object.keys(jobData).length) { - mlJobService - .updateJob(job.job_id, jobData) - .then((resp) => { - if (resp.success) { - saveDatafeedWrapper(); - } else { - reject(resp); - } + ml.updateJob({ jobId: job.job_id, job: jobData }) + .then(() => { + saveDatafeedWrapper(); }) .catch((error) => { reject(error); @@ -59,17 +54,17 @@ export function saveJob(job, newJobData, finish) { }); } -function saveDatafeed(datafeedData, job) { +function saveDatafeed(datafeedConfig, job) { return new Promise((resolve, reject) => { - if (Object.keys(datafeedData).length) { + if (Object.keys(datafeedConfig).length) { const datafeedId = job.datafeed_config.datafeed_id; - mlJobService.updateDatafeed(datafeedId, datafeedData).then((resp) => { - if (resp.success) { + ml.updateDatafeed({ datafeedId, datafeedConfig }) + .then(() => { resolve(); - } else { - reject(resp); - } - }); + }) + .catch((error) => { + reject(error); + }); } else { resolve(); } diff --git a/x-pack/plugins/ml/public/application/jobs/jobs_list/components/utils.js b/x-pack/plugins/ml/public/application/jobs/jobs_list/components/utils.js index 913727bda67df..f71f22f9e8839 100644 --- a/x-pack/plugins/ml/public/application/jobs/jobs_list/components/utils.js +++ b/x-pack/plugins/ml/public/application/jobs/jobs_list/components/utils.js @@ -5,18 +5,19 @@ */ import { each } from 'lodash'; -import { mlMessageBarService } from '../../../components/messagebar'; +import { i18n } from '@kbn/i18n'; import rison from 'rison-node'; +import { mlMessageBarService } from '../../../components/messagebar'; import { mlJobService } from '../../../services/job_service'; import { toastNotificationServiceProvider } from '../../../services/toast_notification_service'; -import { ml } from '../../../services/ml_api_service'; import { getToastNotifications } from '../../../util/dependency_cache'; +import { ml } from '../../../services/ml_api_service'; import { stringMatch } from '../../../util/string_utils'; import { JOB_STATE, DATAFEED_STATE } from '../../../../../common/constants/states'; import { parseInterval } from '../../../../../common/util/parse_interval'; -import { i18n } from '@kbn/i18n'; import { mlCalendarService } from '../../../services/calendar_service'; +import { extractErrorMessage } from '../../../../../common/util/errors'; export function loadFullJob(jobId) { return new Promise((resolve, reject) => { @@ -176,7 +177,7 @@ function showResults(resp, action) { export async function cloneJob(jobId) { try { - const job = await loadFullJob(jobId); + const job = await loadFullJob(jobId + 4); if (job.custom_settings && job.custom_settings.created_by) { // if the job is from a wizards, i.e. contains a created_by property // use tempJobCloningObjects to temporarily store the job @@ -219,13 +220,14 @@ export async function cloneJob(jobId) { window.location.href = '#/jobs/new_job'; } catch (error) { - mlMessageBarService.notify.error(error); + mlMessageBarService.notify.error(extractErrorMessage(error)); const toastNotifications = getToastNotifications(); toastNotifications.addDanger( i18n.translate('xpack.ml.jobsList.cloneJobErrorMessage', { defaultMessage: 'Could not clone {jobId}. Job could not be found', values: { jobId }, - }) + }), + error ); } } diff --git a/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/summary_step/components/post_save_options/post_save_options.tsx b/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/summary_step/components/post_save_options/post_save_options.tsx index 2e7cc9c413a25..82a023cd1779b 100644 --- a/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/summary_step/components/post_save_options/post_save_options.tsx +++ b/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/summary_step/components/post_save_options/post_save_options.tsx @@ -10,7 +10,7 @@ import { i18n } from '@kbn/i18n'; import { FormattedMessage } from '@kbn/i18n/react'; import { JobRunner } from '../../../../../common/job_runner'; import { useMlKibana } from '../../../../../../../contexts/kibana'; -import { getErrorMessage } from '../../../../../../../../../common/util/errors'; +import { extractErrorMessage } from '../../../../../../../../../common/util/errors'; // @ts-ignore import { CreateWatchFlyout } from '../../../../../../jobs_list/components/create_watch_flyout/index'; @@ -70,7 +70,7 @@ export const PostSaveOptions: FC = ({ jobRunner }) => { defaultMessage: `Error starting job`, } ), - text: getErrorMessage(error), + text: extractErrorMessage(error), }); } } diff --git a/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/summary_step/summary.tsx b/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/summary_step/summary.tsx index 24d7fb9fc2a40..78c8072261b30 100644 --- a/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/summary_step/summary.tsx +++ b/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/summary_step/summary.tsx @@ -22,7 +22,7 @@ import { JobCreatorContext } from '../job_creator_context'; import { JobRunner } from '../../../common/job_runner'; import { mlJobService } from '../../../../../services/job_service'; import { JsonEditorFlyout, EDITOR_MODE } from '../common/json_editor_flyout'; -import { getErrorMessage } from '../../../../../../../common/util/errors'; +import { extractErrorMessage } from '../../../../../../../common/util/errors'; import { isSingleMetricJobCreator, isAdvancedJobCreator } from '../../../common/job_creator'; import { JobDetails } from './components/job_details'; import { DatafeedDetails } from './components/datafeed_details'; @@ -78,7 +78,7 @@ export const SummaryStep: FC = ({ setCurrentStep, isCurrentStep }) => title: i18n.translate('xpack.ml.newJob.wizard.summaryStep.createJobError', { defaultMessage: `Job creation error`, }), - text: getErrorMessage(error), + text: extractErrorMessage(error), }); setCreatingJob(false); } @@ -97,7 +97,7 @@ export const SummaryStep: FC = ({ setCurrentStep, isCurrentStep }) => title: i18n.translate('xpack.ml.newJob.wizard.summaryStep.createJobError', { defaultMessage: `Job creation error`, }), - text: getErrorMessage(error), + text: extractErrorMessage(error), }); setCreatingJob(false); } diff --git a/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/validation_step/validation.tsx b/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/validation_step/validation.tsx index 19b89ffec02ac..3bde32f40eeb5 100644 --- a/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/validation_step/validation.tsx +++ b/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/validation_step/validation.tsx @@ -8,7 +8,7 @@ import React, { Fragment, FC, useContext, useState, useEffect } from 'react'; import { WizardNav } from '../wizard_nav'; import { WIZARD_STEPS, StepProps } from '../step_types'; import { JobCreatorContext } from '../job_creator_context'; -import { mlJobService } from '../../../../../services/job_service'; +import { ml } from '../../../../../services/ml_api_service'; import { ValidateJob } from '../../../../../components/validate_job'; import { JOB_TYPE } from '../../../../../../../common/constants/new_job'; @@ -66,7 +66,7 @@ export const ValidationStep: FC = ({ setCurrentStep, isCurrentStep }) { - return { success: true }; - }) - .catch((err) => { - // TODO - all the functions in here should just return the error and not - // display the toast, as currently both the component and this service display - // errors, so we end up with duplicate toasts. - const toastNotifications = getToastNotifications(); - const toastNotificationService = toastNotificationServiceProvider(toastNotifications); - toastNotificationService.displayErrorToast( - err, - i18n.translate('xpack.ml.jobService.updateJobErrorTitle', { - defaultMessage: 'Could not update job: {jobId}', - values: { jobId }, - }) - ); - - console.error('update job', err); - return { success: false, message: err }; - }); - } - - validateJob(obj) { - // return the promise chain - return ml - .validateJob(obj) - .then((messages) => { - return { success: true, messages }; - }) - .catch((err) => { - const toastNotifications = getToastNotifications(); - const toastNotificationService = toastNotificationServiceProvider(toastNotifications); - toastNotificationService.displayErrorToast( - err, - i18n.translate('xpack.ml.jobService.validateJobErrorTitle', { - defaultMessage: 'Job Validation Error', - }) - ); - - console.log('validate job', err); - return { - success: false, - messages: [ - { - status: 'error', - text: err.message, - }, - ], - }; - }); - } - // find a job based on the id getJob(jobId) { const job = find(jobs, (j) => { @@ -638,25 +580,6 @@ class JobService { }); } - updateDatafeed(datafeedId, datafeedConfig) { - return ml - .updateDatafeed({ datafeedId, datafeedConfig }) - .then((resp) => { - console.log('update datafeed', resp); - return { success: true }; - }) - .catch((err) => { - msgs.notify.error( - i18n.translate('xpack.ml.jobService.couldNotUpdateDatafeedErrorMessage', { - defaultMessage: 'Could not update datafeed: {datafeedId}', - values: { datafeedId }, - }) - ); - console.log('update datafeed', err); - return { success: false, message: err.message }; - }); - } - // start the datafeed for a given job // refresh the job state on start success startDatafeed(datafeedId, jobId, start, end) { diff --git a/x-pack/plugins/ml/public/application/settings/calendars/list/delete_calendars.js b/x-pack/plugins/ml/public/application/settings/calendars/list/delete_calendars.js index 50777485903d2..e0c7a4db6e898 100644 --- a/x-pack/plugins/ml/public/application/settings/calendars/list/delete_calendars.js +++ b/x-pack/plugins/ml/public/application/settings/calendars/list/delete_calendars.js @@ -7,7 +7,7 @@ import { getToastNotifications } from '../../../util/dependency_cache'; import { ml } from '../../../services/ml_api_service'; import { i18n } from '@kbn/i18n'; -import { getErrorMessage } from '../../../../../common/util/errors'; +import { extractErrorMessage } from '../../../../../common/util/errors'; export async function deleteCalendars(calendarsToDelete, callback) { if (calendarsToDelete === undefined || calendarsToDelete.length === 0) { @@ -47,7 +47,7 @@ export async function deleteCalendars(calendarsToDelete, callback) { }, } ), - text: getErrorMessage(error), + text: extractErrorMessage(error), }); } } From a100bca05d44ff9639a268c20eaf641d3ea1d62e Mon Sep 17 00:00:00 2001 From: James Gowdy Date: Mon, 7 Sep 2020 12:53:05 +0100 Subject: [PATCH 02/17] adding stacktrace to request errors --- .../anomaly_detection_jobs/combined_job.ts | 2 +- .../annotations/annotation_flyout/index.tsx | 2 +- .../messagebar/messagebar_service.d.ts | 4 +- .../messagebar/messagebar_service.js | 13 ++- .../group_selector/group_selector.js | 4 +- .../jobs/jobs_list/components/utils.js | 11 ++- .../estimate_bucket_span.ts | 2 +- .../categorization_view/metric_selection.tsx | 2 +- .../multi_metric_view/metric_selection.tsx | 4 +- .../metric_selection_summary.tsx | 4 +- .../population_view/metric_selection.tsx | 2 +- .../metric_selection_summary.tsx | 2 +- .../single_metric_view/metric_selection.tsx | 2 +- .../metric_selection_summary.tsx | 2 +- .../application/services/job_service.js | 89 +++---------------- .../ml/public/application/util/ml_error.ts | 4 + .../plugins/ml/server/routes/job_service.ts | 1 - 17 files changed, 41 insertions(+), 109 deletions(-) diff --git a/x-pack/plugins/ml/common/types/anomaly_detection_jobs/combined_job.ts b/x-pack/plugins/ml/common/types/anomaly_detection_jobs/combined_job.ts index 42dee46e71fd6..e7b10ff7c0422 100644 --- a/x-pack/plugins/ml/common/types/anomaly_detection_jobs/combined_job.ts +++ b/x-pack/plugins/ml/common/types/anomaly_detection_jobs/combined_job.ts @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import { cloneDeep } from 'lodash'; +import cloneDeep from 'lodash/cloneDeep'; import { Datafeed } from './datafeed'; import { DatafeedStats } from './datafeed_stats'; import { Job } from './job'; diff --git a/x-pack/plugins/ml/public/application/components/annotations/annotation_flyout/index.tsx b/x-pack/plugins/ml/public/application/components/annotations/annotation_flyout/index.tsx index 84abe3ed8a821..67d03beced5f0 100644 --- a/x-pack/plugins/ml/public/application/components/annotations/annotation_flyout/index.tsx +++ b/x-pack/plugins/ml/public/application/components/annotations/annotation_flyout/index.tsx @@ -7,7 +7,7 @@ import React, { Component, FC, ReactNode, useCallback } from 'react'; import useObservable from 'react-use/lib/useObservable'; import * as Rx from 'rxjs'; -import { cloneDeep } from 'lodash'; +import cloneDeep from 'lodash/cloneDeep'; import { EuiButton, diff --git a/x-pack/plugins/ml/public/application/components/messagebar/messagebar_service.d.ts b/x-pack/plugins/ml/public/application/components/messagebar/messagebar_service.d.ts index 29a537a7ca8d8..ef5088e281319 100644 --- a/x-pack/plugins/ml/public/application/components/messagebar/messagebar_service.d.ts +++ b/x-pack/plugins/ml/public/application/components/messagebar/messagebar_service.d.ts @@ -5,9 +5,7 @@ */ declare interface MlMessageBarService { - notify: { - error(text: any, resp?: any): void; - }; + error(text: any, resp?: any): void; } export const mlMessageBarService: MlMessageBarService; diff --git a/x-pack/plugins/ml/public/application/components/messagebar/messagebar_service.js b/x-pack/plugins/ml/public/application/components/messagebar/messagebar_service.js index 1dc7140bd2687..7220f8f7b4ba6 100644 --- a/x-pack/plugins/ml/public/application/components/messagebar/messagebar_service.js +++ b/x-pack/plugins/ml/public/application/components/messagebar/messagebar_service.js @@ -4,14 +4,15 @@ * you may not use this file except in compliance with the Elastic License. */ +import { i18n } from '@kbn/i18n'; import { getToastNotifications } from '../../util/dependency_cache'; import { MLRequestFailure } from '../../util/ml_error'; -import { i18n } from '@kbn/i18n'; +import { extractErrorMessage } from '../../../../common/util/errors'; function errorNotify(text, resp) { let err = null; - if (typeof text === 'object' && text.response !== undefined) { - resp = text.response; + if (typeof text === 'object') { + err = new Error(extractErrorMessage(text)); } else if (typeof text === 'object' && text.message !== undefined) { err = new Error(text.message); } else { @@ -19,7 +20,7 @@ function errorNotify(text, resp) { } const toastNotifications = getToastNotifications(); - toastNotifications.addError(new MLRequestFailure(err, resp), { + toastNotifications.addError(new MLRequestFailure(err, resp ?? text), { title: i18n.translate('xpack.ml.messagebarService.errorTitle', { defaultMessage: 'An error has occurred', }), @@ -27,7 +28,5 @@ function errorNotify(text, resp) { } export const mlMessageBarService = { - notify: { - error: errorNotify, - }, + error: errorNotify, }; diff --git a/x-pack/plugins/ml/public/application/jobs/jobs_list/components/multi_job_actions/group_selector/group_selector.js b/x-pack/plugins/ml/public/application/jobs/jobs_list/components/multi_job_actions/group_selector/group_selector.js index f73dde69a3d4c..6eacd986d5da2 100644 --- a/x-pack/plugins/ml/public/application/jobs/jobs_list/components/multi_job_actions/group_selector/group_selector.js +++ b/x-pack/plugins/ml/public/application/jobs/jobs_list/components/multi_job_actions/group_selector/group_selector.js @@ -160,7 +160,7 @@ export class GroupSelector extends Component { // check success of each job update if (resp.hasOwnProperty(jobId)) { if (resp[jobId].success === false) { - mlMessageBarService.notify.error(resp[jobId].error); + mlMessageBarService.error(resp[jobId].error); success = false; } } @@ -175,7 +175,7 @@ export class GroupSelector extends Component { } }) .catch((error) => { - mlMessageBarService.notify.error(error); + mlMessageBarService.error(error); console.error(error); }); }; diff --git a/x-pack/plugins/ml/public/application/jobs/jobs_list/components/utils.js b/x-pack/plugins/ml/public/application/jobs/jobs_list/components/utils.js index f71f22f9e8839..b8461ecbea9b0 100644 --- a/x-pack/plugins/ml/public/application/jobs/jobs_list/components/utils.js +++ b/x-pack/plugins/ml/public/application/jobs/jobs_list/components/utils.js @@ -17,7 +17,6 @@ import { stringMatch } from '../../../util/string_utils'; import { JOB_STATE, DATAFEED_STATE } from '../../../../../common/constants/states'; import { parseInterval } from '../../../../../common/util/parse_interval'; import { mlCalendarService } from '../../../services/calendar_service'; -import { extractErrorMessage } from '../../../../../common/util/errors'; export function loadFullJob(jobId) { return new Promise((resolve, reject) => { @@ -61,7 +60,7 @@ export function forceStartDatafeeds(jobs, start, end, finish = () => {}) { finish(); }) .catch((error) => { - mlMessageBarService.notify.error(error); + mlMessageBarService.error(error); const toastNotifications = getToastNotifications(); toastNotifications.addDanger( i18n.translate('xpack.ml.jobsList.startJobErrorMessage', { @@ -82,7 +81,7 @@ export function stopDatafeeds(jobs, finish = () => {}) { finish(); }) .catch((error) => { - mlMessageBarService.notify.error(error); + mlMessageBarService.error(error); const toastNotifications = getToastNotifications(); toastNotifications.addDanger( i18n.translate('xpack.ml.jobsList.stopJobErrorMessage', { @@ -220,7 +219,7 @@ export async function cloneJob(jobId) { window.location.href = '#/jobs/new_job'; } catch (error) { - mlMessageBarService.notify.error(extractErrorMessage(error)); + mlMessageBarService.error(error); const toastNotifications = getToastNotifications(); toastNotifications.addDanger( i18n.translate('xpack.ml.jobsList.cloneJobErrorMessage', { @@ -241,7 +240,7 @@ export function closeJobs(jobs, finish = () => {}) { finish(); }) .catch((error) => { - mlMessageBarService.notify.error(error); + mlMessageBarService.error(error); const toastNotifications = getToastNotifications(); toastNotifications.addDanger( i18n.translate('xpack.ml.jobsList.closeJobErrorMessage', { @@ -262,7 +261,7 @@ export function deleteJobs(jobs, finish = () => {}) { finish(); }) .catch((error) => { - mlMessageBarService.notify.error(error); + mlMessageBarService.error(error); const toastNotifications = getToastNotifications(); toastNotifications.addDanger( i18n.translate('xpack.ml.jobsList.deleteJobErrorMessage', { diff --git a/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/bucket_span_estimator/estimate_bucket_span.ts b/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/bucket_span_estimator/estimate_bucket_span.ts index 0ec3b609b604f..91597414fd3fd 100644 --- a/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/bucket_span_estimator/estimate_bucket_span.ts +++ b/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/bucket_span_estimator/estimate_bucket_span.ts @@ -68,7 +68,7 @@ export function useEstimateBucketSpan() { const { name, error, message } = await ml.estimateBucketSpan(data); setStatus(ESTIMATE_STATUS.NOT_RUNNING); if (error === true) { - mlMessageBarService.notify.error(message); + mlMessageBarService.error(message); } else { jobCreator.bucketSpan = name; jobCreatorUpdate(); diff --git a/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/categorization_view/metric_selection.tsx b/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/categorization_view/metric_selection.tsx index cbbddb5bbc5b8..6750cf0c7e9f4 100644 --- a/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/categorization_view/metric_selection.tsx +++ b/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/categorization_view/metric_selection.tsx @@ -94,7 +94,7 @@ export const CategorizationDetectors: FC = ({ setIsValid }) => { setFieldExamples(null); setValidationChecks([]); setOverallValidStatus(CATEGORY_EXAMPLES_VALIDATION_STATUS.INVALID); - mlMessageBarService.notify.error(error); + mlMessageBarService.error(error); } } else { setFieldExamples(null); diff --git a/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/multi_metric_view/metric_selection.tsx b/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/multi_metric_view/metric_selection.tsx index 684cb5b4e0dda..10a2a83179c0e 100644 --- a/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/multi_metric_view/metric_selection.tsx +++ b/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/multi_metric_view/metric_selection.tsx @@ -109,7 +109,7 @@ export const MultiMetricDetectors: FC = ({ setIsValid }) => { .loadFieldExampleValues(splitField) .then(setFieldValues) .catch((error) => { - mlMessageBarService.notify.error(error); + mlMessageBarService.error(error); }); } else { setFieldValues([]); @@ -138,7 +138,7 @@ export const MultiMetricDetectors: FC = ({ setIsValid }) => { ); setLineChartsData(resp); } catch (error) { - mlMessageBarService.notify.error(error); + mlMessageBarService.error(error); setLineChartsData([]); } setLoadingData(false); diff --git a/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/multi_metric_view/metric_selection_summary.tsx b/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/multi_metric_view/metric_selection_summary.tsx index f39a316440e74..a84e3cec9a0f6 100644 --- a/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/multi_metric_view/metric_selection_summary.tsx +++ b/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/multi_metric_view/metric_selection_summary.tsx @@ -43,7 +43,7 @@ export const MultiMetricDetectorsSummary: FC = () => { const tempFieldValues = await chartLoader.loadFieldExampleValues(jobCreator.splitField); setFieldValues(tempFieldValues); } catch (error) { - mlMessageBarService.notify.error(error); + mlMessageBarService.error(error); } } })(); @@ -75,7 +75,7 @@ export const MultiMetricDetectorsSummary: FC = () => { ); setLineChartsData(resp); } catch (error) { - mlMessageBarService.notify.error(error); + mlMessageBarService.error(error); setLineChartsData({}); } setLoadingData(false); diff --git a/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/population_view/metric_selection.tsx b/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/population_view/metric_selection.tsx index e5f5ba48900d9..a22ca0a71f079 100644 --- a/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/population_view/metric_selection.tsx +++ b/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/population_view/metric_selection.tsx @@ -159,7 +159,7 @@ export const PopulationDetectors: FC = ({ setIsValid }) => { setLineChartsData(resp); } catch (error) { - mlMessageBarService.notify.error(error); + mlMessageBarService.error(error); setLineChartsData([]); } setLoadingData(false); diff --git a/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/population_view/metric_selection_summary.tsx b/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/population_view/metric_selection_summary.tsx index 06f7092e8ac06..ef3131132e81a 100644 --- a/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/population_view/metric_selection_summary.tsx +++ b/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/population_view/metric_selection_summary.tsx @@ -81,7 +81,7 @@ export const PopulationDetectorsSummary: FC = () => { setLineChartsData(resp); } catch (error) { - mlMessageBarService.notify.error(error); + mlMessageBarService.error(error); setLineChartsData({}); } setLoadingData(false); diff --git a/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/single_metric_view/metric_selection.tsx b/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/single_metric_view/metric_selection.tsx index f04b63f47789e..bbd96a6bdd0f6 100644 --- a/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/single_metric_view/metric_selection.tsx +++ b/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/single_metric_view/metric_selection.tsx @@ -93,7 +93,7 @@ export const SingleMetricDetectors: FC = ({ setIsValid }) => { setLineChartData(resp); } } catch (error) { - mlMessageBarService.notify.error(error); + mlMessageBarService.error(error); setLineChartData({}); } setLoadingData(false); diff --git a/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/single_metric_view/metric_selection_summary.tsx b/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/single_metric_view/metric_selection_summary.tsx index 85fb5890307ba..ebe915b309f15 100644 --- a/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/single_metric_view/metric_selection_summary.tsx +++ b/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/single_metric_view/metric_selection_summary.tsx @@ -63,7 +63,7 @@ export const SingleMetricDetectorsSummary: FC = () => { setLineChartData(resp); } } catch (error) { - mlMessageBarService.notify.error(error); + mlMessageBarService.error(error); setLineChartData({}); } setLoadingData(false); diff --git a/x-pack/plugins/ml/public/application/services/job_service.js b/x-pack/plugins/ml/public/application/services/job_service.js index 9975fe629adab..7dd4def638e90 100644 --- a/x-pack/plugins/ml/public/application/services/job_service.js +++ b/x-pack/plugins/ml/public/application/services/job_service.js @@ -20,7 +20,6 @@ import { ML_DATA_PREVIEW_COUNT } from '../../../common/util/job_utils'; import { TIME_FORMAT } from '../../../common/constants/time_format'; import { parseInterval } from '../../../common/util/parse_interval'; -const msgs = mlMessageBarService; let jobs = []; let datafeedIds = {}; @@ -117,7 +116,7 @@ class JobService { return new Promise((resolve, reject) => { jobs = []; datafeedIds = {}; - + console.log(12222); ml.getJobs() .then((resp) => { jobs = resp.jobs; @@ -160,7 +159,6 @@ class JobService { } processBasicJobInfo(this, jobs); this.jobs = jobs; - createJobStats(this.jobs, this.jobStats); resolve({ jobs: this.jobs }); }); }) @@ -174,12 +172,7 @@ class JobService { function error(err) { console.log('jobService error getting list of jobs:', err); - msgs.notify.error( - i18n.translate('xpack.ml.jobService.jobsListCouldNotBeRetrievedErrorMessage', { - defaultMessage: 'Jobs list could not be retrieved', - }) - ); - msgs.notify.error('', err); + mlMessageBarService.error(err); reject({ jobs, err }); } }); @@ -246,7 +239,6 @@ class JobService { } } this.jobs = jobs; - createJobStats(this.jobs, this.jobStats); resolve({ jobs: this.jobs }); }); }) @@ -261,12 +253,7 @@ class JobService { function error(err) { console.log('JobService error getting list of jobs:', err); - msgs.notify.error( - i18n.translate('xpack.ml.jobService.jobsListCouldNotBeRetrievedErrorMessage', { - defaultMessage: 'Jobs list could not be retrieved', - }) - ); - msgs.notify.error('', err); + mlMessageBarService.error(err); reject({ jobs, err }); } }); @@ -307,12 +294,7 @@ class JobService { function error(err) { console.log('loadDatafeeds error getting list of datafeeds:', err); - msgs.notify.error( - i18n.translate('xpack.ml.jobService.datafeedsListCouldNotBeRetrievedErrorMessage', { - defaultMessage: 'datafeeds list could not be retrieved', - }) - ); - msgs.notify.error('', err); + mlMessageBarService.error(err); reject({ jobs, err }); } }); @@ -591,7 +573,7 @@ class JobService { } ml.startDatafeed({ - datafeedId, + datafeedId: datafeedId + 22, start, end, }) @@ -600,7 +582,7 @@ class JobService { }) .catch((err) => { console.log('jobService error starting datafeed:', err); - msgs.notify.error( + mlMessageBarService.error( i18n.translate('xpack.ml.jobService.couldNotStartDatafeedErrorMessage', { defaultMessage: 'Could not start datafeed for {jobId}', values: { jobId }, @@ -624,24 +606,20 @@ class JobService { }) .catch((err) => { console.log('jobService error stopping datafeed:', err); - const couldNotStopDatafeedErrorMessage = i18n.translate( - 'xpack.ml.jobService.couldNotStopDatafeedErrorMessage', - { + mlMessageBarService.error( + i18n.translate('xpack.ml.jobService.couldNotStopDatafeedErrorMessage', { defaultMessage: 'Could not stop datafeed for {jobId}', values: { jobId }, - } + }), + err ); - if (err.statusCode === 500) { - msgs.notify.error(couldNotStopDatafeedErrorMessage); - msgs.notify.error( + mlMessageBarService.error( i18n.translate('xpack.ml.jobService.requestMayHaveTimedOutErrorMessage', { defaultMessage: 'Request may have timed out and may still be running in the background.', }) ); - } else { - msgs.notify.error(couldNotStopDatafeedErrorMessage, err); } reject(err); }); @@ -810,51 +788,6 @@ function processBasicJobInfo(localJobService, jobsList) { return processedJobsList; } -// Loop through the jobs list and create basic stats -// stats are displayed along the top of the Jobs Management page -function createJobStats(jobsList, jobStats) { - jobStats.activeNodes.value = 0; - jobStats.total.value = 0; - jobStats.open.value = 0; - jobStats.closed.value = 0; - jobStats.failed.value = 0; - jobStats.activeDatafeeds.value = 0; - - // object to keep track of nodes being used by jobs - const mlNodes = {}; - let failedJobs = 0; - - each(jobsList, (job) => { - if (job.state === 'opened') { - jobStats.open.value++; - } else if (job.state === 'closed') { - jobStats.closed.value++; - } else if (job.state === 'failed') { - failedJobs++; - } - - if (job.datafeed_config && job.datafeed_config.state === 'started') { - jobStats.activeDatafeeds.value++; - } - - if (job.node && job.node.name) { - mlNodes[job.node.name] = {}; - } - }); - - jobStats.total.value = jobsList.length; - - // // Only show failed jobs if it is non-zero - if (failedJobs) { - jobStats.failed.value = failedJobs; - jobStats.failed.show = true; - } else { - jobStats.failed.show = false; - } - - jobStats.activeNodes.value = Object.keys(mlNodes).length; -} - function createResultsUrlForJobs(jobsList, resultsPage) { let from = undefined; let to = undefined; diff --git a/x-pack/plugins/ml/public/application/util/ml_error.ts b/x-pack/plugins/ml/public/application/util/ml_error.ts index 2a0280404c189..f72b04ab12a7d 100644 --- a/x-pack/plugins/ml/public/application/util/ml_error.ts +++ b/x-pack/plugins/ml/public/application/util/ml_error.ts @@ -9,6 +9,7 @@ import { KbnError } from '../../../../../../src/plugins/kibana_utils/public'; export class MLRequestFailure extends KbnError { origError: any; resp: any; + stack: any; // takes an Error object and and optional response object // if error is falsy (null) the response object will be used // notify will show the full expandable stack trace of the response if a response object is used and no error is passed in. @@ -18,5 +19,8 @@ export class MLRequestFailure extends KbnError { this.origError = error; this.resp = typeof resp === 'string' ? JSON.parse(resp) : resp; + if (resp.body) { + this.stack = JSON.stringify(resp.body, null, 2); + } } } diff --git a/x-pack/plugins/ml/server/routes/job_service.ts b/x-pack/plugins/ml/server/routes/job_service.ts index 3c7f35b871b10..2a75fac2d125e 100644 --- a/x-pack/plugins/ml/server/routes/job_service.ts +++ b/x-pack/plugins/ml/server/routes/job_service.ts @@ -498,7 +498,6 @@ export function jobServiceRoutes({ router, mlLicense }: RouteInitialization) { splitFieldName, splitFieldValue, } = request.body; - const { newJobLineChart } = jobServiceProvider(client); const resp = await newJobLineChart( indexPatternTitle, From 25f6f6883b4d425c58eb4809a5c4df02bcd02078 Mon Sep 17 00:00:00 2001 From: James Gowdy Date: Mon, 7 Sep 2020 13:50:39 +0100 Subject: [PATCH 03/17] copying error parsing to transforms --- .../use_create_analytics_form.test.tsx | 23 -------------- .../application/services/job_service.js | 3 -- .../plugins/transform/common/utils/errors.ts | 31 +++++++++++++++++++ .../public/__mocks__/shared_imports.ts | 1 - .../public/app/hooks/use_delete_transform.tsx | 3 +- .../public/app/hooks/use_index_data.ts | 2 +- .../public/app/hooks/use_pivot_data.ts | 2 +- .../step_create/step_create_form.tsx | 2 +- .../step_details/step_details_form.tsx | 2 +- .../edit_transform_flyout.tsx | 2 +- .../transform/public/shared_imports.ts | 1 - 11 files changed, 38 insertions(+), 34 deletions(-) create mode 100644 x-pack/plugins/transform/common/utils/errors.ts 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 3fe0f9e92cb30..f833cf4708cec 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 @@ -11,7 +11,6 @@ import { MlContext } from '../../../../../contexts/ml'; import { kibanaContextValueMock } from '../../../../../contexts/ml/__mocks__/kibana_context_value'; import { useCreateAnalyticsForm } from './use_create_analytics_form'; -import { extractErrorMessage } from '../../../../../../../common/util/errors'; const getMountedHook = () => mountHook( @@ -21,28 +20,6 @@ const getMountedHook = () => ) ); -describe('extractErrorMessage()', () => { - test('verify error message response formats', () => { - const customError1 = { - body: { statusCode: 403, error: 'Forbidden', message: 'the-error-message' }, - }; - const errorMessage1 = extractErrorMessage(customError1); - expect(errorMessage1).toBe('Forbidden: the-error-message'); - - const customError2 = new Error('the-error-message'); - const errorMessage2 = extractErrorMessage(customError2); - expect(errorMessage2).toBe('the-error-message'); - - const customError3 = { customErrorMessage: 'the-error-message' }; - const errorMessage3 = extractErrorMessage(customError3); - expect(errorMessage3).toBe('{"customErrorMessage":"the-error-message"}'); - - const customError4 = { message: 'the-error-message' }; - const errorMessage4 = extractErrorMessage(customError4); - expect(errorMessage4).toBe('the-error-message'); - }); -}); - describe('useCreateAnalyticsForm()', () => { test('initialization', () => { const { getLastHookValue } = getMountedHook(); diff --git a/x-pack/plugins/ml/public/application/services/job_service.js b/x-pack/plugins/ml/public/application/services/job_service.js index 409d2b3974eba..324bd16581128 100644 --- a/x-pack/plugins/ml/public/application/services/job_service.js +++ b/x-pack/plugins/ml/public/application/services/job_service.js @@ -19,10 +19,7 @@ import { isWebUrl } from '../util/url_utils'; import { ML_DATA_PREVIEW_COUNT } from '../../../common/util/job_utils'; import { TIME_FORMAT } from '../../../common/constants/time_format'; import { parseInterval } from '../../../common/util/parse_interval'; - -import { toastNotificationServiceProvider } from '../services/toast_notification_service'; import { validateTimeRange } from '../util/date_utils'; -const msgs = mlMessageBarService; let jobs = []; let datafeedIds = {}; diff --git a/x-pack/plugins/transform/common/utils/errors.ts b/x-pack/plugins/transform/common/utils/errors.ts new file mode 100644 index 0000000000000..0c31d7e1584f0 --- /dev/null +++ b/x-pack/plugins/transform/common/utils/errors.ts @@ -0,0 +1,31 @@ +/* + * 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; + attributes?: any; + }; + name: string; +} + +export function 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}`; + } + + if (typeof error === 'object' && typeof error.message === 'string') { + return error.message; + } + + return JSON.stringify(error); +} diff --git a/x-pack/plugins/transform/public/__mocks__/shared_imports.ts b/x-pack/plugins/transform/public/__mocks__/shared_imports.ts index e115e086f45b5..f7441fd93f38a 100644 --- a/x-pack/plugins/transform/public/__mocks__/shared_imports.ts +++ b/x-pack/plugins/transform/public/__mocks__/shared_imports.ts @@ -15,7 +15,6 @@ export const useRequest = jest.fn(() => ({ // just passing through the reimports export { - getErrorMessage, getDataGridSchemaFromKibanaFieldType, getFieldsFromKibanaIndexPattern, multiColumnSortFactory, diff --git a/x-pack/plugins/transform/public/app/hooks/use_delete_transform.tsx b/x-pack/plugins/transform/public/app/hooks/use_delete_transform.tsx index 43c5ae6fad1b1..fdf77c8ebee51 100644 --- a/x-pack/plugins/transform/public/app/hooks/use_delete_transform.tsx +++ b/x-pack/plugins/transform/public/app/hooks/use_delete_transform.tsx @@ -12,7 +12,8 @@ import { DeleteTransformStatus, TransformEndpointRequest, } from '../../../common'; -import { extractErrorMessage, getErrorMessage } from '../../shared_imports'; +import { extractErrorMessage } from '../../shared_imports'; +import { getErrorMessage } from '../../../common/utils/errors'; import { useAppDependencies, useToastNotifications } from '../app_dependencies'; import { REFRESH_TRANSFORM_LIST_STATE, refreshTransformList$, TransformListRow } from '../common'; import { ToastNotificationText } from '../components'; diff --git a/x-pack/plugins/transform/public/app/hooks/use_index_data.ts b/x-pack/plugins/transform/public/app/hooks/use_index_data.ts index ad5850f26be2e..946f7991d049d 100644 --- a/x-pack/plugins/transform/public/app/hooks/use_index_data.ts +++ b/x-pack/plugins/transform/public/app/hooks/use_index_data.ts @@ -12,7 +12,6 @@ import { getFieldType, getDataGridSchemaFromKibanaFieldType, getFieldsFromKibanaIndexPattern, - getErrorMessage, showDataGridColumnChartErrorMessageToast, useDataGrid, useRenderCellValue, @@ -21,6 +20,7 @@ import { UseIndexDataReturnType, INDEX_STATUS, } from '../../shared_imports'; +import { getErrorMessage } from '../../../common/utils/errors'; import { isDefaultQuery, matchAllQuery, PivotQuery } from '../common'; diff --git a/x-pack/plugins/transform/public/app/hooks/use_pivot_data.ts b/x-pack/plugins/transform/public/app/hooks/use_pivot_data.ts index a9f34996b9b51..a0e7c5dde494a 100644 --- a/x-pack/plugins/transform/public/app/hooks/use_pivot_data.ts +++ b/x-pack/plugins/transform/public/app/hooks/use_pivot_data.ts @@ -18,13 +18,13 @@ import { formatHumanReadableDateTimeSeconds } from '../../shared_imports'; import { getNestedProperty } from '../../../common/utils/object_utils'; import { - getErrorMessage, multiColumnSortFactory, useDataGrid, RenderCellValue, UseIndexDataReturnType, INDEX_STATUS, } from '../../shared_imports'; +import { getErrorMessage } from '../../../common/utils/errors'; import { getPreviewRequestBody, diff --git a/x-pack/plugins/transform/public/app/sections/create_transform/components/step_create/step_create_form.tsx b/x-pack/plugins/transform/public/app/sections/create_transform/components/step_create/step_create_form.tsx index 255a245081d5a..2fa1b7c713370 100644 --- a/x-pack/plugins/transform/public/app/sections/create_transform/components/step_create/step_create_form.tsx +++ b/x-pack/plugins/transform/public/app/sections/create_transform/components/step_create/step_create_form.tsx @@ -32,7 +32,7 @@ import { toMountPoint } from '../../../../../../../../../src/plugins/kibana_reac import { PROGRESS_REFRESH_INTERVAL_MS } from '../../../../../../common/constants'; -import { getErrorMessage } from '../../../../../shared_imports'; +import { getErrorMessage } from '../../../../../../common/utils/errors'; import { getTransformProgress, getDiscoverUrl } from '../../../../common'; import { useApi } from '../../../../hooks/use_api'; diff --git a/x-pack/plugins/transform/public/app/sections/create_transform/components/step_details/step_details_form.tsx b/x-pack/plugins/transform/public/app/sections/create_transform/components/step_details/step_details_form.tsx index 271fde27f519a..85f4065e8c069 100644 --- a/x-pack/plugins/transform/public/app/sections/create_transform/components/step_details/step_details_form.tsx +++ b/x-pack/plugins/transform/public/app/sections/create_transform/components/step_details/step_details_form.tsx @@ -16,7 +16,7 @@ import { toMountPoint } from '../../../../../../../../../src/plugins/kibana_reac import { TransformId } from '../../../../../../common'; import { isValidIndexName } from '../../../../../../common/utils/es_utils'; -import { getErrorMessage } from '../../../../../shared_imports'; +import { getErrorMessage } from '../../../../../../common/utils/errors'; import { useAppDependencies, useToastNotifications } from '../../../../app_dependencies'; import { ToastNotificationText } from '../../../../components'; diff --git a/x-pack/plugins/transform/public/app/sections/transform_management/components/edit_transform_flyout/edit_transform_flyout.tsx b/x-pack/plugins/transform/public/app/sections/transform_management/components/edit_transform_flyout/edit_transform_flyout.tsx index 77a7ae25ce887..735a059e57e14 100644 --- a/x-pack/plugins/transform/public/app/sections/transform_management/components/edit_transform_flyout/edit_transform_flyout.tsx +++ b/x-pack/plugins/transform/public/app/sections/transform_management/components/edit_transform_flyout/edit_transform_flyout.tsx @@ -23,7 +23,7 @@ import { EuiTitle, } from '@elastic/eui'; -import { getErrorMessage } from '../../../../../shared_imports'; +import { getErrorMessage } from '../../../../../../common/utils/errors'; import { refreshTransformList$, diff --git a/x-pack/plugins/transform/public/shared_imports.ts b/x-pack/plugins/transform/public/shared_imports.ts index abbc39dd6c728..196df250b7a3d 100644 --- a/x-pack/plugins/transform/public/shared_imports.ts +++ b/x-pack/plugins/transform/public/shared_imports.ts @@ -15,7 +15,6 @@ export { export { getFieldType, - getErrorMessage, extractErrorMessage, formatHumanReadableDateTimeSeconds, getDataGridSchemaFromKibanaFieldType, From 5a105ebc974dea685296a983e21c5b517f004767 Mon Sep 17 00:00:00 2001 From: James Gowdy Date: Mon, 7 Sep 2020 14:00:09 +0100 Subject: [PATCH 04/17] update snapshot --- .../validate_job_view.test.js.snap | 146 +++++++++++++++--- 1 file changed, 127 insertions(+), 19 deletions(-) diff --git a/x-pack/plugins/ml/public/application/components/validate_job/__snapshots__/validate_job_view.test.js.snap b/x-pack/plugins/ml/public/application/components/validate_job/__snapshots__/validate_job_view.test.js.snap index cbbf84b9e51ec..66490b4d336e0 100644 --- a/x-pack/plugins/ml/public/application/components/validate_job/__snapshots__/validate_job_view.test.js.snap +++ b/x-pack/plugins/ml/public/application/components/validate_job/__snapshots__/validate_job_view.test.js.snap @@ -79,25 +79,133 @@ exports[`ValidateJob renders button and modal with a message 1`] = ` `; exports[`ValidateJob renders the button 1`] = ` - -
- - - -
- + `; exports[`ValidateJob renders the button and modal with a success message 1`] = ` From 88c5a23d65d2d6b9a6eb4bb3ecad0e559a00f8ce Mon Sep 17 00:00:00 2001 From: James Gowdy Date: Mon, 7 Sep 2020 14:20:49 +0100 Subject: [PATCH 05/17] fixing jest tests --- .../validate_job_view.test.js.snap | 252 ++---------------- .../validate_job/validate_job_view.test.js | 17 +- 2 files changed, 35 insertions(+), 234 deletions(-) diff --git a/x-pack/plugins/ml/public/application/components/validate_job/__snapshots__/validate_job_view.test.js.snap b/x-pack/plugins/ml/public/application/components/validate_job/__snapshots__/validate_job_view.test.js.snap index 66490b4d336e0..f6a161cc01999 100644 --- a/x-pack/plugins/ml/public/application/components/validate_job/__snapshots__/validate_job_view.test.js.snap +++ b/x-pack/plugins/ml/public/application/components/validate_job/__snapshots__/validate_job_view.test.js.snap @@ -8,7 +8,7 @@ exports[`ValidateJob renders button and modal with a message 1`] = ` iconSide="right" iconType="questionInCircle" isDisabled={false} - isLoading={false} + isLoading={true} onClick={[Function]} size="s" > @@ -18,194 +18,30 @@ exports[`ValidateJob renders button and modal with a message 1`] = ` values={Object {}} /> - - } - > - - - - - - - - , - } - } - /> - - `; exports[`ValidateJob renders the button 1`] = ` - + +
+ + + +
+ `; exports[`ValidateJob renders the button and modal with a success message 1`] = ` @@ -216,7 +52,7 @@ exports[`ValidateJob renders the button and modal with a success message 1`] = ` iconSide="right" iconType="questionInCircle" isDisabled={false} - isLoading={false} + isLoading={true} onClick={[Function]} size="s" > @@ -226,52 +62,6 @@ exports[`ValidateJob renders the button and modal with a success message 1`] = ` values={Object {}} /> - - } - > - - - - - - - - , - } - } - /> - - `; diff --git a/x-pack/plugins/ml/public/application/components/validate_job/validate_job_view.test.js b/x-pack/plugins/ml/public/application/components/validate_job/validate_job_view.test.js index cc8a5abb4e9ab..280dbd76d5487 100644 --- a/x-pack/plugins/ml/public/application/components/validate_job/validate_job_view.test.js +++ b/x-pack/plugins/ml/public/application/components/validate_job/validate_job_view.test.js @@ -16,6 +16,12 @@ jest.mock('../../util/dependency_cache', () => ({ }), })); +jest.mock('../../../../../../../src/plugins/kibana_react/public', () => ({ + withKibana: (comp) => { + return comp; + }, +})); + const job = { job_id: 'test-id', }; @@ -25,11 +31,16 @@ const getJobConfig = () => job; function prepareTest(messages) { const p = Promise.resolve(messages); - const mlJobService = { - validateJob: () => p, + const ml = { + validateJob: () => Promise.resolve(messages), + }; + const kibana = { + services: { + notifications: { toasts: { addDanger: jest.fn() } }, + }, }; - const component = ; + const component = ; const wrapper = shallowWithIntl(component); From 339be769b8541a493d82fab7f9ded400da006917 Mon Sep 17 00:00:00 2001 From: James Gowdy Date: Mon, 7 Sep 2020 14:48:46 +0100 Subject: [PATCH 06/17] adding test and removing debug log output --- .../common/types/anomaly_detection_jobs/combined_job.ts | 2 +- x-pack/plugins/ml/common/util/errors.test.ts | 9 +++++++++ x-pack/plugins/ml/common/util/errors.ts | 1 - .../components/annotations/annotation_flyout/index.tsx | 2 +- .../application/jobs/jobs_list/components/utils.js | 2 +- .../ml/public/application/services/job_service.js | 3 +-- x-pack/plugins/ml/server/routes/job_service.ts | 1 + 7 files changed, 14 insertions(+), 6 deletions(-) diff --git a/x-pack/plugins/ml/common/types/anomaly_detection_jobs/combined_job.ts b/x-pack/plugins/ml/common/types/anomaly_detection_jobs/combined_job.ts index e7b10ff7c0422..42dee46e71fd6 100644 --- a/x-pack/plugins/ml/common/types/anomaly_detection_jobs/combined_job.ts +++ b/x-pack/plugins/ml/common/types/anomaly_detection_jobs/combined_job.ts @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import cloneDeep from 'lodash/cloneDeep'; +import { cloneDeep } from 'lodash'; import { Datafeed } from './datafeed'; import { DatafeedStats } from './datafeed_stats'; import { Job } from './job'; diff --git a/x-pack/plugins/ml/common/util/errors.test.ts b/x-pack/plugins/ml/common/util/errors.test.ts index 0b99799e3b6ec..4004d60479db2 100644 --- a/x-pack/plugins/ml/common/util/errors.test.ts +++ b/x-pack/plugins/ml/common/util/errors.test.ts @@ -45,6 +45,15 @@ describe('ML - error message utils', () => { }; expect(extractErrorMessage(bodyWithStringMessage)).toBe(testMsg); + const bodyWithAttributes: MLCustomHttpResponseOptions = { + body: { + message: 'Something else', + attributes: { body: { status: 404, error: { reason: testMsg } } }, + }, + statusCode: 404, + }; + expect(extractErrorMessage(bodyWithAttributes)).toBe(testMsg); + const bodyWithString: MLCustomHttpResponseOptions = { body: testMsg, statusCode: 404, diff --git a/x-pack/plugins/ml/common/util/errors.ts b/x-pack/plugins/ml/common/util/errors.ts index 2a408b1cb665a..cd8f98ca2425d 100644 --- a/x-pack/plugins/ml/common/util/errors.ts +++ b/x-pack/plugins/ml/common/util/errors.ts @@ -131,7 +131,6 @@ export const extractErrorProperties = ( } } - // I DON'T THINK THESE ONES BELOW WILL BE HIT if (typeof error.body.message === 'string') { return { message: error.body.message, diff --git a/x-pack/plugins/ml/public/application/components/annotations/annotation_flyout/index.tsx b/x-pack/plugins/ml/public/application/components/annotations/annotation_flyout/index.tsx index 67d03beced5f0..84abe3ed8a821 100644 --- a/x-pack/plugins/ml/public/application/components/annotations/annotation_flyout/index.tsx +++ b/x-pack/plugins/ml/public/application/components/annotations/annotation_flyout/index.tsx @@ -7,7 +7,7 @@ import React, { Component, FC, ReactNode, useCallback } from 'react'; import useObservable from 'react-use/lib/useObservable'; import * as Rx from 'rxjs'; -import cloneDeep from 'lodash/cloneDeep'; +import { cloneDeep } from 'lodash'; import { EuiButton, diff --git a/x-pack/plugins/ml/public/application/jobs/jobs_list/components/utils.js b/x-pack/plugins/ml/public/application/jobs/jobs_list/components/utils.js index b8461ecbea9b0..603ac57a42cfe 100644 --- a/x-pack/plugins/ml/public/application/jobs/jobs_list/components/utils.js +++ b/x-pack/plugins/ml/public/application/jobs/jobs_list/components/utils.js @@ -176,7 +176,7 @@ function showResults(resp, action) { export async function cloneJob(jobId) { try { - const job = await loadFullJob(jobId + 4); + const job = await loadFullJob(jobId); if (job.custom_settings && job.custom_settings.created_by) { // if the job is from a wizards, i.e. contains a created_by property // use tempJobCloningObjects to temporarily store the job diff --git a/x-pack/plugins/ml/public/application/services/job_service.js b/x-pack/plugins/ml/public/application/services/job_service.js index 324bd16581128..61ec4c7f801fd 100644 --- a/x-pack/plugins/ml/public/application/services/job_service.js +++ b/x-pack/plugins/ml/public/application/services/job_service.js @@ -117,7 +117,6 @@ class JobService { return new Promise((resolve, reject) => { jobs = []; datafeedIds = {}; - console.log(12222); ml.getJobs() .then((resp) => { jobs = resp.jobs; @@ -574,7 +573,7 @@ class JobService { } ml.startDatafeed({ - datafeedId: datafeedId + 22, + datafeedId, start, end, }) diff --git a/x-pack/plugins/ml/server/routes/job_service.ts b/x-pack/plugins/ml/server/routes/job_service.ts index 2a75fac2d125e..3c7f35b871b10 100644 --- a/x-pack/plugins/ml/server/routes/job_service.ts +++ b/x-pack/plugins/ml/server/routes/job_service.ts @@ -498,6 +498,7 @@ export function jobServiceRoutes({ router, mlLicense }: RouteInitialization) { splitFieldName, splitFieldValue, } = request.body; + const { newJobLineChart } = jobServiceProvider(client); const resp = await newJobLineChart( indexPatternTitle, From 7ff0187450f604ebd5081f9b9aba9ef693408b8a Mon Sep 17 00:00:00 2001 From: James Gowdy Date: Mon, 7 Sep 2020 16:00:29 +0100 Subject: [PATCH 07/17] updating translations --- x-pack/plugins/translations/translations/ja-JP.json | 4 ---- x-pack/plugins/translations/translations/zh-CN.json | 4 ---- 2 files changed, 8 deletions(-) diff --git a/x-pack/plugins/translations/translations/ja-JP.json b/x-pack/plugins/translations/translations/ja-JP.json index eacb1febd20ff..928596a332158 100644 --- a/x-pack/plugins/translations/translations/ja-JP.json +++ b/x-pack/plugins/translations/translations/ja-JP.json @@ -11364,14 +11364,10 @@ "xpack.ml.jobService.closedJobsLabel": "ジョブを作成", "xpack.ml.jobService.couldNotStartDatafeedErrorMessage": "{jobId} のデータフィードを開始できませんでした", "xpack.ml.jobService.couldNotStopDatafeedErrorMessage": "{jobId} のデータフィードを停止できませんでした", - "xpack.ml.jobService.couldNotUpdateDatafeedErrorMessage": "データフィードを更新できませんでした: {datafeedId}", - "xpack.ml.jobService.datafeedsListCouldNotBeRetrievedErrorMessage": "データフィードリストを取得できませんでした", "xpack.ml.jobService.failedJobsLabel": "失敗したジョブ", - "xpack.ml.jobService.jobsListCouldNotBeRetrievedErrorMessage": "ジョブリストを取得できませんでした", "xpack.ml.jobService.openJobsLabel": "ジョブを開く", "xpack.ml.jobService.requestMayHaveTimedOutErrorMessage": "リクエストがタイムアウトし、まだバックグラウンドで実行中の可能性があります。", "xpack.ml.jobService.totalJobsLabel": "合計ジョブ数", - "xpack.ml.jobService.updateJobErrorTitle": "ジョブを更新できませんでした: {jobId}", "xpack.ml.jobService.validateJobErrorTitle": "ジョブ検証エラー", "xpack.ml.jobsList.actionExecuteSuccessfullyNotificationMessage": "{successesJobsCount, plural, one{{successJob}} other{# 件のジョブ}} {actionTextPT}成功", "xpack.ml.jobsList.actionFailedNotificationMessage": "{failureId} が {actionText} に失敗しました", diff --git a/x-pack/plugins/translations/translations/zh-CN.json b/x-pack/plugins/translations/translations/zh-CN.json index bd30703dd5bd6..3dcb3c507a8c9 100644 --- a/x-pack/plugins/translations/translations/zh-CN.json +++ b/x-pack/plugins/translations/translations/zh-CN.json @@ -11371,14 +11371,10 @@ "xpack.ml.jobService.closedJobsLabel": "已关闭的作业", "xpack.ml.jobService.couldNotStartDatafeedErrorMessage": "无法开始 {jobId} 的数据馈送", "xpack.ml.jobService.couldNotStopDatafeedErrorMessage": "无法停止 {jobId} 的数据馈送", - "xpack.ml.jobService.couldNotUpdateDatafeedErrorMessage": "无法更新数据馈送：{datafeedId}", - "xpack.ml.jobService.datafeedsListCouldNotBeRetrievedErrorMessage": "无法检索数据馈送列表", "xpack.ml.jobService.failedJobsLabel": "失败的作业", - "xpack.ml.jobService.jobsListCouldNotBeRetrievedErrorMessage": "无法检索作业列表", "xpack.ml.jobService.openJobsLabel": "打开的作业", "xpack.ml.jobService.requestMayHaveTimedOutErrorMessage": "请求可能已超时，并可能仍在后台运行。", "xpack.ml.jobService.totalJobsLabel": "总计作业数", - "xpack.ml.jobService.updateJobErrorTitle": "无法更新作业：{jobId}", "xpack.ml.jobService.validateJobErrorTitle": "作业验证错误", "xpack.ml.jobsList.actionExecuteSuccessfullyNotificationMessage": "{successesJobsCount, plural, one{{successJob}} other{# 个作业}}{actionTextPT}已成功", "xpack.ml.jobsList.actionFailedNotificationMessage": "{failureId} 未能{actionText}", From ede532b7d0dcecde1bd0ab6dd8909265a6c0cd92 Mon Sep 17 00:00:00 2001 From: James Gowdy Date: Tue, 8 Sep 2020 21:08:30 +0100 Subject: [PATCH 08/17] rewriting error extracting code --- .../ml/common/types/data_frame_analytics.ts | 5 +- x-pack/plugins/ml/common/util/errors.ts | 177 +++++++----------- .../messagebar/messagebar_service.js | 14 +- .../analytics_service/delete_analytics.ts | 9 +- .../edit_job_flyout/edit_job_flyout.js | 4 +- .../services/toast_notification_service.ts | 84 ++------- .../ml/public/application/util/ml_error.ts | 23 +-- .../ml/server/models/job_service/datafeeds.ts | 5 + .../ml/server/routes/data_frame_analytics.ts | 13 +- 9 files changed, 127 insertions(+), 207 deletions(-) diff --git a/x-pack/plugins/ml/common/types/data_frame_analytics.ts b/x-pack/plugins/ml/common/types/data_frame_analytics.ts index f0aac75047585..bc89703dfa49f 100644 --- a/x-pack/plugins/ml/common/types/data_frame_analytics.ts +++ b/x-pack/plugins/ml/common/types/data_frame_analytics.ts @@ -4,11 +4,12 @@ * you may not use this file except in compliance with the Elastic License. */ -import { CustomHttpResponseOptions, ResponseError } from 'kibana/server'; +import Boom from 'boom'; +import { EsErrorBody } from '../util/errors'; export interface DeleteDataFrameAnalyticsWithIndexStatus { success: boolean; - error?: CustomHttpResponseOptions; + error?: EsErrorBody | Boom; } export type IndexName = string; diff --git a/x-pack/plugins/ml/common/util/errors.ts b/x-pack/plugins/ml/common/util/errors.ts index cd8f98ca2425d..fd802961def76 100644 --- a/x-pack/plugins/ml/common/util/errors.ts +++ b/x-pack/plugins/ml/common/util/errors.ts @@ -4,143 +4,113 @@ * you may not use this file except in compliance with the Elastic License. */ -import { ResponseError, ResponseHeaders } from 'kibana/server'; - -// Adding temporary types until Kibana ResponseError is updated -export interface BoomResponse { - data: any; - isBoom: boolean; - isServer: boolean; - output: { - statusCode: number; - payload: { - statusCode: number; - error: string; - message: string; - }; - headers: {}; +import { HttpFetchError } from 'kibana/public'; +import Boom from 'boom'; + +export interface EsErrorRootCause { + type: string; + reason: string; +} + +export interface EsErrorBody { + error: { + root_cause?: EsErrorRootCause[]; + type: string; + reason: string; }; + status: number; } -export type MLResponseError = - | { - message: { - msg: string; - }; - } - | { msg: string; statusCode: number; response: string }; - -export interface MLCustomHttpResponseOptions< - T extends ResponseError | MLResponseError | BoomResponse -> { - /** HTTP message to send to the client */ - body?: T; - /** HTTP Headers with additional information about response */ - headers?: ResponseHeaders; + +export interface MLResponseError { statusCode: number; + error: string; + message: string; + attributes?: { + body: EsErrorBody; + }; +} + +export interface MLHttpFetchError extends HttpFetchError { + body: T; +} + +export type ErrorType = MLHttpFetchError | EsErrorBody | Boom | string | undefined; + +function isEsErrorBody(error: any): error is EsErrorBody { + return error && error.error?.reason !== undefined; +} + +function isErrorString(error: any): error is string { + return typeof error === 'string'; +} + +function isMLResponseError(error: any): error is MLResponseError { + return typeof error.body === 'object' && 'message' in error.body; +} + +function isBoomError(error: any): error is Boom { + return error.isBoom === true; } export interface MLErrorObject { message: string; - fullErrorMessage?: string; // For use in a 'See full error' popover. statusCode?: number; + fullError?: EsErrorBody; } -export const extractErrorProperties = ( - error: - | MLCustomHttpResponseOptions - | string - | undefined -): MLErrorObject => { +export const extractErrorProperties = (error: ErrorType): MLErrorObject => { // extract properties of the error object from within the response error // coming from Kibana, Elasticsearch, and our own ML messages - let message = ''; - let fullErrorMessage; - let statusCode; - if (typeof error === 'string') { + // some responses contain raw es errors as part of a bulk response + // e.g. if some jobs fail the action in a bulk request + if (isEsErrorBody(error)) { return { - message: error, + message: error.error.reason, + statusCode: error.status, + fullError: error, }; } - if (error?.body === undefined) { + + if (isErrorString(error)) { return { - message: '', + message: error, }; } - if (typeof error.body === 'string') { + if (isBoomError(error)) { return { - message: error.body, + message: error.output.payload.message, + statusCode: error.output.payload.statusCode, }; } - if ( - typeof error.body === 'object' && - 'output' in error.body && - error.body.output.payload.message - ) { + + if (error?.body === undefined) { return { - message: error.body.output.payload.message, + message: '', }; } - if ( - typeof error.body === 'object' && - 'response' in error.body && - typeof error.body.response === 'string' - ) { - const errorResponse = JSON.parse(error.body.response); - if ('error' in errorResponse && typeof errorResponse === 'object') { - const errorResponseError = errorResponse.error; - if ('reason' in errorResponseError) { - message = errorResponseError.reason; - } - if ('caused_by' in errorResponseError) { - const causedByMessage = JSON.stringify(errorResponseError.caused_by); - // Only add a fullErrorMessage if different to the message. - if (causedByMessage !== message) { - fullErrorMessage = causedByMessage; - } - } - return { - message, - fullErrorMessage, - statusCode: error.statusCode, - }; - } - } - - if (typeof error.body === 'object' && 'msg' in error.body && typeof error.body.msg === 'string') { + if (typeof error.body === 'string') { return { - message: error.body.msg, + message: error.body, }; } - if (typeof error.body === 'object' && 'message' in error.body) { + if (isMLResponseError(error)) { if ( - 'attributes' in error.body && typeof error.body.attributes === 'object' && - error.body.attributes.body?.status !== undefined + typeof error.body.attributes.body?.error?.reason === 'string' ) { - statusCode = error.body.attributes.body.status; - - if (typeof error.body.attributes.body.error?.reason === 'string') { - return { - message: error.body.attributes.body.error.reason, - statusCode, - }; - } - } - - if (typeof error.body.message === 'string') { return { - message: error.body.message, - statusCode, + message: error.body.attributes.body.error.reason, + statusCode: error.body.statusCode, + fullError: error.body.attributes.body, }; - } - if (!(error.body.message instanceof Error) && typeof (error.body.message.msg === 'string')) { + } else { return { - message: error.body.message.msg, - statusCode, + message: error.body.message, + statusCode: error.body.statusCode, }; } } @@ -151,12 +121,7 @@ export const extractErrorProperties = ( }; }; -export const extractErrorMessage = ( - error: - | MLCustomHttpResponseOptions - | undefined - | string -): string => { +export const extractErrorMessage = (error: ErrorType): string => { // extract only the error message within the response error coming from Kibana, Elasticsearch, and our own ML messages const errorObj = extractErrorProperties(error); return errorObj.message; diff --git a/x-pack/plugins/ml/public/application/components/messagebar/messagebar_service.js b/x-pack/plugins/ml/public/application/components/messagebar/messagebar_service.js index 7220f8f7b4ba6..7f4ee44ebbca0 100644 --- a/x-pack/plugins/ml/public/application/components/messagebar/messagebar_service.js +++ b/x-pack/plugins/ml/public/application/components/messagebar/messagebar_service.js @@ -9,18 +9,18 @@ import { getToastNotifications } from '../../util/dependency_cache'; import { MLRequestFailure } from '../../util/ml_error'; import { extractErrorMessage } from '../../../../common/util/errors'; -function errorNotify(text, resp) { +function errorNotify(error, resp) { let err = null; - if (typeof text === 'object') { - err = new Error(extractErrorMessage(text)); - } else if (typeof text === 'object' && text.message !== undefined) { - err = new Error(text.message); + if (typeof error === 'object') { + err = new Error(extractErrorMessage(error)); + } else if (typeof error === 'object' && error.message !== undefined) { + err = new Error(error.message); } else { - err = new Error(text); + err = new Error(error); } const toastNotifications = getToastNotifications(); - toastNotifications.addError(new MLRequestFailure(err, resp ?? text), { + toastNotifications.addError(new MLRequestFailure(err, resp ?? error), { title: i18n.translate('xpack.ml.messagebarService.errorTitle', { defaultMessage: 'An error has occurred', }), diff --git a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/services/analytics_service/delete_analytics.ts b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/services/analytics_service/delete_analytics.ts index 9de859742438e..a21be83732613 100644 --- a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/services/analytics_service/delete_analytics.ts +++ b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/services/analytics_service/delete_analytics.ts @@ -85,12 +85,11 @@ export const deleteAnalyticsAndDestIndex = async ( ); } if (status.destIndexDeleted?.error) { - const error = extractErrorMessage(status.destIndexDeleted.error); - toastNotificationService.displayDangerToast( + toastNotificationService.displayErrorToast( + status.destIndexDeleted.error, i18n.translate('xpack.ml.dataframe.analyticsList.deleteAnalyticsWithIndexErrorMessage', { - defaultMessage: - 'An error occurred deleting destination index {destinationIndex}: {error}', - values: { destinationIndex, error }, + defaultMessage: 'An error occurred deleting destination index {destinationIndex}', + values: { destinationIndex }, }) ); } diff --git a/x-pack/plugins/ml/public/application/jobs/jobs_list/components/edit_job_flyout/edit_job_flyout.js b/x-pack/plugins/ml/public/application/jobs/jobs_list/components/edit_job_flyout/edit_job_flyout.js index 9d0082ffcb568..bd781d32a6b06 100644 --- a/x-pack/plugins/ml/public/application/jobs/jobs_list/components/edit_job_flyout/edit_job_flyout.js +++ b/x-pack/plugins/ml/public/application/jobs/jobs_list/components/edit_job_flyout/edit_job_flyout.js @@ -7,6 +7,8 @@ import PropTypes from 'prop-types'; import React, { Component } from 'react'; import { cloneDeep, isEqual, pick } from 'lodash'; +import { i18n } from '@kbn/i18n'; +import { FormattedMessage } from '@kbn/i18n/react'; import { EuiButton, EuiButtonEmpty, @@ -28,8 +30,6 @@ import { loadFullJob } from '../utils'; import { validateModelMemoryLimit, validateGroupNames, isValidCustomUrls } from '../validate_job'; import { toastNotificationServiceProvider } from '../../../../services/toast_notification_service'; import { withKibana } from '../../../../../../../../../src/plugins/kibana_react/public'; -import { i18n } from '@kbn/i18n'; -import { FormattedMessage } from '@kbn/i18n/react'; import { collapseLiteralStrings } from '../../../../../../shared_imports'; import { DATAFEED_STATE } from '../../../../../../common/constants/states'; diff --git a/x-pack/plugins/ml/public/application/services/toast_notification_service.ts b/x-pack/plugins/ml/public/application/services/toast_notification_service.ts index 94381ae3f1e51..0c11eef0e449c 100644 --- a/x-pack/plugins/ml/public/application/services/toast_notification_service.ts +++ b/x-pack/plugins/ml/public/application/services/toast_notification_service.ts @@ -5,78 +5,30 @@ */ import { ToastInput, ToastOptions, ToastsStart } from 'kibana/public'; -import { ResponseError } from 'kibana/server'; import { useMemo } from 'react'; import { useNotifications } from '../contexts/kibana'; -import { - BoomResponse, - extractErrorProperties, - MLCustomHttpResponseOptions, - MLErrorObject, - MLResponseError, -} from '../../../common/util/errors'; +import { MLRequestFailure } from '../util/ml_error'; +import { ErrorType, extractErrorProperties } from '../../../common/util/errors'; export type ToastNotificationService = ReturnType; export function toastNotificationServiceProvider(toastNotifications: ToastsStart) { - return { - displayDangerToast(toastOrTitle: ToastInput, options?: ToastOptions) { - toastNotifications.addDanger(toastOrTitle, options); - }, - - displaySuccessToast(toastOrTitle: ToastInput, options?: ToastOptions) { - toastNotifications.addSuccess(toastOrTitle, options); - }, - - displayErrorToast(error: any, toastTitle: string) { - const errorObj = this.parseErrorMessage(error); - if (errorObj.fullErrorMessage !== undefined) { - // Provide access to the full error message via the 'See full error' button. - toastNotifications.addError(new Error(errorObj.fullErrorMessage), { - title: toastTitle, - toastMessage: errorObj.message, - }); - } else { - toastNotifications.addDanger( - { - title: toastTitle, - text: errorObj.message, - }, - { toastLifeTimeMs: 30000 } - ); - } - }, - - parseErrorMessage( - error: - | MLCustomHttpResponseOptions - | undefined - | string - | MLResponseError - ): MLErrorObject { - if ( - typeof error === 'object' && - 'response' in error && - typeof error.response === 'string' && - error.statusCode !== undefined - ) { - // MLResponseError which has been received back as part of a 'successful' response - // where the error was passed in a separate property in the response. - const wrapMlResponseError = { - body: error, - statusCode: error.statusCode, - }; - return extractErrorProperties(wrapMlResponseError); - } - - return extractErrorProperties( - error as - | MLCustomHttpResponseOptions - | undefined - | string - ); - }, - }; + function displayDangerToast(toastOrTitle: ToastInput, options?: ToastOptions) { + toastNotifications.addDanger(toastOrTitle, options); + } + + function displaySuccessToast(toastOrTitle: ToastInput, options?: ToastOptions) { + toastNotifications.addSuccess(toastOrTitle, options); + } + + function displayErrorToast(error: ErrorType, toastTitle: string) { + const errorObj = extractErrorProperties(error); + toastNotifications.addError(new MLRequestFailure(errorObj, error), { + title: toastTitle, + }); + } + + return { displayDangerToast, displaySuccessToast, displayErrorToast }; } /** diff --git a/x-pack/plugins/ml/public/application/util/ml_error.ts b/x-pack/plugins/ml/public/application/util/ml_error.ts index f72b04ab12a7d..bd4c9c0f7dcd1 100644 --- a/x-pack/plugins/ml/public/application/util/ml_error.ts +++ b/x-pack/plugins/ml/public/application/util/ml_error.ts @@ -7,20 +7,21 @@ import { KbnError } from '../../../../../../src/plugins/kibana_utils/public'; export class MLRequestFailure extends KbnError { - origError: any; - resp: any; - stack: any; // takes an Error object and and optional response object - // if error is falsy (null) the response object will be used - // notify will show the full expandable stack trace of the response if a response object is used and no error is passed in. + constructor(error: any, resp: any) { - error = error || {}; - super(error.message || JSON.stringify(resp)); + super(error.message); - this.origError = error; - this.resp = typeof resp === 'string' ? JSON.parse(resp) : resp; - if (resp.body) { - this.stack = JSON.stringify(resp.body, null, 2); + if (typeof resp === 'object') { + if ('body' in resp) { + this.stack = JSON.stringify(resp.body, null, 2); + } else { + try { + this.stack = JSON.stringify(resp, null, 2); + } catch (e) { + // fail silently + } + } } } } diff --git a/x-pack/plugins/ml/server/models/job_service/datafeeds.ts b/x-pack/plugins/ml/server/models/job_service/datafeeds.ts index c0eb1b72825df..62ef9b3621610 100644 --- a/x-pack/plugins/ml/server/models/job_service/datafeeds.ts +++ b/x-pack/plugins/ml/server/models/job_service/datafeeds.ts @@ -118,6 +118,11 @@ export function datafeedsProvider({ asInternalUser }: IScopedClusterClient) { } catch (error) { if (isRequestTimeout(error)) { return fillResultsWithTimeouts(results, datafeedId, datafeedIds, DATAFEED_STATE.STOPPED); + } else { + results[datafeedId] = { + started: false, + error: error.body, + }; } } } diff --git a/x-pack/plugins/ml/server/routes/data_frame_analytics.ts b/x-pack/plugins/ml/server/routes/data_frame_analytics.ts index dea4803e8275e..999613fdb70f6 100644 --- a/x-pack/plugins/ml/server/routes/data_frame_analytics.ts +++ b/x-pack/plugins/ml/server/routes/data_frame_analytics.ts @@ -348,8 +348,8 @@ export function dataFrameAnalyticsRoutes({ router, mlLicense }: RouteInitializat index: destinationIndex, }); destIndexDeleted.success = true; - } catch (deleteIndexError) { - destIndexDeleted.error = wrapError(deleteIndexError); + } catch ({ body }) { + destIndexDeleted.error = body; } } else { return response.forbidden(); @@ -365,7 +365,7 @@ export function dataFrameAnalyticsRoutes({ router, mlLicense }: RouteInitializat } destIndexPatternDeleted.success = true; } catch (deleteDestIndexPatternError) { - destIndexPatternDeleted.error = wrapError(deleteDestIndexPatternError); + destIndexPatternDeleted.error = deleteDestIndexPatternError; } } } @@ -377,11 +377,8 @@ export function dataFrameAnalyticsRoutes({ router, mlLicense }: RouteInitializat id: analyticsId, }); analyticsJobDeleted.success = true; - } catch (deleteDFAError) { - analyticsJobDeleted.error = wrapError(deleteDFAError); - if (analyticsJobDeleted.error.statusCode === 404) { - return response.notFound(); - } + } catch ({ body }) { + analyticsJobDeleted.error = body; } const results = { analyticsJobDeleted, From fe55673c8e086675b2db534671912d3d77714de8 Mon Sep 17 00:00:00 2001 From: James Gowdy Date: Wed, 9 Sep 2020 09:35:22 +0100 Subject: [PATCH 09/17] fixing tests --- x-pack/plugins/ml/common/util/errors.test.ts | 122 ++++++++++--------- 1 file changed, 66 insertions(+), 56 deletions(-) diff --git a/x-pack/plugins/ml/common/util/errors.test.ts b/x-pack/plugins/ml/common/util/errors.test.ts index 4004d60479db2..39dacbde70472 100644 --- a/x-pack/plugins/ml/common/util/errors.test.ts +++ b/x-pack/plugins/ml/common/util/errors.test.ts @@ -4,86 +4,96 @@ * you may not use this file except in compliance with the Elastic License. */ -import { - BoomResponse, - extractErrorMessage, - MLCustomHttpResponseOptions, - MLResponseError, -} from './errors'; -import { ResponseError } from 'kibana/server'; +import Boom from 'boom'; + +import { extractErrorMessage, MLHttpFetchError, MLResponseError, EsErrorBody } from './errors'; describe('ML - error message utils', () => { describe('extractErrorMessage', () => { test('returns just the error message', () => { - const testMsg = 'Saved object [index-pattern/blahblahblah] not found'; + const testMsg = 'Saved object [index-pattern/indexpattern] not found'; - const bodyWithNestedErrorMsg: MLCustomHttpResponseOptions = { - body: { - message: { - msg: testMsg, - }, - }, - statusCode: 404, - }; - expect(extractErrorMessage(bodyWithNestedErrorMsg)).toBe(testMsg); + // bad error, return empty string + const badError = {} as any; + expect(extractErrorMessage(badError)).toBe(''); - const bodyWithStringMsg: MLCustomHttpResponseOptions = { - body: { - msg: testMsg, - statusCode: 404, - response: `{"error":{"reason":"${testMsg}"}}`, + // raw es error + const esErrorMsg: EsErrorBody = { + error: { + root_cause: [ + { + type: 'type', + reason: 'reason', + }, + ], + type: 'type', + reason: testMsg, }, - statusCode: 404, + status: 404, }; - expect(extractErrorMessage(bodyWithStringMsg)).toBe(testMsg); + expect(extractErrorMessage(esErrorMsg)).toBe(testMsg); - const bodyWithStringMessage: MLCustomHttpResponseOptions = { + // error is basic string + const stringMessage = testMsg; + expect(extractErrorMessage(stringMessage)).toBe(testMsg); + + // kibana error without attributes + const bodyWithoutAttributes: MLHttpFetchError = { + name: 'name', + req: {} as Request, + request: {} as Request, + message: 'Something else', body: { + statusCode: 404, + error: 'error', message: testMsg, }, - statusCode: 404, }; - expect(extractErrorMessage(bodyWithStringMessage)).toBe(testMsg); + expect(extractErrorMessage(bodyWithoutAttributes)).toBe(testMsg); - const bodyWithAttributes: MLCustomHttpResponseOptions = { + // kibana error with attributes + const bodyWithAttributes: MLHttpFetchError = { + name: 'name', + req: {} as Request, + request: {} as Request, + message: 'Something else', body: { + statusCode: 404, + error: 'error', message: 'Something else', - attributes: { body: { status: 404, error: { reason: testMsg } } }, + attributes: { + body: { + status: 404, + error: { + reason: testMsg, + type: 'type', + root_cause: [{ type: 'type', reason: 'reason' }], + }, + }, + }, }, - statusCode: 404, }; expect(extractErrorMessage(bodyWithAttributes)).toBe(testMsg); - const bodyWithString: MLCustomHttpResponseOptions = { - body: testMsg, - statusCode: 404, - }; - expect(extractErrorMessage(bodyWithString)).toBe(testMsg); - - const bodyWithError: MLCustomHttpResponseOptions = { - body: new Error(testMsg), - statusCode: 404, - }; - expect(extractErrorMessage(bodyWithError)).toBe(testMsg); - - const bodyWithBoomError: MLCustomHttpResponseOptions = { - statusCode: 404, - body: { - data: [], - isBoom: true, - isServer: false, - output: { + // boom error + const boomError: Boom = { + message: '', + reformat: () => '', + name: '', + data: [], + isBoom: true, + isServer: false, + output: { + statusCode: 404, + payload: { statusCode: 404, - payload: { - statusCode: 404, - error: testMsg, - message: testMsg, - }, - headers: {}, + error: testMsg, + message: testMsg, }, + headers: {}, }, }; - expect(extractErrorMessage(bodyWithBoomError)).toBe(testMsg); + expect(extractErrorMessage(boomError)).toBe(testMsg); }); }); }); From 734d9d255440ff6be95cff6d22f55bd90dd8b831 Mon Sep 17 00:00:00 2001 From: James Gowdy Date: Wed, 9 Sep 2020 13:16:53 +0100 Subject: [PATCH 10/17] removing message bar service --- .../messagebar/messagebar_service.d.ts | 11 ---- .../messagebar/messagebar_service.js | 32 ------------ .../components/rule_editor/utils.js | 2 +- .../group_selector/group_selector.js | 12 ++--- .../jobs/jobs_list/components/utils.js | 32 +++++------- .../estimate_bucket_span.ts | 6 +-- .../categorization_view/metric_selection.tsx | 4 +- .../multi_metric_view/metric_selection.tsx | 6 +-- .../metric_selection_summary.tsx | 6 +-- .../population_view/metric_selection.tsx | 4 +- .../metric_selection_summary.tsx | 4 +- .../single_metric_view/metric_selection.tsx | 4 +- .../metric_selection_summary.tsx | 4 +- .../pages/components/summary_step/summary.tsx | 26 +++++----- .../application/services/job_service.js | 52 ++----------------- .../services/ml_api_service/index.ts | 2 +- .../toast_notification_service}/index.ts | 7 ++- .../toast_notification_service.ts | 21 ++++++-- 18 files changed, 80 insertions(+), 155 deletions(-) delete mode 100644 x-pack/plugins/ml/public/application/components/messagebar/messagebar_service.d.ts delete mode 100644 x-pack/plugins/ml/public/application/components/messagebar/messagebar_service.js rename x-pack/plugins/ml/public/application/{components/messagebar => services/toast_notification_service}/index.ts (58%) rename x-pack/plugins/ml/public/application/services/{ => toast_notification_service}/toast_notification_service.ts (64%) diff --git a/x-pack/plugins/ml/public/application/components/messagebar/messagebar_service.d.ts b/x-pack/plugins/ml/public/application/components/messagebar/messagebar_service.d.ts deleted file mode 100644 index ef5088e281319..0000000000000 --- a/x-pack/plugins/ml/public/application/components/messagebar/messagebar_service.d.ts +++ /dev/null @@ -1,11 +0,0 @@ -/* - * 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. - */ - -declare interface MlMessageBarService { - error(text: any, resp?: any): void; -} - -export const mlMessageBarService: MlMessageBarService; diff --git a/x-pack/plugins/ml/public/application/components/messagebar/messagebar_service.js b/x-pack/plugins/ml/public/application/components/messagebar/messagebar_service.js deleted file mode 100644 index 7f4ee44ebbca0..0000000000000 --- a/x-pack/plugins/ml/public/application/components/messagebar/messagebar_service.js +++ /dev/null @@ -1,32 +0,0 @@ -/* - * 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. - */ - -import { i18n } from '@kbn/i18n'; -import { getToastNotifications } from '../../util/dependency_cache'; -import { MLRequestFailure } from '../../util/ml_error'; -import { extractErrorMessage } from '../../../../common/util/errors'; - -function errorNotify(error, resp) { - let err = null; - if (typeof error === 'object') { - err = new Error(extractErrorMessage(error)); - } else if (typeof error === 'object' && error.message !== undefined) { - err = new Error(error.message); - } else { - err = new Error(error); - } - - const toastNotifications = getToastNotifications(); - toastNotifications.addError(new MLRequestFailure(err, resp ?? error), { - title: i18n.translate('xpack.ml.messagebarService.errorTitle', { - defaultMessage: 'An error has occurred', - }), - }); -} - -export const mlMessageBarService = { - error: errorNotify, -}; diff --git a/x-pack/plugins/ml/public/application/components/rule_editor/utils.js b/x-pack/plugins/ml/public/application/components/rule_editor/utils.js index d5072ae699d61..6a8e20647cdbe 100644 --- a/x-pack/plugins/ml/public/application/components/rule_editor/utils.js +++ b/x-pack/plugins/ml/public/application/components/rule_editor/utils.js @@ -146,7 +146,7 @@ export function updateJobRules(job, detectorIndex, rules) { } return new Promise((resolve, reject) => { - ml.updateJob(jobId, jobData) + ml.updateJob({ jobId: jobId, job: jobData }) .then(() => { // Refresh the job data in the job service before resolving. mlJobService diff --git a/x-pack/plugins/ml/public/application/jobs/jobs_list/components/multi_job_actions/group_selector/group_selector.js b/x-pack/plugins/ml/public/application/jobs/jobs_list/components/multi_job_actions/group_selector/group_selector.js index 6eacd986d5da2..ffb2dfe7e495c 100644 --- a/x-pack/plugins/ml/public/application/jobs/jobs_list/components/multi_job_actions/group_selector/group_selector.js +++ b/x-pack/plugins/ml/public/application/jobs/jobs_list/components/multi_job_actions/group_selector/group_selector.js @@ -6,6 +6,8 @@ import PropTypes from 'prop-types'; import React, { Component } from 'react'; +import { i18n } from '@kbn/i18n'; +import { FormattedMessage } from '@kbn/i18n/react'; import { EuiButton, @@ -25,9 +27,7 @@ import { ml } from '../../../../../services/ml_api_service'; import { checkPermission } from '../../../../../capabilities/check_capabilities'; import { GroupList } from './group_list'; import { NewGroupInput } from './new_group_input'; -import { mlMessageBarService } from '../../../../../components/messagebar'; -import { i18n } from '@kbn/i18n'; -import { FormattedMessage } from '@kbn/i18n/react'; +import { getToastNotificationService } from '../../../../../services/toast_notification_service'; function createSelectedGroups(jobs, groups) { const jobIds = jobs.map((j) => j.id); @@ -151,7 +151,7 @@ export class GroupSelector extends Component { } } - const tempJobs = newJobs.map((j) => ({ job_id: j.id, groups: j.newGroups })); + const tempJobs = newJobs.map((j) => ({ job_id: j.id + 2, groups: j.newGroups })); ml.jobs .updateGroups(tempJobs) .then((resp) => { @@ -160,7 +160,7 @@ export class GroupSelector extends Component { // check success of each job update if (resp.hasOwnProperty(jobId)) { if (resp[jobId].success === false) { - mlMessageBarService.error(resp[jobId].error); + getToastNotificationService().displayErrorToast(resp[jobId].error); success = false; } } @@ -175,7 +175,7 @@ export class GroupSelector extends Component { } }) .catch((error) => { - mlMessageBarService.error(error); + getToastNotificationService().displayErrorToast(error); console.error(error); }); }; diff --git a/x-pack/plugins/ml/public/application/jobs/jobs_list/components/utils.js b/x-pack/plugins/ml/public/application/jobs/jobs_list/components/utils.js index 603ac57a42cfe..21824aac18cdd 100644 --- a/x-pack/plugins/ml/public/application/jobs/jobs_list/components/utils.js +++ b/x-pack/plugins/ml/public/application/jobs/jobs_list/components/utils.js @@ -8,9 +8,11 @@ import { each } from 'lodash'; import { i18n } from '@kbn/i18n'; import rison from 'rison-node'; -import { mlMessageBarService } from '../../../components/messagebar'; import { mlJobService } from '../../../services/job_service'; -import { toastNotificationServiceProvider } from '../../../services/toast_notification_service'; +import { + getToastNotificationService, + toastNotificationServiceProvider, +} from '../../../services/toast_notification_service'; import { getToastNotifications } from '../../../util/dependency_cache'; import { ml } from '../../../services/ml_api_service'; import { stringMatch } from '../../../util/string_utils'; @@ -60,7 +62,6 @@ export function forceStartDatafeeds(jobs, start, end, finish = () => {}) { finish(); }) .catch((error) => { - mlMessageBarService.error(error); const toastNotifications = getToastNotifications(); toastNotifications.addDanger( i18n.translate('xpack.ml.jobsList.startJobErrorMessage', { @@ -81,7 +82,6 @@ export function stopDatafeeds(jobs, finish = () => {}) { finish(); }) .catch((error) => { - mlMessageBarService.error(error); const toastNotifications = getToastNotifications(); toastNotifications.addDanger( i18n.translate('xpack.ml.jobsList.stopJobErrorMessage', { @@ -219,14 +219,12 @@ export async function cloneJob(jobId) { window.location.href = '#/jobs/new_job'; } catch (error) { - mlMessageBarService.error(error); - const toastNotifications = getToastNotifications(); - toastNotifications.addDanger( + getToastNotificationService().displayErrorToast( + error, i18n.translate('xpack.ml.jobsList.cloneJobErrorMessage', { defaultMessage: 'Could not clone {jobId}. Job could not be found', values: { jobId }, - }), - error + }) ); } } @@ -240,13 +238,11 @@ export function closeJobs(jobs, finish = () => {}) { finish(); }) .catch((error) => { - mlMessageBarService.error(error); - const toastNotifications = getToastNotifications(); - toastNotifications.addDanger( + getToastNotificationService().displayErrorToast( + error, i18n.translate('xpack.ml.jobsList.closeJobErrorMessage', { defaultMessage: 'Jobs failed to close', - }), - error + }) ); finish(); }); @@ -261,13 +257,11 @@ export function deleteJobs(jobs, finish = () => {}) { finish(); }) .catch((error) => { - mlMessageBarService.error(error); - const toastNotifications = getToastNotifications(); - toastNotifications.addDanger( + getToastNotificationService().displayErrorToast( + error, i18n.translate('xpack.ml.jobsList.deleteJobErrorMessage', { defaultMessage: 'Jobs failed to delete', - }), - error + }) ); finish(); }); diff --git a/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/bucket_span_estimator/estimate_bucket_span.ts b/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/bucket_span_estimator/estimate_bucket_span.ts index 91597414fd3fd..ad8c1c6e16037 100644 --- a/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/bucket_span_estimator/estimate_bucket_span.ts +++ b/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/bucket_span_estimator/estimate_bucket_span.ts @@ -15,7 +15,7 @@ import { } from '../../../../../common/job_creator'; import { ml, BucketSpanEstimatorData } from '../../../../../../../services/ml_api_service'; import { useMlContext } from '../../../../../../../contexts/ml'; -import { mlMessageBarService } from '../../../../../../../components/messagebar'; +import { getToastNotificationService } from '../../../../../../../services/toast_notification_service'; export enum ESTIMATE_STATUS { NOT_RUNNING, @@ -45,7 +45,7 @@ export function useEstimateBucketSpan() { (isMultiMetricJobCreator(jobCreator) || isPopulationJobCreator(jobCreator)) && jobCreator.splitField !== null ) { - data.splitField = jobCreator.splitField.id; + data.splitField = jobCreator.splitField.id + 33; } else if (isAdvancedJobCreator(jobCreator)) { jobCreator.richDetectors.some((d) => { if (d.partitionField !== null) { @@ -68,7 +68,7 @@ export function useEstimateBucketSpan() { const { name, error, message } = await ml.estimateBucketSpan(data); setStatus(ESTIMATE_STATUS.NOT_RUNNING); if (error === true) { - mlMessageBarService.error(message); + getToastNotificationService().displayErrorToast(message); } else { jobCreator.bucketSpan = name; jobCreatorUpdate(); diff --git a/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/categorization_view/metric_selection.tsx b/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/categorization_view/metric_selection.tsx index 6750cf0c7e9f4..da2e5cc0e63d9 100644 --- a/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/categorization_view/metric_selection.tsx +++ b/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/categorization_view/metric_selection.tsx @@ -6,7 +6,7 @@ import React, { FC, useContext, useEffect, useState } from 'react'; import { EuiHorizontalRule } from '@elastic/eui'; -import { mlMessageBarService } from '../../../../../../../components/messagebar'; +import { getToastNotificationService } from '../../../../../../../services/toast_notification_service'; import { JobCreatorContext } from '../../../job_creator_context'; import { CategorizationJobCreator } from '../../../../../common/job_creator'; @@ -94,7 +94,7 @@ export const CategorizationDetectors: FC = ({ setIsValid }) => { setFieldExamples(null); setValidationChecks([]); setOverallValidStatus(CATEGORY_EXAMPLES_VALIDATION_STATUS.INVALID); - mlMessageBarService.error(error); + getToastNotificationService().displayErrorToast(error); } } else { setFieldExamples(null); diff --git a/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/multi_metric_view/metric_selection.tsx b/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/multi_metric_view/metric_selection.tsx index 10a2a83179c0e..762d18a5367f1 100644 --- a/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/multi_metric_view/metric_selection.tsx +++ b/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/multi_metric_view/metric_selection.tsx @@ -15,7 +15,7 @@ import { AggFieldPair } from '../../../../../../../../../common/types/fields'; import { getChartSettings, defaultChartSettings } from '../../../charts/common/settings'; import { MetricSelector } from './metric_selector'; import { ChartGrid } from './chart_grid'; -import { mlMessageBarService } from '../../../../../../../components/messagebar'; +import { getToastNotificationService } from '../../../../../../../services/toast_notification_service'; interface Props { setIsValid: (na: boolean) => void; @@ -109,7 +109,7 @@ export const MultiMetricDetectors: FC = ({ setIsValid }) => { .loadFieldExampleValues(splitField) .then(setFieldValues) .catch((error) => { - mlMessageBarService.error(error); + getToastNotificationService().displayErrorToast(error); }); } else { setFieldValues([]); @@ -138,7 +138,7 @@ export const MultiMetricDetectors: FC = ({ setIsValid }) => { ); setLineChartsData(resp); } catch (error) { - mlMessageBarService.error(error); + getToastNotificationService().displayErrorToast(error); setLineChartsData([]); } setLoadingData(false); diff --git a/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/multi_metric_view/metric_selection_summary.tsx b/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/multi_metric_view/metric_selection_summary.tsx index a84e3cec9a0f6..cc0fbf2fc0a04 100644 --- a/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/multi_metric_view/metric_selection_summary.tsx +++ b/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/multi_metric_view/metric_selection_summary.tsx @@ -12,7 +12,7 @@ import { Results, ModelItem, Anomaly } from '../../../../../common/results_loade import { LineChartData } from '../../../../../common/chart_loader'; import { getChartSettings, defaultChartSettings } from '../../../charts/common/settings'; import { ChartGrid } from './chart_grid'; -import { mlMessageBarService } from '../../../../../../../components/messagebar'; +import { getToastNotificationService } from '../../../../../../../services/toast_notification_service'; export const MultiMetricDetectorsSummary: FC = () => { const { jobCreator: jc, chartLoader, resultsLoader, chartInterval } = useContext( @@ -43,7 +43,7 @@ export const MultiMetricDetectorsSummary: FC = () => { const tempFieldValues = await chartLoader.loadFieldExampleValues(jobCreator.splitField); setFieldValues(tempFieldValues); } catch (error) { - mlMessageBarService.error(error); + getToastNotificationService().displayErrorToast(error); } } })(); @@ -75,7 +75,7 @@ export const MultiMetricDetectorsSummary: FC = () => { ); setLineChartsData(resp); } catch (error) { - mlMessageBarService.error(error); + getToastNotificationService().displayErrorToast(error); setLineChartsData({}); } setLoadingData(false); diff --git a/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/population_view/metric_selection.tsx b/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/population_view/metric_selection.tsx index a22ca0a71f079..46f91550f6e32 100644 --- a/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/population_view/metric_selection.tsx +++ b/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/population_view/metric_selection.tsx @@ -17,7 +17,7 @@ import { getChartSettings, defaultChartSettings } from '../../../charts/common/s import { MetricSelector } from './metric_selector'; import { SplitFieldSelector } from '../split_field'; import { ChartGrid } from './chart_grid'; -import { mlMessageBarService } from '../../../../../../../components/messagebar'; +import { getToastNotificationService } from '../../../../../../../services/toast_notification_service'; interface Props { setIsValid: (na: boolean) => void; @@ -159,7 +159,7 @@ export const PopulationDetectors: FC = ({ setIsValid }) => { setLineChartsData(resp); } catch (error) { - mlMessageBarService.error(error); + getToastNotificationService().displayErrorToast(error); setLineChartsData([]); } setLoadingData(false); diff --git a/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/population_view/metric_selection_summary.tsx b/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/population_view/metric_selection_summary.tsx index ef3131132e81a..c32cc6ecc445a 100644 --- a/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/population_view/metric_selection_summary.tsx +++ b/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/population_view/metric_selection_summary.tsx @@ -15,7 +15,7 @@ import { LineChartData } from '../../../../../common/chart_loader'; import { Field, AggFieldPair } from '../../../../../../../../../common/types/fields'; import { getChartSettings, defaultChartSettings } from '../../../charts/common/settings'; import { ChartGrid } from './chart_grid'; -import { mlMessageBarService } from '../../../../../../../components/messagebar'; +import { getToastNotificationService } from '../../../../../../../services/toast_notification_service'; type DetectorFieldValues = Record; @@ -81,7 +81,7 @@ export const PopulationDetectorsSummary: FC = () => { setLineChartsData(resp); } catch (error) { - mlMessageBarService.error(error); + getToastNotificationService().displayErrorToast(error); setLineChartsData({}); } setLoadingData(false); diff --git a/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/single_metric_view/metric_selection.tsx b/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/single_metric_view/metric_selection.tsx index bbd96a6bdd0f6..5844e59225ab5 100644 --- a/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/single_metric_view/metric_selection.tsx +++ b/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/single_metric_view/metric_selection.tsx @@ -13,7 +13,7 @@ import { newJobCapsService } from '../../../../../../../services/new_job_capabil import { AggFieldPair } from '../../../../../../../../../common/types/fields'; import { AnomalyChart, CHART_TYPE } from '../../../charts/anomaly_chart'; import { getChartSettings } from '../../../charts/common/settings'; -import { mlMessageBarService } from '../../../../../../../components/messagebar'; +import { getToastNotificationService } from '../../../../../../../services/toast_notification_service'; interface Props { setIsValid: (na: boolean) => void; @@ -93,7 +93,7 @@ export const SingleMetricDetectors: FC = ({ setIsValid }) => { setLineChartData(resp); } } catch (error) { - mlMessageBarService.error(error); + getToastNotificationService().displayErrorToast(error); setLineChartData({}); } setLoadingData(false); diff --git a/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/single_metric_view/metric_selection_summary.tsx b/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/single_metric_view/metric_selection_summary.tsx index ebe915b309f15..ae019ee1bbf84 100644 --- a/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/single_metric_view/metric_selection_summary.tsx +++ b/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/single_metric_view/metric_selection_summary.tsx @@ -11,7 +11,7 @@ import { Results, ModelItem, Anomaly } from '../../../../../common/results_loade import { LineChartData } from '../../../../../common/chart_loader'; import { AnomalyChart, CHART_TYPE } from '../../../charts/anomaly_chart'; import { getChartSettings } from '../../../charts/common/settings'; -import { mlMessageBarService } from '../../../../../../../components/messagebar'; +import { getToastNotificationService } from '../../../../../../../services/toast_notification_service'; const DTR_IDX = 0; @@ -63,7 +63,7 @@ export const SingleMetricDetectorsSummary: FC = () => { setLineChartData(resp); } } catch (error) { - mlMessageBarService.error(error); + getToastNotificationService().displayErrorToast(error); setLineChartData({}); } setLoadingData(false); diff --git a/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/summary_step/summary.tsx b/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/summary_step/summary.tsx index 78c8072261b30..8948a1a6f6a85 100644 --- a/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/summary_step/summary.tsx +++ b/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/summary_step/summary.tsx @@ -22,13 +22,13 @@ import { JobCreatorContext } from '../job_creator_context'; import { JobRunner } from '../../../common/job_runner'; import { mlJobService } from '../../../../../services/job_service'; import { JsonEditorFlyout, EDITOR_MODE } from '../common/json_editor_flyout'; -import { extractErrorMessage } from '../../../../../../../common/util/errors'; import { isSingleMetricJobCreator, isAdvancedJobCreator } from '../../../common/job_creator'; import { JobDetails } from './components/job_details'; import { DatafeedDetails } from './components/datafeed_details'; import { DetectorChart } from './components/detector_chart'; import { JobProgress } from './components/job_progress'; import { PostSaveOptions } from './components/post_save_options'; +import { toastNotificationServiceProvider } from '../../../../../services/toast_notification_service'; import { convertToAdvancedJob, resetJob, @@ -73,13 +73,13 @@ export const SummaryStep: FC = ({ setCurrentStep, isCurrentStep }) => setJobRunner(jr); } catch (error) { // catch and display all job creation errors - const { toasts } = notifications; - toasts.addDanger({ - title: i18n.translate('xpack.ml.newJob.wizard.summaryStep.createJobError', { + const { displayErrorToast } = toastNotificationServiceProvider(notifications.toasts); + displayErrorToast( + error, + i18n.translate('xpack.ml.newJob.wizard.summaryStep.createJobError', { defaultMessage: `Job creation error`, - }), - text: extractErrorMessage(error), - }); + }) + ); setCreatingJob(false); } } @@ -92,13 +92,13 @@ export const SummaryStep: FC = ({ setCurrentStep, isCurrentStep }) => advancedStartDatafeed(jobCreator, navigateToPath); } catch (error) { // catch and display all job creation errors - const { toasts } = notifications; - toasts.addDanger({ - title: i18n.translate('xpack.ml.newJob.wizard.summaryStep.createJobError', { + const { displayErrorToast } = toastNotificationServiceProvider(notifications.toasts); + displayErrorToast( + error, + i18n.translate('xpack.ml.newJob.wizard.summaryStep.createJobError', { defaultMessage: `Job creation error`, - }), - text: extractErrorMessage(error), - }); + }) + ); setCreatingJob(false); } } diff --git a/x-pack/plugins/ml/public/application/services/job_service.js b/x-pack/plugins/ml/public/application/services/job_service.js index 61ec4c7f801fd..eb0800f58094b 100644 --- a/x-pack/plugins/ml/public/application/services/job_service.js +++ b/x-pack/plugins/ml/public/application/services/job_service.js @@ -14,7 +14,7 @@ import { i18n } from '@kbn/i18n'; import { ml } from './ml_api_service'; -import { mlMessageBarService } from '../components/messagebar'; +import { getToastNotificationService } from '../services/toast_notification_service'; import { isWebUrl } from '../util/url_utils'; import { ML_DATA_PREVIEW_COUNT } from '../../../common/util/job_utils'; import { TIME_FORMAT } from '../../../common/constants/time_format'; @@ -172,7 +172,7 @@ class JobService { function error(err) { console.log('jobService error getting list of jobs:', err); - mlMessageBarService.error(err); + getToastNotificationService().displayErrorToast(err); reject({ jobs, err }); } }); @@ -253,7 +253,7 @@ class JobService { function error(err) { console.log('JobService error getting list of jobs:', err); - mlMessageBarService.error(err); + getToastNotificationService().displayErrorToast(err); reject({ jobs, err }); } }); @@ -265,9 +265,6 @@ class JobService { ml.getDatafeeds(sId) .then((resp) => { - // console.log('loadDatafeeds query response:', resp); - - // make deep copy of datafeeds const datafeeds = resp.datafeeds; // load datafeeds stats @@ -294,7 +291,7 @@ class JobService { function error(err) { console.log('loadDatafeeds error getting list of datafeeds:', err); - mlMessageBarService.error(err); + getToastNotificationService().displayErrorToast(err); reject({ jobs, err }); } }); @@ -573,7 +570,7 @@ class JobService { } ml.startDatafeed({ - datafeedId, + datafeedId: 22, start, end, }) @@ -582,45 +579,6 @@ class JobService { }) .catch((err) => { console.log('jobService error starting datafeed:', err); - mlMessageBarService.error( - i18n.translate('xpack.ml.jobService.couldNotStartDatafeedErrorMessage', { - defaultMessage: 'Could not start datafeed for {jobId}', - values: { jobId }, - }), - err - ); - reject(err); - }); - }); - } - - // stop the datafeed for a given job - // refresh the job state on stop success - stopDatafeed(datafeedId, jobId) { - return new Promise((resolve, reject) => { - ml.stopDatafeed({ - datafeedId, - }) - .then((resp) => { - resolve(resp); - }) - .catch((err) => { - console.log('jobService error stopping datafeed:', err); - mlMessageBarService.error( - i18n.translate('xpack.ml.jobService.couldNotStopDatafeedErrorMessage', { - defaultMessage: 'Could not stop datafeed for {jobId}', - values: { jobId }, - }), - err - ); - if (err.statusCode === 500) { - mlMessageBarService.error( - i18n.translate('xpack.ml.jobService.requestMayHaveTimedOutErrorMessage', { - defaultMessage: - 'Request may have timed out and may still be running in the background.', - }) - ); - } reject(err); }); }); diff --git a/x-pack/plugins/ml/public/application/services/ml_api_service/index.ts b/x-pack/plugins/ml/public/application/services/ml_api_service/index.ts index 184039729f9ef..baa6817093000 100644 --- a/x-pack/plugins/ml/public/application/services/ml_api_service/index.ts +++ b/x-pack/plugins/ml/public/application/services/ml_api_service/index.ts @@ -62,7 +62,7 @@ export interface BucketSpanEstimatorResponse { name: string; ms: number; error?: boolean; - message?: { msg: string } | string; + message?: string; } export interface GetTimeFieldRangeResponse { diff --git a/x-pack/plugins/ml/public/application/components/messagebar/index.ts b/x-pack/plugins/ml/public/application/services/toast_notification_service/index.ts similarity index 58% rename from x-pack/plugins/ml/public/application/components/messagebar/index.ts rename to x-pack/plugins/ml/public/application/services/toast_notification_service/index.ts index 35130d28a890d..1259f3b47d8e0 100644 --- a/x-pack/plugins/ml/public/application/components/messagebar/index.ts +++ b/x-pack/plugins/ml/public/application/services/toast_notification_service/index.ts @@ -4,4 +4,9 @@ * you may not use this file except in compliance with the Elastic License. */ -export { mlMessageBarService } from './messagebar_service'; +export { + ToastNotificationService, + toastNotificationServiceProvider, + useToastNotificationService, + getToastNotificationService, +} from './toast_notification_service'; diff --git a/x-pack/plugins/ml/public/application/services/toast_notification_service.ts b/x-pack/plugins/ml/public/application/services/toast_notification_service/toast_notification_service.ts similarity index 64% rename from x-pack/plugins/ml/public/application/services/toast_notification_service.ts rename to x-pack/plugins/ml/public/application/services/toast_notification_service/toast_notification_service.ts index 0c11eef0e449c..b552ceae2f9a5 100644 --- a/x-pack/plugins/ml/public/application/services/toast_notification_service.ts +++ b/x-pack/plugins/ml/public/application/services/toast_notification_service/toast_notification_service.ts @@ -4,11 +4,13 @@ * you may not use this file except in compliance with the Elastic License. */ +import { i18n } from '@kbn/i18n'; import { ToastInput, ToastOptions, ToastsStart } from 'kibana/public'; import { useMemo } from 'react'; -import { useNotifications } from '../contexts/kibana'; -import { MLRequestFailure } from '../util/ml_error'; -import { ErrorType, extractErrorProperties } from '../../../common/util/errors'; +import { getToastNotifications } from '../../util/dependency_cache'; +import { useNotifications } from '../../contexts/kibana'; +import { MLRequestFailure } from '../../util/ml_error'; +import { ErrorType, extractErrorProperties } from '../../../../common/util/errors'; export type ToastNotificationService = ReturnType; @@ -21,16 +23,25 @@ export function toastNotificationServiceProvider(toastNotifications: ToastsStart toastNotifications.addSuccess(toastOrTitle, options); } - function displayErrorToast(error: ErrorType, toastTitle: string) { + function displayErrorToast(error: ErrorType, title?: string) { const errorObj = extractErrorProperties(error); toastNotifications.addError(new MLRequestFailure(errorObj, error), { - title: toastTitle, + title: + title ?? + i18n.translate('xpack.ml.toastNotificationService.errorTitle', { + defaultMessage: 'An error has occurred', + }), }); } return { displayDangerToast, displaySuccessToast, displayErrorToast }; } +export function getToastNotificationService() { + const toastNotifications = getToastNotifications(); + return toastNotificationServiceProvider(toastNotifications); +} + /** * Hook to use {@link ToastNotificationService} in React components. */ From 4447901f832a8bbcc5e7a88832b4fb964ca3bc1b Mon Sep 17 00:00:00 2001 From: James Gowdy Date: Wed, 9 Sep 2020 14:45:34 +0100 Subject: [PATCH 11/17] removing test code --- .../multi_job_actions/group_selector/group_selector.js | 2 +- .../components/bucket_span_estimator/estimate_bucket_span.ts | 2 +- x-pack/plugins/ml/public/application/services/job_service.js | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/ml/public/application/jobs/jobs_list/components/multi_job_actions/group_selector/group_selector.js b/x-pack/plugins/ml/public/application/jobs/jobs_list/components/multi_job_actions/group_selector/group_selector.js index ffb2dfe7e495c..a379f49a83159 100644 --- a/x-pack/plugins/ml/public/application/jobs/jobs_list/components/multi_job_actions/group_selector/group_selector.js +++ b/x-pack/plugins/ml/public/application/jobs/jobs_list/components/multi_job_actions/group_selector/group_selector.js @@ -151,7 +151,7 @@ export class GroupSelector extends Component { } } - const tempJobs = newJobs.map((j) => ({ job_id: j.id + 2, groups: j.newGroups })); + const tempJobs = newJobs.map((j) => ({ job_id: j.id, groups: j.newGroups })); ml.jobs .updateGroups(tempJobs) .then((resp) => { diff --git a/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/bucket_span_estimator/estimate_bucket_span.ts b/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/bucket_span_estimator/estimate_bucket_span.ts index ad8c1c6e16037..a87ba4c29baa9 100644 --- a/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/bucket_span_estimator/estimate_bucket_span.ts +++ b/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/pick_fields_step/components/bucket_span_estimator/estimate_bucket_span.ts @@ -45,7 +45,7 @@ export function useEstimateBucketSpan() { (isMultiMetricJobCreator(jobCreator) || isPopulationJobCreator(jobCreator)) && jobCreator.splitField !== null ) { - data.splitField = jobCreator.splitField.id + 33; + data.splitField = jobCreator.splitField.id; } else if (isAdvancedJobCreator(jobCreator)) { jobCreator.richDetectors.some((d) => { if (d.partitionField !== null) { diff --git a/x-pack/plugins/ml/public/application/services/job_service.js b/x-pack/plugins/ml/public/application/services/job_service.js index eb0800f58094b..dfa1b5f4e68cd 100644 --- a/x-pack/plugins/ml/public/application/services/job_service.js +++ b/x-pack/plugins/ml/public/application/services/job_service.js @@ -570,7 +570,7 @@ class JobService { } ml.startDatafeed({ - datafeedId: 22, + datafeedId, start, end, }) From 912c224c978ec1c0d9227eb41e211978274d2879 Mon Sep 17 00:00:00 2001 From: James Gowdy Date: Wed, 9 Sep 2020 14:54:09 +0100 Subject: [PATCH 12/17] updating translations --- x-pack/plugins/translations/translations/ja-JP.json | 5 ----- x-pack/plugins/translations/translations/zh-CN.json | 5 ----- 2 files changed, 10 deletions(-) diff --git a/x-pack/plugins/translations/translations/ja-JP.json b/x-pack/plugins/translations/translations/ja-JP.json index 928596a332158..363a01ca0eee9 100644 --- a/x-pack/plugins/translations/translations/ja-JP.json +++ b/x-pack/plugins/translations/translations/ja-JP.json @@ -10894,7 +10894,6 @@ "xpack.ml.dataframe.analyticsList.deleteAnalyticsErrorMessage": "データフレーム分析ジョブ{analyticsId}の削除中にエラーが発生しました。", "xpack.ml.dataframe.analyticsList.deleteAnalyticsPrivilegeErrorMessage": "ユーザーはインデックス{indexName}を削除する権限がありません。{error}", "xpack.ml.dataframe.analyticsList.deleteAnalyticsSuccessMessage": "データフレーム分析ジョブ{analyticsId}の削除リクエストが受け付けられました。", - "xpack.ml.dataframe.analyticsList.deleteAnalyticsWithIndexErrorMessage": "ディスティネーションインデックス{destinationIndex}の削除中にエラーが発生しました。{error}", "xpack.ml.dataframe.analyticsList.deleteAnalyticsWithIndexPatternErrorMessage": "インデックスパターン{destinationIndex}の削除中にエラーが発生しました。{error}", "xpack.ml.dataframe.analyticsList.deleteAnalyticsWithIndexPatternSuccessMessage": "インデックスパターン{destinationIndex}を削除する要求が確認されました。", "xpack.ml.dataframe.analyticsList.deleteAnalyticsWithIndexSuccessMessage": "ディスティネーションインデックス{destinationIndex}を削除する要求が確認されました。", @@ -11362,11 +11361,8 @@ "xpack.ml.jobService.activeDatafeedsLabel": "アクティブなデータフィード", "xpack.ml.jobService.activeMLNodesLabel": "アクティブな ML ノード", "xpack.ml.jobService.closedJobsLabel": "ジョブを作成", - "xpack.ml.jobService.couldNotStartDatafeedErrorMessage": "{jobId} のデータフィードを開始できませんでした", - "xpack.ml.jobService.couldNotStopDatafeedErrorMessage": "{jobId} のデータフィードを停止できませんでした", "xpack.ml.jobService.failedJobsLabel": "失敗したジョブ", "xpack.ml.jobService.openJobsLabel": "ジョブを開く", - "xpack.ml.jobService.requestMayHaveTimedOutErrorMessage": "リクエストがタイムアウトし、まだバックグラウンドで実行中の可能性があります。", "xpack.ml.jobService.totalJobsLabel": "合計ジョブ数", "xpack.ml.jobService.validateJobErrorTitle": "ジョブ検証エラー", "xpack.ml.jobsList.actionExecuteSuccessfullyNotificationMessage": "{successesJobsCount, plural, one{{successJob}} other{# 件のジョブ}} {actionTextPT}成功", @@ -11572,7 +11568,6 @@ "xpack.ml.maxFileSizeSettingsDescription": "ファイルデータビジュアライザーでデータをインポートするときのファイルサイズ上限を設定します。この設定でサポートされている最大値は1 GBです。", "xpack.ml.maxFileSizeSettingsError": "200 MB、1 GBなどの有効なデータサイズにしてください。", "xpack.ml.maxFileSizeSettingsName": "ファイルデータビジュアライザーの最大ファイルアップロードサイズ", - "xpack.ml.messagebarService.errorTitle": "エラーが発生しました", "xpack.ml.models.jobService.allOtherRequestsCancelledDescription": " 他のすべてのリクエストはキャンセルされました。", "xpack.ml.models.jobService.categorization.messages.failureToGetTokens": "フィールド値の例のサンプルをトークン化することができませんでした。{message}", "xpack.ml.models.jobService.categorization.messages.insufficientPrivileges": "権限が不十分なため、フィールド値の例のトークン化を実行できませんでした。そのため、フィールド値を確認し、カテゴリー分けジョブでの使用が適当かを確認することができません。", diff --git a/x-pack/plugins/translations/translations/zh-CN.json b/x-pack/plugins/translations/translations/zh-CN.json index 3dcb3c507a8c9..ac8e34e397438 100644 --- a/x-pack/plugins/translations/translations/zh-CN.json +++ b/x-pack/plugins/translations/translations/zh-CN.json @@ -10900,7 +10900,6 @@ "xpack.ml.dataframe.analyticsList.deleteAnalyticsErrorMessage": "删除数据帧分析作业 {analyticsId} 时发生错误", "xpack.ml.dataframe.analyticsList.deleteAnalyticsPrivilegeErrorMessage": "用户无权删除索引 {indexName}：{error}", "xpack.ml.dataframe.analyticsList.deleteAnalyticsSuccessMessage": "删除的数据帧分析作业 {analyticsId} 的请求已确认。", - "xpack.ml.dataframe.analyticsList.deleteAnalyticsWithIndexErrorMessage": "删除目标索引 {destinationIndex} 时发生错误：{error}", "xpack.ml.dataframe.analyticsList.deleteAnalyticsWithIndexPatternErrorMessage": "删除索引模式 {destinationIndex} 时发生错误：{error}", "xpack.ml.dataframe.analyticsList.deleteAnalyticsWithIndexPatternSuccessMessage": "删除索引模式 {destinationIndex} 的请求已确认。", "xpack.ml.dataframe.analyticsList.deleteAnalyticsWithIndexSuccessMessage": "删除目标索引 {destinationIndex} 的请求已确认。", @@ -11369,11 +11368,8 @@ "xpack.ml.jobService.activeDatafeedsLabel": "活动数据馈送", "xpack.ml.jobService.activeMLNodesLabel": "活动 ML 节点", "xpack.ml.jobService.closedJobsLabel": "已关闭的作业", - "xpack.ml.jobService.couldNotStartDatafeedErrorMessage": "无法开始 {jobId} 的数据馈送", - "xpack.ml.jobService.couldNotStopDatafeedErrorMessage": "无法停止 {jobId} 的数据馈送", "xpack.ml.jobService.failedJobsLabel": "失败的作业", "xpack.ml.jobService.openJobsLabel": "打开的作业", - "xpack.ml.jobService.requestMayHaveTimedOutErrorMessage": "请求可能已超时，并可能仍在后台运行。", "xpack.ml.jobService.totalJobsLabel": "总计作业数", "xpack.ml.jobService.validateJobErrorTitle": "作业验证错误", "xpack.ml.jobsList.actionExecuteSuccessfullyNotificationMessage": "{successesJobsCount, plural, one{{successJob}} other{# 个作业}}{actionTextPT}已成功", @@ -11579,7 +11575,6 @@ "xpack.ml.maxFileSizeSettingsDescription": "设置在文件数据可视化工具中导入数据时的文件大小限制。此设置支持的最高值为 1GB。", "xpack.ml.maxFileSizeSettingsError": "应为有效的数据大小。如 200MB、1GB", "xpack.ml.maxFileSizeSettingsName": "文件数据可视化工具最大文件上传大小", - "xpack.ml.messagebarService.errorTitle": "发生了错误", "xpack.ml.models.jobService.allOtherRequestsCancelledDescription": " 所有其他请求已取消。", "xpack.ml.models.jobService.categorization.messages.failureToGetTokens": "无法对示例字段值样本进行分词。{message}", "xpack.ml.models.jobService.categorization.messages.insufficientPrivileges": "由于权限不足，无法对字段值示例执行分词。因此，无法检查字段值是否适合用于归类作业。", From 9027b2aad6a7e788410528121288096a8e69b44c Mon Sep 17 00:00:00 2001 From: James Gowdy Date: Wed, 9 Sep 2020 15:59:06 +0100 Subject: [PATCH 13/17] combining job creation error handling --- .../pages/components/summary_step/summary.tsx | 31 ++++++++----------- 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/summary_step/summary.tsx b/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/summary_step/summary.tsx index 8948a1a6f6a85..3000ce8449138 100644 --- a/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/summary_step/summary.tsx +++ b/x-pack/plugins/ml/public/application/jobs/new_job/pages/components/summary_step/summary.tsx @@ -72,15 +72,7 @@ export const SummaryStep: FC = ({ setCurrentStep, isCurrentStep }) => const jr = await jobCreator.createAndStartJob(); setJobRunner(jr); } catch (error) { - // catch and display all job creation errors - const { displayErrorToast } = toastNotificationServiceProvider(notifications.toasts); - displayErrorToast( - error, - i18n.translate('xpack.ml.newJob.wizard.summaryStep.createJobError', { - defaultMessage: `Job creation error`, - }) - ); - setCreatingJob(false); + handleJobCreationError(error); } } @@ -91,18 +83,21 @@ export const SummaryStep: FC = ({ setCurrentStep, isCurrentStep }) => await jobCreator.createDatafeed(); advancedStartDatafeed(jobCreator, navigateToPath); } catch (error) { - // catch and display all job creation errors - const { displayErrorToast } = toastNotificationServiceProvider(notifications.toasts); - displayErrorToast( - error, - i18n.translate('xpack.ml.newJob.wizard.summaryStep.createJobError', { - defaultMessage: `Job creation error`, - }) - ); - setCreatingJob(false); + handleJobCreationError(error); } } + function handleJobCreationError(error: any) { + const { displayErrorToast } = toastNotificationServiceProvider(notifications.toasts); + displayErrorToast( + error, + i18n.translate('xpack.ml.newJob.wizard.summaryStep.createJobError', { + defaultMessage: `Job creation error`, + }) + ); + setCreatingJob(false); + } + function viewResults() { const url = mlJobService.createResultsUrl( [jobCreator.jobId], From 430dbdb5da30a5bb27f20e979e026378afdfcb66 Mon Sep 17 00:00:00 2001 From: James Gowdy Date: Wed, 9 Sep 2020 16:52:44 +0100 Subject: [PATCH 14/17] refactoring error files --- .../common/util/{ => errors}/errors.test.ts | 2 +- x-pack/plugins/ml/common/util/errors/index.ts | 20 ++++++ .../{errors.ts => errors/process_errors.ts} | 61 +++---------------- .../util/errors/request_error.ts} | 11 ++-- x-pack/plugins/ml/common/util/errors/types.ts | 59 ++++++++++++++++++ .../explorer_chart_config_builder.test.js | 2 - .../toast_notification_service.ts | 7 ++- 7 files changed, 98 insertions(+), 64 deletions(-) rename x-pack/plugins/ml/common/util/{ => errors}/errors.test.ts (98%) create mode 100644 x-pack/plugins/ml/common/util/errors/index.ts rename x-pack/plugins/ml/common/util/{errors.ts => errors/process_errors.ts} (63%) rename x-pack/plugins/ml/{public/application/util/ml_error.ts => common/util/errors/request_error.ts} (65%) create mode 100644 x-pack/plugins/ml/common/util/errors/types.ts diff --git a/x-pack/plugins/ml/common/util/errors.test.ts b/x-pack/plugins/ml/common/util/errors/errors.test.ts similarity index 98% rename from x-pack/plugins/ml/common/util/errors.test.ts rename to x-pack/plugins/ml/common/util/errors/errors.test.ts index 39dacbde70472..ddaf0b04dd12b 100644 --- a/x-pack/plugins/ml/common/util/errors.test.ts +++ b/x-pack/plugins/ml/common/util/errors/errors.test.ts @@ -6,7 +6,7 @@ import Boom from 'boom'; -import { extractErrorMessage, MLHttpFetchError, MLResponseError, EsErrorBody } from './errors'; +import { extractErrorMessage, MLHttpFetchError, MLResponseError, EsErrorBody } from './index'; describe('ML - error message utils', () => { describe('extractErrorMessage', () => { diff --git a/x-pack/plugins/ml/common/util/errors/index.ts b/x-pack/plugins/ml/common/util/errors/index.ts new file mode 100644 index 0000000000000..d6d33b86e6fa9 --- /dev/null +++ b/x-pack/plugins/ml/common/util/errors/index.ts @@ -0,0 +1,20 @@ +/* + * 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 { MLRequestFailure } from './request_error'; +export { extractErrorMessage, extractErrorProperties } from './process_errors'; +export { + ErrorType, + EsErrorBody, + EsErrorRootCause, + MLErrorObject, + MLHttpFetchError, + MLResponseError, + isBoomError, + isErrorString, + isEsErrorBody, + isMLResponseError, +} from './types'; diff --git a/x-pack/plugins/ml/common/util/errors.ts b/x-pack/plugins/ml/common/util/errors/process_errors.ts similarity index 63% rename from x-pack/plugins/ml/common/util/errors.ts rename to x-pack/plugins/ml/common/util/errors/process_errors.ts index fd802961def76..4addfa414ef56 100644 --- a/x-pack/plugins/ml/common/util/errors.ts +++ b/x-pack/plugins/ml/common/util/errors/process_errors.ts @@ -4,59 +4,14 @@ * you may not use this file except in compliance with the Elastic License. */ -import { HttpFetchError } from 'kibana/public'; -import Boom from 'boom'; - -export interface EsErrorRootCause { - type: string; - reason: string; -} - -export interface EsErrorBody { - error: { - root_cause?: EsErrorRootCause[]; - type: string; - reason: string; - }; - status: number; -} - -export interface MLResponseError { - statusCode: number; - error: string; - message: string; - attributes?: { - body: EsErrorBody; - }; -} - -export interface MLHttpFetchError extends HttpFetchError { - body: T; -} - -export type ErrorType = MLHttpFetchError | EsErrorBody | Boom | string | undefined; - -function isEsErrorBody(error: any): error is EsErrorBody { - return error && error.error?.reason !== undefined; -} - -function isErrorString(error: any): error is string { - return typeof error === 'string'; -} - -function isMLResponseError(error: any): error is MLResponseError { - return typeof error.body === 'object' && 'message' in error.body; -} - -function isBoomError(error: any): error is Boom { - return error.isBoom === true; -} - -export interface MLErrorObject { - message: string; - statusCode?: number; - fullError?: EsErrorBody; -} +import { + ErrorType, + MLErrorObject, + isBoomError, + isErrorString, + isEsErrorBody, + isMLResponseError, +} from './types'; export const extractErrorProperties = (error: ErrorType): MLErrorObject => { // extract properties of the error object from within the response error diff --git a/x-pack/plugins/ml/public/application/util/ml_error.ts b/x-pack/plugins/ml/common/util/errors/request_error.ts similarity index 65% rename from x-pack/plugins/ml/public/application/util/ml_error.ts rename to x-pack/plugins/ml/common/util/errors/request_error.ts index bd4c9c0f7dcd1..faa8a8e2dc0d4 100644 --- a/x-pack/plugins/ml/public/application/util/ml_error.ts +++ b/x-pack/plugins/ml/common/util/errors/request_error.ts @@ -4,15 +4,14 @@ * you may not use this file except in compliance with the Elastic License. */ -import { KbnError } from '../../../../../../src/plugins/kibana_utils/public'; +import { MLErrorObject, ErrorType } from './types'; -export class MLRequestFailure extends KbnError { - // takes an Error object and and optional response object - - constructor(error: any, resp: any) { +export class MLRequestFailure extends Error { + constructor(error: MLErrorObject, resp: ErrorType) { super(error.message); + Object.setPrototypeOf(this, new.target.prototype); - if (typeof resp === 'object') { + if (typeof resp !== 'string' && typeof resp !== 'undefined') { if ('body' in resp) { this.stack = JSON.stringify(resp.body, null, 2); } else { diff --git a/x-pack/plugins/ml/common/util/errors/types.ts b/x-pack/plugins/ml/common/util/errors/types.ts new file mode 100644 index 0000000000000..ee6104efc13ea --- /dev/null +++ b/x-pack/plugins/ml/common/util/errors/types.ts @@ -0,0 +1,59 @@ +/* + * 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. + */ + +import { HttpFetchError } from 'kibana/public'; +import Boom from 'boom'; + +export interface EsErrorRootCause { + type: string; + reason: string; +} + +export interface EsErrorBody { + error: { + root_cause?: EsErrorRootCause[]; + type: string; + reason: string; + }; + status: number; +} + +export interface MLResponseError { + statusCode: number; + error: string; + message: string; + attributes?: { + body: EsErrorBody; + }; +} + +export interface MLErrorObject { + message: string; + statusCode?: number; + fullError?: EsErrorBody; +} + +export interface MLHttpFetchError extends HttpFetchError { + body: T; +} + +export type ErrorType = MLHttpFetchError | EsErrorBody | Boom | string | undefined; + +export function isEsErrorBody(error: any): error is EsErrorBody { + return error && error.error?.reason !== undefined; +} + +export function isErrorString(error: any): error is string { + return typeof error === 'string'; +} + +export function isMLResponseError(error: any): error is MLResponseError { + return typeof error.body === 'object' && 'message' in error.body; +} + +export function isBoomError(error: any): error is Boom { + return error.isBoom === true; +} diff --git a/x-pack/plugins/ml/public/application/explorer/explorer_charts/explorer_chart_config_builder.test.js b/x-pack/plugins/ml/public/application/explorer/explorer_charts/explorer_chart_config_builder.test.js index d705e47a5e906..58adf3d892f66 100644 --- a/x-pack/plugins/ml/public/application/explorer/explorer_charts/explorer_chart_config_builder.test.js +++ b/x-pack/plugins/ml/public/application/explorer/explorer_charts/explorer_chart_config_builder.test.js @@ -8,8 +8,6 @@ import mockAnomalyRecord from './__mocks__/mock_anomaly_record.json'; import mockDetectorsByJob from './__mocks__/mock_detectors_by_job.json'; import mockJobConfig from './__mocks__/mock_job_config.json'; -jest.mock('../../util/ml_error', () => class MLRequestFailure {}); - jest.mock('../../services/job_service', () => ({ mlJobService: { getJob() { diff --git a/x-pack/plugins/ml/public/application/services/toast_notification_service/toast_notification_service.ts b/x-pack/plugins/ml/public/application/services/toast_notification_service/toast_notification_service.ts index b552ceae2f9a5..6f11c221fe78b 100644 --- a/x-pack/plugins/ml/public/application/services/toast_notification_service/toast_notification_service.ts +++ b/x-pack/plugins/ml/public/application/services/toast_notification_service/toast_notification_service.ts @@ -9,8 +9,11 @@ import { ToastInput, ToastOptions, ToastsStart } from 'kibana/public'; import { useMemo } from 'react'; import { getToastNotifications } from '../../util/dependency_cache'; import { useNotifications } from '../../contexts/kibana'; -import { MLRequestFailure } from '../../util/ml_error'; -import { ErrorType, extractErrorProperties } from '../../../../common/util/errors'; +import { + ErrorType, + extractErrorProperties, + MLRequestFailure, +} from '../../../../common/util/errors'; export type ToastNotificationService = ReturnType; From d6d57b86c364e925ef3c6f1affa29568f5c3a199 Mon Sep 17 00:00:00 2001 From: James Gowdy Date: Wed, 9 Sep 2020 20:58:35 +0100 Subject: [PATCH 15/17] updating test --- .../test/api_integration/apis/ml/data_frame_analytics/delete.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/test/api_integration/apis/ml/data_frame_analytics/delete.ts b/x-pack/test/api_integration/apis/ml/data_frame_analytics/delete.ts index c6043b7a282d4..53a9d9e790d67 100644 --- a/x-pack/test/api_integration/apis/ml/data_frame_analytics/delete.ts +++ b/x-pack/test/api_integration/apis/ml/data_frame_analytics/delete.ts @@ -120,7 +120,7 @@ export default ({ getService }: FtrProviderContext) => { .expect(404); expect(body.error).to.eql('Not Found'); - expect(body.message).to.eql('Not Found'); + expect(body.message).to.eql('resource_not_found_exception'); }); describe('with deleteDestIndex setting', function () { From e9848afc45bc01e4d137e346906d2d0fb04988e0 Mon Sep 17 00:00:00 2001 From: James Gowdy Date: Wed, 9 Sep 2020 21:21:23 +0100 Subject: [PATCH 16/17] fixing bug in DFA deletion --- .../ml/server/routes/data_frame_analytics.ts | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/x-pack/plugins/ml/server/routes/data_frame_analytics.ts b/x-pack/plugins/ml/server/routes/data_frame_analytics.ts index 999613fdb70f6..526c3477cc5b2 100644 --- a/x-pack/plugins/ml/server/routes/data_frame_analytics.ts +++ b/x-pack/plugins/ml/server/routes/data_frame_analytics.ts @@ -324,19 +324,20 @@ export function dataFrameAnalyticsRoutes({ router, mlLicense }: RouteInitializat success: false, }; - // Check if analyticsId is valid and get destination index - if (deleteDestIndex || deleteDestIndexPattern) { - try { - const { body } = await client.asInternalUser.ml.getDataFrameAnalytics({ - id: analyticsId, - }); - if (Array.isArray(body.data_frame_analytics) && body.data_frame_analytics.length > 0) { - destinationIndex = body.data_frame_analytics[0].dest.index; - } - } catch (e) { - return response.customError(wrapError(e)); + try { + // Check if analyticsId is valid and get destination index + const { body } = await client.asInternalUser.ml.getDataFrameAnalytics({ + id: analyticsId, + }); + if (Array.isArray(body.data_frame_analytics) && body.data_frame_analytics.length > 0) { + destinationIndex = body.data_frame_analytics[0].dest.index; } + } catch (e) { + // exist early if the job doesn't exist + return response.customError(wrapError(e)); + } + if (deleteDestIndex || deleteDestIndexPattern) { // If user checks box to delete the destinationIndex associated with the job if (destinationIndex && deleteDestIndex) { // Verify if user has privilege to delete the destination index From 8875cfa888c07bf9e0f267fd734787afbe934b22 Mon Sep 17 00:00:00 2001 From: James Gowdy Date: Thu, 10 Sep 2020 17:29:03 +0100 Subject: [PATCH 17/17] improving mml warning --- x-pack/plugins/ml/common/util/errors/types.ts | 1 + .../common/job_creator/util/model_memory_estimator.ts | 11 ++++++----- .../toast_notification_service.ts | 6 +++++- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/x-pack/plugins/ml/common/util/errors/types.ts b/x-pack/plugins/ml/common/util/errors/types.ts index ee6104efc13ea..3d9fc82be8d85 100644 --- a/x-pack/plugins/ml/common/util/errors/types.ts +++ b/x-pack/plugins/ml/common/util/errors/types.ts @@ -15,6 +15,7 @@ export interface EsErrorRootCause { export interface EsErrorBody { error: { root_cause?: EsErrorRootCause[]; + caused_by?: EsErrorRootCause; type: string; reason: string; }; diff --git a/x-pack/plugins/ml/public/application/jobs/new_job/common/job_creator/util/model_memory_estimator.ts b/x-pack/plugins/ml/public/application/jobs/new_job/common/job_creator/util/model_memory_estimator.ts index 0011c88d2b524..6671aaa83abe0 100644 --- a/x-pack/plugins/ml/public/application/jobs/new_job/common/job_creator/util/model_memory_estimator.ts +++ b/x-pack/plugins/ml/public/application/jobs/new_job/common/job_creator/util/model_memory_estimator.ts @@ -23,7 +23,7 @@ import { useEffect, useMemo } from 'react'; import { DEFAULT_MODEL_MEMORY_LIMIT } from '../../../../../../../common/constants/new_job'; import { ml } from '../../../../../services/ml_api_service'; import { JobValidator, VALIDATION_DELAY_MS } from '../../job_validator/job_validator'; -import { ErrorResponse } from '../../../../../../../common/types/errors'; +import { MLHttpFetchError, MLResponseError } from '../../../../../../../common/util/errors'; import { useMlKibana } from '../../../../../contexts/kibana'; import { JobCreator } from '../job_creator'; @@ -36,10 +36,10 @@ export const modelMemoryEstimatorProvider = ( jobValidator: JobValidator ) => { const modelMemoryCheck$ = new Subject(); - const error$ = new Subject(); + const error$ = new Subject>(); return { - get error$(): Observable { + get error$(): Observable> { return error$.asObservable(); }, get updates$(): Observable { @@ -64,7 +64,7 @@ export const modelMemoryEstimatorProvider = ( catchError((error) => { // eslint-disable-next-line no-console console.error('Model memory limit could not be calculated', error.body); - error$.next(error.body); + error$.next(error); // fallback to the default in case estimation failed return of(DEFAULT_MODEL_MEMORY_LIMIT); }) @@ -120,7 +120,8 @@ export const useModelMemoryEstimator = ( title: i18n.translate('xpack.ml.newJob.wizard.estimateModelMemoryError', { defaultMessage: 'Model memory limit could not be calculated', }), - text: error.message, + text: + error.body.attributes?.body.error.caused_by?.reason || error.body.message || undefined, }); }) ); diff --git a/x-pack/plugins/ml/public/application/services/toast_notification_service/toast_notification_service.ts b/x-pack/plugins/ml/public/application/services/toast_notification_service/toast_notification_service.ts index 6f11c221fe78b..61e0480313ebe 100644 --- a/x-pack/plugins/ml/public/application/services/toast_notification_service/toast_notification_service.ts +++ b/x-pack/plugins/ml/public/application/services/toast_notification_service/toast_notification_service.ts @@ -22,6 +22,10 @@ export function toastNotificationServiceProvider(toastNotifications: ToastsStart toastNotifications.addDanger(toastOrTitle, options); } + function displayWarningToast(toastOrTitle: ToastInput, options?: ToastOptions) { + toastNotifications.addWarning(toastOrTitle, options); + } + function displaySuccessToast(toastOrTitle: ToastInput, options?: ToastOptions) { toastNotifications.addSuccess(toastOrTitle, options); } @@ -37,7 +41,7 @@ export function toastNotificationServiceProvider(toastNotifications: ToastsStart }); } - return { displayDangerToast, displaySuccessToast, displayErrorToast }; + return { displayDangerToast, displayWarningToast, displaySuccessToast, displayErrorToast }; } export function getToastNotificationService() {