From a5482d84b81473dfbf6b3b05c84ab22f52168c10 Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Wed, 26 Sep 2018 14:42:56 -0700 Subject: [PATCH] Add SearchError for surfacing courier search errors. (#23382) (#23543) --- .../courier/fetch/__tests__/call_client.js | 5 ++- src/ui/public/courier/fetch/fetch_now.js | 5 ++- 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 | 39 +++++++++++++++++++ .../search_strategy_registry.js | 13 ++----- src/ui/public/vis/editors/default/sidebar.js | 3 -- 8 files changed, 73 insertions(+), 19 deletions(-) create mode 100644 src/ui/public/courier/search_strategy/search_error.js diff --git a/src/ui/public/courier/fetch/__tests__/call_client.js b/src/ui/public/courier/fetch/__tests__/call_client.js index 188a1dddec5ed..a4858347d4b80 100644 --- a/src/ui/public/courier/fetch/__tests__/call_client.js +++ b/src/ui/public/courier/fetch/__tests__/call_client.js @@ -186,7 +186,7 @@ describe('callClient', () => { }); }); - it(`calls searchRequest.handleFailure() with the ES error that's thrown`, async () => { + it(`calls searchRequest.handleFailure() with the SearchError that's thrown`, async () => { esShouldError = true; const searchRequest = createSearchRequest(1); @@ -195,7 +195,8 @@ describe('callClient', () => { searchRequests = [ searchRequest ]; return callClient(searchRequests).then(() => { - sinon.assert.calledWith(handleFailureSpy, 'fake es error'); + sinon.assert.calledOnce(handleFailureSpy); + expect(handleFailureSpy.args[0][0].name).to.be('SearchError'); }); }); }); diff --git a/src/ui/public/courier/fetch/fetch_now.js b/src/ui/public/courier/fetch/fetch_now.js index 70aa20a26619c..adc3635d9e9c0 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) { 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 a9af1012ec985..fd2a180456544 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) => { @@ -79,14 +80,30 @@ export const defaultSearchStrategy = { const searching = es.msearch(msearchParams); 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..9406ceb4beaeb --- /dev/null +++ b/src/ui/public/courier/search_strategy/search_error.js @@ -0,0 +1,39 @@ +/* + * 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; + + // captureStackTrace is only available in the V8 engine, so any browser using + // a different JS engine won't have access to this method. + if (Error.captureStackTrace) { + 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.js b/src/ui/public/vis/editors/default/sidebar.js index 6503a11b998f1..886de3fbd6e0d 100644 --- a/src/ui/public/vis/editors/default/sidebar.js +++ b/src/ui/public/vis/editors/default/sidebar.js @@ -26,15 +26,12 @@ import sidebarTemplate from './sidebar.html'; uiModules .get('app/visualize') .directive('visEditorSidebar', function () { - - return { restrict: 'E', template: sidebarTemplate, scope: true, controllerAs: 'sidebar', controller: function ($scope) { - $scope.$watch('vis.type', (visType) => { if (visType) { this.showData = visType.schemas.buckets || visType.schemas.metrics;