Skip to content

Commit

Permalink
Add SearchError for surfacing courier search errors.
Browse files Browse the repository at this point in the history
- Use toastNotifications to surface errors in visualization editor.
- Add call-out react directive and use it to surface rollup errors in the visualization editor sidebar.
- Temporarily assign timezone and interval values from rollup job to the search to avoid errors..
  • Loading branch information
cjcenizal committed Jul 23, 2018
1 parent de444d5 commit 43f59dd
Show file tree
Hide file tree
Showing 12 changed files with 210 additions and 44 deletions.
12 changes: 9 additions & 3 deletions src/ui/public/courier/fetch/fetch_now.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import { CallClientProvider } from './call_client';
import { CallResponseHandlersProvider } from './call_response_handlers';
import { ContinueIncompleteProvider } from './continue_incomplete';
import { RequestStatus } from './req_status';
import { location } from './notifier';

/**
* Fetch now provider should be used if you want the results searched and returned immediately.
Expand Down Expand Up @@ -53,7 +52,10 @@ export function FetchNowProvider(Private, Promise) {

return searchRequest.retry();
}))
.catch(error => fatalError(error, location));
.catch(error => {
// If any errors occur after the search requests have resolved, then we kill Kibana.
fatalError(error, 'Courier fetch');
});
}

function fetchSearchResults(searchRequests) {
Expand All @@ -71,7 +73,11 @@ export function FetchNowProvider(Private, Promise) {
return startRequests(searchRequests)
.then(function () {
replaceAbortedRequests();
return callClient(searchRequests);
return callClient(searchRequests)
.catch(() => {
// Silently swallow errors that result from search requests so the consumer can surface
// them as notifications instead of courier forcing fatal errors.
});
})
.then(function (responses) {
replaceAbortedRequests();
Expand Down
2 changes: 2 additions & 0 deletions src/ui/public/courier/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ export {
} from './search_source';

export {
addSearchStrategy,
hasSearchStategyForIndexPattern,
isDefaultTypeIndexPattern,
SearchError,
} from './search_strategy';
23 changes: 20 additions & 3 deletions src/ui/public/courier/search_strategy/default_search_strategy.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import { addSearchStrategy } from './search_strategy_registry';
import { isDefaultTypeIndexPattern } from './is_default_type_index_pattern';
import { SearchError } from './search_error';

function getAllFetchParams(searchRequests, Promise) {
return Promise.map(searchRequests, (searchRequest) => {
Expand Down Expand Up @@ -71,14 +72,30 @@ export const defaultSearchStrategy = {
const searching = es.msearch({ body: serializedFetchParams });

return {
// Unwrap the responses object returned by the es client.
searching: searching.then(({ responses }) => responses),
// Munge data into shape expected by consumer.
searching: new Promise((resolve, reject) => {
// Unwrap the responses object returned by the ES client.
searching.then(({ responses }) => {
resolve(responses);
}).catch(error => {
// Format ES client error as a SearchError.
const { statusCode, displayName, message, path } = error;

const searchError = new SearchError({
status: statusCode,
title: displayName,
message,
path,
});

reject(searchError);
});
}),
abort: searching.abort,
failedSearchRequests,
};
},

// Accept multiple criteria for determining viability to be as flexible as possible.
isViable: (indexPattern) => {
if (!indexPattern) {
return false;
Expand Down
2 changes: 2 additions & 0 deletions src/ui/public/courier/search_strategy/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,5 @@ export {
} from './search_strategy_registry';

export { isDefaultTypeIndexPattern } from './is_default_type_index_pattern';

export { SearchError } from './search_error';
34 changes: 34 additions & 0 deletions src/ui/public/courier/search_strategy/search_error.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

export class SearchError extends Error {
constructor({ status, title, message, path }) {
super(message);
this.name = 'SearchError';
this.status = status;
this.title = title;
this.message = message;
this.path = path;
Error.captureStackTrace(this, SearchError);

// Babel doesn't support traditional `extends` syntax for built-in classes.
// https://babeljs.io/docs/en/caveats/#classes
Object.setPrototypeOf(this, SearchError.prototype);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ export const assignSearchRequestsToSearchStrategies = searchRequests => {
const searchStrategyById = {};

searchRequests.forEach(searchRequest => {

const indexPattern = searchRequest.source.getField('index');
const matchingSearchStrategy = getSearchStrategy(indexPattern);

Expand Down
9 changes: 9 additions & 0 deletions src/ui/public/vis/editors/default/sidebar.html
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,15 @@
</div>

<nav class="navbar navbar-default subnav">
<div class="visEditorSidebarError" ng-if="sidebar.persistentError">
<call-out
color="'danger'"
iconType="'alert'"
title="sidebar.persistentError"
size="'s'"
/>
</div>

<div class="container-fluid">

<!-- tabs -->
Expand Down
20 changes: 18 additions & 2 deletions src/ui/public/vis/editors/default/sidebar.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,18 @@ import './vis_options';
import { uiModules } from '../../../modules';
import sidebarTemplate from './sidebar.html';

import './sidebar.less';

uiModules
.get('app/visualize')
.directive('visEditorSidebar', function () {


return {
restrict: 'E',
template: sidebarTemplate,
scope: true,
controllerAs: 'sidebar',
controller: function ($scope) {
this.persistentError = undefined;

$scope.$watch('vis.type', (visType) => {
if (visType) {
Expand All @@ -49,6 +50,21 @@ uiModules
this.section = this.section || (this.showData ? 'data' : _.get(visType, 'editorConfig.optionTabs[0].name'));
}
});

// TODO: This doesn't update when requestError is set.
$scope.$watch('vis.requestError', (requestError) => {
if (requestError && requestError.messsage) {
const { message } = requestError;
const isRollupError = message.includes('rollup');

if (isRollupError) {
this.persistentError = message;
return;
}
}

this.persistentError = undefined;
});
}
};
});
3 changes: 3 additions & 0 deletions src/ui/public/vis/editors/default/sidebar.less
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
.visEditorSidebarError {
padding: 8px;
}
40 changes: 30 additions & 10 deletions src/ui/public/visualize/loader/visualize_data_loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
* specific language governing permissions and limitations
* under the License.
*/

import React, { Fragment } from 'react';
import { isEqual } from 'lodash';
import { VisRequestHandlersRegistryProvider } from '../../registry/vis_request_handlers';
import { VisResponseHandlersRegistryProvider } from '../../registry/vis_response_handlers';
Expand All @@ -25,6 +27,7 @@ import {
} from '../../elasticsearch_errors';

import { toastNotifications } from 'ui/notify';
import { SearchError } from 'ui/courier';

function getHandler(from, name) {
if (typeof name === 'function') return name;
Expand Down Expand Up @@ -62,24 +65,41 @@ export class VisualizeDataLoader {

this._previousVisState = this._vis.getState();
this._previousRequestHandlerResponse = requestHandlerResponse;
this._vis.requestError = undefined;

if (!canSkipResponseHandler) {
this._visData = await Promise.resolve(this._responseHandler(this._vis, requestHandlerResponse));
}
return this._visData;
}
catch (e) {
catch (error) {
props.searchSource.cancelQueued();
this._vis.requestError = e;
if (isTermSizeZeroError(e)) {
return toastNotifications.addDanger(
`Your visualization ('${props.vis.title}') has an error: it has a term ` +
`aggregation with a size of 0. Please set it to a number greater than 0 to resolve ` +
`the error.`
);
this._vis.requestError = error;

if (isTermSizeZeroError(error)) {
return toastNotifications.addDanger({
title: `Visualization ('${this._vis.title}') has term aggregation of size 0`,
text: `Set it to a size greater than 0`,
});
}

if (error instanceof SearchError) {
const { message, path, status } = error;
return toastNotifications.addDanger({
title: `Visualization '${this._vis.title}' search failed`,
text: (
<Fragment>
<p>{message}</p>
<p><code>{status} {path}</code></p>
</Fragment>
),
});
}
toastNotifications.addDanger(e);

toastNotifications.addDanger({
title: `Visualization '${this._vis.title}' has an error`,
text: error,
});
}
}

}
2 changes: 1 addition & 1 deletion x-pack/plugins/rollup/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { PLUGIN } from './common/constants';
import { registerLicenseChecker } from './server/lib/register_license_checker';
import { registerIndicesRoute, registerFieldsForWildcardRoute, registerSearchRoute } from './server/routes/api';

export function rollup(kibana) {
export function rollup(kibana) {
return new kibana.Plugin({
id: PLUGIN.ID,
publicDir: resolve(__dirname, 'public'),
Expand Down
Loading

0 comments on commit 43f59dd

Please sign in to comment.