[Rollups] Improve Rollup Job Wizard error handling (#25092) (#25171)

* Consistently format errors with showApiWarning and showApiError helpers.
* Use fatalError for unexpected errors.
* Localize errors.
* Surface error in Job List when the jobs can't be loaded.
This commit is contained in:
CJ Cenizal 2018-11-05 16:49:04 -08:00 committed by GitHub
parent a221b6ebbd
commit 655b09c685
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 168 additions and 33 deletions

View file

@ -9,7 +9,7 @@ import PropTypes from 'prop-types';
import { Switch, Route, Redirect } from 'react-router-dom';
import { CRUD_APP_BASE_PATH } from './constants';
import { registerRouter } from './services';
import { registerRouter, setUserHasLeftApp } from './services';
import { JobList, JobCreate } from './sections';
export class App extends Component {
@ -33,6 +33,11 @@ export class App extends Component {
registerRouter(router);
}
componentWillUnmount() {
// Set internal flag so we can prevent reacting to route changes internally.
setUserHasLeftApp(true);
}
render() {
return (
<div>

View file

@ -267,7 +267,9 @@ export class JobCreateUi extends Component {
// This error isn't an HTTP error, so let the fatal error screen tell the user something
// unexpected happened.
fatalError(error, 'Rollup Job Wizard index pattern validation');
fatalError(error, i18n.translate('xpack.rollupJobs.create.errors.indexPatternValidationFatalErrorTitle', {
defaultMessage: 'Rollup Job Wizard index pattern validation',
}));
});
}, 300);

View file

@ -127,6 +127,34 @@ export class JobListUi extends Component {
);
}
renderError(error) {
// We can safely depend upon the shape of this error coming from Angular $http, because we
// handle unexpected error shapes in the API action.
const {
statusCode,
error: errorString,
} = error.data;
const { intl } = this.props;
const title = intl.formatMessage({
id: 'xpack.rollupJobs.jobList.loadingErrorTitle',
defaultMessage: 'Error loading rollup jobs',
});
return (
<Fragment>
{this.getHeaderSection()}
<EuiSpacer size="m" />
<EuiCallOut
title={title}
color="danger"
iconType="alert"
>
{statusCode} {errorString}
</EuiCallOut>
</Fragment>
);
}
renderEmpty() {
return (
<EuiEmptyPrompt
@ -224,8 +252,13 @@ export class JobListUi extends Component {
const { isLoading, jobs, jobLoadError } = this.props;
let content;
if (jobLoadError && jobLoadError.status === 403) {
content = this.renderNoPermission();
if (jobLoadError) {
if (jobLoadError.status === 403) {
content = this.renderNoPermission();
} else {
content = this.renderError(jobLoadError);
}
} else if (!isLoading && !jobs.length) {
content = this.renderEmpty();
} else {

View file

@ -0,0 +1,42 @@
/*
* 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 { fatalError, toastNotifications } from 'ui/notify';
function createToastConfig(error, errorTitle) {
// Expect an error in the shape provided by Angular's $http service.
if (error && error.data) {
const { error: errorString, statusCode, message } = error.data;
return {
title: errorTitle,
text: `${statusCode}: ${errorString}. ${message}`,
};
}
}
export function showApiWarning(error, errorTitle) {
const toastConfig = createToastConfig(error, errorTitle);
if (toastConfig) {
return toastNotifications.addWarning(toastConfig);
}
// This error isn't an HTTP error, so let the fatal error screen tell the user something
// unexpected happened.
return fatalError(error, errorTitle);
}
export function showApiError(error, errorTitle) {
const toastConfig = createToastConfig(error, errorTitle);
if (toastConfig) {
return toastNotifications.addDanger(toastConfig);
}
// This error isn't an HTTP error, so let the fatal error screen tell the user something
// unexpected happened.
fatalError(error, errorTitle);
}

View file

@ -13,6 +13,11 @@ export {
validateIndexPattern,
} from './api';
export {
showApiError,
showApiWarning,
} from './api_errors';
export {
cronExpressionToParts,
cronPartsToExpression,

View file

@ -4,11 +4,13 @@
* you may not use this file except in compliance with the Elastic License.
*/
import { toastNotifications } from 'ui/notify';
import { i18n } from '@kbn/i18n';
import {
startJobs as sendStartJobsRequest,
stopJobs as sendStopJobsRequest,
createNoticeableDelay,
showApiError,
} from '../../services';
import {
@ -31,7 +33,9 @@ export const startJobs = (jobIds) => async (dispatch) => {
type: UPDATE_JOB_FAILURE,
});
return toastNotifications.addDanger(error.data.message);
return showApiError(error, i18n.translate('xpack.rollupJobs.startJobsAction.errorTitle', {
defaultMessage: 'Error starting rollup jobs',
}));
}
dispatch({
@ -53,7 +57,9 @@ export const stopJobs = (jobIds) => async (dispatch) => {
type: UPDATE_JOB_FAILURE,
});
return toastNotifications.addDanger(error.data.message);
return showApiError(error, i18n.translate('xpack.rollupJobs.stopJobsAction.errorTitle', {
defaultMessage: 'Error stopping rollup jobs',
}));
}
dispatch({

View file

@ -4,6 +4,9 @@
* you may not use this file except in compliance with the Elastic License.
*/
import { i18n } from '@kbn/i18n';
import { fatalError } from 'ui/notify';
import { CRUD_APP_BASE_PATH } from '../../constants';
import {
createJob as sendCreateJobRequest,
@ -33,34 +36,46 @@ export const createJob = (jobConfig) => async (dispatch) => {
new Promise(resolve => setTimeout(resolve, 500)),
]);
} catch (error) {
const { statusCode, data } = error;
if (error) {
const { statusCode, data } = error;
// Some errors have statusCode directly available but some are under a data property.
switch (statusCode || data.statusCode) {
case 409:
dispatch({
type: CREATE_JOB_FAILURE,
payload: {
error: {
message: `A job with ID '${jobConfig.id}' already exists.`,
// Expect an error in the shape provided by Angular's $http service.
if (data) {
// Some errors have statusCode directly available but some are under a data property.
if ((statusCode || (data && data.statusCode)) === 409) {
return dispatch({
type: CREATE_JOB_FAILURE,
payload: {
error: {
message: i18n.translate('xpack.rollupJobs.createAction.jobIdAlreadyExistsErrorMessage', {
defaultMessage: `A job with ID '{jobConfigId}' already exists.`,
values: { jobConfigId: jobConfig.id },
}),
},
},
},
});
break;
});
}
default:
dispatch({
return dispatch({
type: CREATE_JOB_FAILURE,
payload: {
error: {
message: `Request failed with a ${statusCode} error. ${data.message}`,
message: i18n.translate('xpack.rollupJobs.createAction.failedDefaultErrorMessage', {
defaultMessage: 'Request failed with a {statusCode} error. {message}',
values: { statusCode, message: data.message },
}),
cause: data.cause,
},
},
});
}
}
return;
// This error isn't an HTTP error, so let the fatal error screen tell the user something
// unexpected happened.
return fatalError(error, i18n.translate('xpack.rollupJobs.createAction.errorTitle', {
defaultMessage: 'Error creating rollup job',
}));
}
dispatch({

View file

@ -4,9 +4,14 @@
* you may not use this file except in compliance with the Elastic License.
*/
import { i18n } from '@kbn/i18n';
import { toastNotifications } from 'ui/notify';
import { deleteJobs as sendDeleteJobsRequest, createNoticeableDelay } from '../../services';
import {
deleteJobs as sendDeleteJobsRequest,
createNoticeableDelay,
showApiError,
} from '../../services';
import { getDetailPanelJob } from '../selectors';
import {
@ -30,13 +35,21 @@ export const deleteJobs = (jobIds) => async (dispatch, getState) => {
type: UPDATE_JOB_FAILURE,
});
return toastNotifications.addDanger(error.data.message);
return showApiError(error, i18n.translate('xpack.rollupJobs.deleteAction.errorTitle', {
defaultMessage: 'Error deleting rollup jobs',
}));
}
if (jobIds.length === 1) {
toastNotifications.addSuccess(`Rollup job '${jobIds[0]}' was deleted`);
toastNotifications.addSuccess(i18n.translate('xpack.rollupJobs.deleteAction.successSingleNotificationTitle', {
defaultMessage: `Rollup job '{jobId}' was deleted`,
values: { jobId: jobIds[0] },
}));
} else {
toastNotifications.addSuccess(`${jobIds.length} rollup jobs were deleted`);
toastNotifications.addSuccess(i18n.translate('xpack.rollupJobs.deleteAction.successMultipleNotificationTitle', {
defaultMessage: '{count} rollup jobs were deleted',
values: { count: jobIds.length },
}));
}
// If we've just deleted a job we were looking at, we need to close the panel.

View file

@ -4,8 +4,13 @@
* you may not use this file except in compliance with the Elastic License.
*/
import { toastNotifications } from 'ui/notify';
import { loadJobs as sendLoadJobsRequest, deserializeJobs } from '../../services';
import { i18n } from '@kbn/i18n';
import {
loadJobs as sendLoadJobsRequest,
deserializeJobs,
showApiError,
} from '../../services';
import {
LOAD_JOBS_START,
LOAD_JOBS_SUCCESS,
@ -26,7 +31,9 @@ export const loadJobs = () => async (dispatch) => {
payload: { error }
});
return toastNotifications.addDanger(error.data.message);
return showApiError(error, i18n.translate('xpack.rollupJobs.loadAction.errorTitle', {
defaultMessage: 'Error loading rollup jobs',
}));
}
dispatch({

View file

@ -4,8 +4,13 @@
* you may not use this file except in compliance with the Elastic License.
*/
import { toastNotifications } from 'ui/notify';
import { loadJobs as sendLoadJobsRequest, deserializeJobs } from '../../services';
import { i18n } from '@kbn/i18n';
import {
loadJobs as sendLoadJobsRequest,
deserializeJobs,
showApiWarning,
} from '../../services';
import {
REFRESH_JOBS_SUCCESS,
} from '../action_types';
@ -15,7 +20,9 @@ export const refreshJobs = () => async (dispatch) => {
try {
jobs = await sendLoadJobsRequest();
} catch (error) {
return toastNotifications.addWarning(error.data.message);
return showApiWarning(error, i18n.translate('xpack.rollupJobs.refreshAction.errorTitle', {
defaultMessage: 'Error refreshing rollup jobs',
}));
}
dispatch({