Skip to content

Commit

Permalink
Add rollupSearchStrategy and SearchError (#20505)
Browse files Browse the repository at this point in the history
* Add /api/rollup/search endpoint and rollupSearchStrategy.
* Add SearchError for surfacing courier search errors.
- 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 authored and jen-huang committed Jul 24, 2018
1 parent bdfcd20 commit 189d35c
Show file tree
Hide file tree
Showing 17 changed files with 319 additions and 29 deletions.
11 changes: 9 additions & 2 deletions src/ui/public/courier/fetch/fetch_now.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,10 @@ export function FetchNowProvider(Private, Promise) {

return searchRequest.retry();
}))
.catch(error => fatalError(error, 'Courier fetch'));
.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 @@ -70,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);
}
}
13 changes: 3 additions & 10 deletions src/ui/public/courier/search_strategy/search_strategy_registry.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

const searchStrategies = [];

const addSearchStrategy = searchStrategy => {
export const addSearchStrategy = searchStrategy => {
if (searchStrategies.includes(searchStrategy)) {
return;
}
Expand Down Expand Up @@ -47,12 +47,11 @@ const getSearchStrategy = indexPattern => {
* We use an array of objects to preserve the order of the search requests, which we use to
* deterministically associate each response with the originating request.
*/
const assignSearchRequestsToSearchStrategies = searchRequests => {
export const assignSearchRequestsToSearchStrategies = searchRequests => {
const searchStrategiesWithRequests = [];
const searchStrategyById = {};

searchRequests.forEach(searchRequest => {

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

Expand All @@ -76,12 +75,6 @@ const assignSearchRequestsToSearchStrategies = searchRequests => {
return searchStrategiesWithRequests;
};

const hasSearchStategyForIndexPattern = indexPattern => {
export const hasSearchStategyForIndexPattern = indexPattern => {
return Boolean(getSearchStrategy(indexPattern));
};

export {
assignSearchRequestsToSearchStrategies,
addSearchStrategy,
hasSearchStategyForIndexPattern,
};
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,
});
}
}

}
8 changes: 6 additions & 2 deletions x-pack/plugins/rollup/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@
import { resolve } from 'path';
import { PLUGIN } from './common/constants';
import { registerLicenseChecker } from './server/lib/register_license_checker';
import { registerIndicesRoute, registerFieldsForWildcardRoute } from './server/routes/api';
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 All @@ -22,11 +22,15 @@ export function rollup(kibana) {
visualize: [
'plugins/rollup/visualize',
],
search: [
'plugins/rollup/search',
],
},
init: function (server) {
registerLicenseChecker(server);
registerIndicesRoute(server);
registerFieldsForWildcardRoute(server);
registerSearchRoute(server);
}
});
}
7 changes: 7 additions & 0 deletions x-pack/plugins/rollup/public/search/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
/*
* 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 './register';
10 changes: 10 additions & 0 deletions x-pack/plugins/rollup/public/search/register.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
/*
* 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 { addSearchStrategy } from 'ui/courier';
import { rollupSearchStrategy } from './rollup_search_strategy';

addSearchStrategy(rollupSearchStrategy);
Loading

0 comments on commit 189d35c

Please sign in to comment.