Skip to content

Commit

Permalink
Add SearchError for surfacing courier search errors. (#23382)
Browse files Browse the repository at this point in the history
  • Loading branch information
cjcenizal authored Sep 26, 2018
1 parent 1df2981 commit 265a324
Show file tree
Hide file tree
Showing 8 changed files with 73 additions and 19 deletions.
5 changes: 3 additions & 2 deletions src/ui/public/courier/fetch/__tests__/call_client.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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');
});
});
});
Expand Down
5 changes: 4 additions & 1 deletion 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 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 @@ -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;
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';
39 changes: 39 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,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);
}
}
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,
};
3 changes: 0 additions & 3 deletions src/ui/public/vis/editors/default/sidebar.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 265a324

Please sign in to comment.