From 3fb74a53fc6d30565cded433afdb3684d3948db8 Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Mon, 23 Jul 2018 16:01:34 -0700 Subject: [PATCH] Add rollupSearchStrategy and SearchError (#20505) * 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. --- src/ui/public/courier/fetch/fetch_now.js | 11 +- src/ui/public/courier/index.js | 2 + .../default_search_strategy.js | 23 +++- .../public/courier/search_strategy/index.js | 2 + .../courier/search_strategy/search_error.js | 34 ++++++ .../search_strategy_registry.js | 13 +- .../public/vis/editors/default/sidebar.html | 9 ++ src/ui/public/vis/editors/default/sidebar.js | 20 ++- .../public/vis/editors/default/sidebar.less | 3 + x-pack/plugins/rollup/index.js | 8 +- x-pack/plugins/rollup/public/search/index.js | 7 ++ .../plugins/rollup/public/search/register.js | 10 ++ .../public/search/rollup_search_strategy.js | 114 ++++++++++++++++++ .../server/client/elasticsearch_rollup.js | 15 +++ .../plugins/rollup/server/routes/api/index.js | 1 + .../rollup/server/routes/api/search.js | 36 ++++++ 16 files changed, 289 insertions(+), 19 deletions(-) create mode 100644 src/ui/public/courier/search_strategy/search_error.js create mode 100644 src/ui/public/vis/editors/default/sidebar.less create mode 100644 x-pack/plugins/rollup/public/search/index.js create mode 100644 x-pack/plugins/rollup/public/search/register.js create mode 100644 x-pack/plugins/rollup/public/search/rollup_search_strategy.js create mode 100644 x-pack/plugins/rollup/server/routes/api/search.js diff --git a/src/ui/public/courier/fetch/fetch_now.js b/src/ui/public/courier/fetch/fetch_now.js index 70aa20a26619c..7bb3890fbf95e 100644 --- a/src/ui/public/courier/fetch/fetch_now.js +++ b/src/ui/public/courier/fetch/fetch_now.js @@ -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) { @@ -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(); diff --git a/src/ui/public/courier/index.js b/src/ui/public/courier/index.js index d466b2e080e3f..583172f1731cd 100644 --- a/src/ui/public/courier/index.js +++ b/src/ui/public/courier/index.js @@ -30,6 +30,8 @@ export { } from './search_source'; export { + addSearchStrategy, hasSearchStategyForIndexPattern, isDefaultTypeIndexPattern, + SearchError, } from './search_strategy'; diff --git a/src/ui/public/courier/search_strategy/default_search_strategy.js b/src/ui/public/courier/search_strategy/default_search_strategy.js index 05fc45c5c67d1..f5bd529b35c0b 100644 --- a/src/ui/public/courier/search_strategy/default_search_strategy.js +++ b/src/ui/public/courier/search_strategy/default_search_strategy.js @@ -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) => { @@ -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; diff --git a/src/ui/public/courier/search_strategy/index.js b/src/ui/public/courier/search_strategy/index.js index 41991b63f950d..102610538df98 100644 --- a/src/ui/public/courier/search_strategy/index.js +++ b/src/ui/public/courier/search_strategy/index.js @@ -24,3 +24,5 @@ export { } from './search_strategy_registry'; export { isDefaultTypeIndexPattern } from './is_default_type_index_pattern'; + +export { SearchError } from './search_error'; diff --git a/src/ui/public/courier/search_strategy/search_error.js b/src/ui/public/courier/search_strategy/search_error.js new file mode 100644 index 0000000000000..fecafb9b6fc13 --- /dev/null +++ b/src/ui/public/courier/search_strategy/search_error.js @@ -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); + } +} diff --git a/src/ui/public/courier/search_strategy/search_strategy_registry.js b/src/ui/public/courier/search_strategy/search_strategy_registry.js index a0341932565da..79e959b2a80ad 100644 --- a/src/ui/public/courier/search_strategy/search_strategy_registry.js +++ b/src/ui/public/courier/search_strategy/search_strategy_registry.js @@ -19,7 +19,7 @@ const searchStrategies = []; -const addSearchStrategy = searchStrategy => { +export const addSearchStrategy = searchStrategy => { if (searchStrategies.includes(searchStrategy)) { return; } @@ -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); @@ -76,12 +75,6 @@ const assignSearchRequestsToSearchStrategies = searchRequests => { return searchStrategiesWithRequests; }; -const hasSearchStategyForIndexPattern = indexPattern => { +export const hasSearchStategyForIndexPattern = indexPattern => { return Boolean(getSearchStrategy(indexPattern)); }; - -export { - assignSearchRequestsToSearchStrategies, - addSearchStrategy, - hasSearchStategyForIndexPattern, -}; diff --git a/src/ui/public/vis/editors/default/sidebar.html b/src/ui/public/vis/editors/default/sidebar.html index 6b04bb2981976..7d7356d2a69d1 100644 --- a/src/ui/public/vis/editors/default/sidebar.html +++ b/src/ui/public/vis/editors/default/sidebar.html @@ -19,6 +19,15 @@