Surface vis loader errors in the UI. (#30594)

This commit is contained in:
Luke Elmers 2019-02-21 18:43:15 -07:00 committed by GitHub
parent 75e49e667c
commit f4d93ec990
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 74 additions and 304 deletions

View file

@ -1,69 +0,0 @@
/*
* 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.
*/
import expect from 'expect.js';
import { ElasticsearchError } from '../elasticsearch_error';
describe('ElasticsearchError', () => {
function createError(rootCauses = []) {
// Elasticsearch errors are characterized by the resp.error.root_cause array.
return {
resp: {
error: {
root_cause: rootCauses.map(rootCause => ({
reason: rootCause,
})),
}
}
};
}
describe('interface', () => {
describe('constructor', () => {
it('throws an error if instantiated with a non-elasticsearch error', () => {
expect(() => new ElasticsearchError({})).to.throwError();
});
});
describe('getRootCauses', () => {
it(`returns the root_cause array's reason values`, () => {
const rootCauses = ['a', 'b'];
const error = createError(rootCauses);
const esError = new ElasticsearchError(error);
expect(esError.getRootCauses()).to.eql(rootCauses);
});
});
describe('hasRootCause', () => {
it(`returns true if the cause occurs in the root_cause array's reasons, insensitive to case`, () => {
const rootCauses = ['a very detailed error', 'a slightly more detailed error'];
const error = createError(rootCauses);
const esError = new ElasticsearchError(error);
expect(esError.hasRootCause('slightly MORE')).to.be(true);
});
it(`returns false if the cause doesn't occur in the root_cause array's reasons`, () => {
const rootCauses = ['a very detailed error', 'a slightly more detailed error'];
const error = createError(rootCauses);
const esError = new ElasticsearchError(error);
expect(esError.hasRootCause('nonexistent error')).to.be(false);
});
});
});
});

View file

@ -1,55 +0,0 @@
/*
* 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.
*/
import expect from 'expect.js';
import { isTermSizeZeroError } from '../is_term_size_zero_error';
describe('isTermSizeZeroError', () => {
const identifyingString = 'size must be positive, got 0';
it('returns true if it contains the identifying string', () => {
const error = {
resp: {
error: {
root_cause: [{
reason: `Some crazy Java exception: ${identifyingString}`,
}],
}
}
};
expect(isTermSizeZeroError(error)).to.be(true);
});
it(`returns false if it doesn't contain the identifying string`, () => {
const error = {
resp: {
error: {
root_cause: [{
reason: `Some crazy Java exception`,
}],
}
}
};
expect(isTermSizeZeroError(error)).to.be(false);
});
it ('returns false for non-elasticsearch error input', () => {
expect(isTermSizeZeroError({ foo: 'bar' })).to.be(false);
});
});

View file

@ -1,62 +0,0 @@
/*
* 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.
*/
import _ from 'lodash';
export class ElasticsearchError {
constructor(error) {
this.error = error;
this.getRootCauses = this.getRootCauses.bind(this);
this.hasRootCause = this.hasRootCause.bind(this);
if (!this.getRootCauses().length) {
throw new Error(
'ElasticsearchError must be instantiated with an elasticsearch error, i.e. it must have' +
`a resp.error.root_cause property. Instead got ${JSON.stringify(error)}`
);
}
}
static hasRootCause(error, cause) {
try {
const esError = new ElasticsearchError(error);
return esError.hasRootCause(cause);
} catch (err) {
// we assume that any failure represents a validation error
// in the ElasticsearchError constructor
return false;
}
}
getRootCauses() {
const rootCauses = _.get(this.error, 'resp.error.root_cause');
return _.pluck(rootCauses, 'reason');
}
hasRootCause(cause) {
const normalizedCause = cause.toLowerCase();
const rootCauses = this.getRootCauses();
const matchingCauses = rootCauses.filter(rootCause => {
const normalizedRootCause = rootCause.toLowerCase();
return normalizedRootCause.indexOf(normalizedCause) !== -1;
});
return matchingCauses.length !== 0;
}
}

View file

@ -1,21 +0,0 @@
/*
* 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 { ElasticsearchError } from './elasticsearch_error';
export { isTermSizeZeroError } from './is_term_size_zero_error';

View file

@ -1,24 +0,0 @@
/*
* 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.
*/
import { ElasticsearchError } from './elasticsearch_error';
export function isTermSizeZeroError(error) {
return ElasticsearchError.hasRootCause(error, 'size must be positive, got 0');
}

View file

@ -46,6 +46,9 @@ import {
} from './types';
import { queryGeohashBounds } from './utils';
import { i18n } from '@kbn/i18n';
import { toastNotifications } from 'ui/notify';
interface EmbeddedVisualizeHandlerParams extends VisualizeLoaderParams {
Private: IPrivate;
queryFilter: any;
@ -428,12 +431,47 @@ export class EmbeddedVisualizeHandler {
this.dataLoaderParams.inspectorAdapters = this.inspectorAdapters;
this.vis.filters = { timeRange: this.dataLoaderParams.timeRange };
this.vis.requestError = undefined;
this.vis.showRequestError = false;
return this.dataLoader.fetch(this.dataLoaderParams).then(data => {
if (data && data.value) {
this.dataSubject.next(data.value);
}
return data;
return this.dataLoader
.fetch(this.dataLoaderParams)
.then(data => {
// Pipeline responses never throw errors, so we need to check for
// `type: 'error'`, and then throw so it can be caught below.
// TODO: We should revisit this after we have fully migrated
// to the new expression pipeline infrastructure.
if (data && data.type === 'error') {
throw data.error;
}
if (data && data.value) {
this.dataSubject.next(data.value);
}
return data;
})
.catch(this.handleDataLoaderError);
};
/**
* When dataLoader returns an error, we need to make sure it surfaces in the UI.
*
* TODO: Eventually we should add some custom error messages for issues that are
* frequently encountered by users.
*/
private handleDataLoaderError = (error: any): void => {
// TODO: come up with a general way to cancel execution of pipeline expressions.
this.dataLoaderParams.searchSource.cancelQueued();
this.vis.requestError = error;
this.vis.showRequestError =
error.type && ['NO_OP_SEARCH_STRATEGY', 'UNSUPPORTED_QUERY'].includes(error.type);
toastNotifications.addDanger({
title: i18n.translate('common.ui.visualize.dataLoaderError', {
defaultMessage: 'Error in visualization',
}),
text: error.message,
});
};

View file

@ -24,8 +24,6 @@ export class PipelineDataLoader {
constructor(private readonly vis: Vis) {}
public async fetch(params: RequestHandlerParams): Promise<any> {
this.vis.requestError = undefined;
this.vis.showRequestError = false;
this.vis.pipelineExpression = buildPipeline(this.vis, params);
return await runPipeline(

View file

@ -33,10 +33,6 @@ import {
import { VisResponseData } from './types';
// @ts-ignore No typing present
import { isTermSizeZeroError } from '../../elasticsearch_errors';
import { toastNotifications } from 'ui/notify';
import { decorateVisObject } from 'ui/visualize/loader/pipeline_helpers/build_pipeline';
function getHandler<T extends RequestHandler | ResponseHandler>(
@ -71,74 +67,43 @@ export class VisualizeDataLoader {
}
public async fetch(params: RequestHandlerParams): Promise<VisResponseData | void> {
this.vis.filters = { timeRange: params.timeRange };
this.vis.requestError = undefined;
this.vis.showRequestError = false;
// add necessary params to vis object (dimensions, bucket, metric, etc)
decorateVisObject(this.vis, { timeRange: params.timeRange });
try {
// searchSource is only there for courier request handler
const requestHandlerResponse = await this.requestHandler({
partialRows: this.vis.params.partialRows || this.vis.type.requiresPartialRows,
isHierarchical: this.vis.isHierarchical(),
visParams: this.vis.params,
...params,
filters: params.filters
? params.filters.filter(filter => !filter.meta.disabled)
: undefined,
});
// searchSource is only there for courier request handler
const requestHandlerResponse = await this.requestHandler({
partialRows: this.vis.params.partialRows || this.vis.type.requiresPartialRows,
isHierarchical: this.vis.isHierarchical(),
visParams: this.vis.params,
...params,
filters: params.filters ? params.filters.filter(filter => !filter.meta.disabled) : undefined,
});
// No need to call the response handler when there have been no data nor has been there changes
// in the vis-state (response handler does not depend on uiStat
const canSkipResponseHandler =
this.previousRequestHandlerResponse &&
this.previousRequestHandlerResponse === requestHandlerResponse &&
this.previousVisState &&
isEqual(this.previousVisState, this.vis.getState());
// No need to call the response handler when there have been no data nor has been there changes
// in the vis-state (response handler does not depend on uiStat
const canSkipResponseHandler =
this.previousRequestHandlerResponse &&
this.previousRequestHandlerResponse === requestHandlerResponse &&
this.previousVisState &&
isEqual(this.previousVisState, this.vis.getState());
this.previousVisState = this.vis.getState();
this.previousRequestHandlerResponse = requestHandlerResponse;
this.previousVisState = this.vis.getState();
this.previousRequestHandlerResponse = requestHandlerResponse;
if (!canSkipResponseHandler) {
this.visData = await Promise.resolve(
this.responseHandler(requestHandlerResponse, this.vis.params.dimensions)
);
}
return {
as: 'visualization',
value: {
visType: this.vis.type.name,
visData: this.visData,
visConfig: this.vis.params,
params: {},
},
};
} catch (error) {
params.searchSource.cancelQueued();
this.vis.requestError = error;
this.vis.showRequestError =
error.type && ['NO_OP_SEARCH_STRATEGY', 'UNSUPPORTED_QUERY'].includes(error.type);
// tslint:disable-next-line
console.error(error);
if (isTermSizeZeroError(error)) {
toastNotifications.addDanger(
`Your visualization ('${this.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.`
);
return;
}
toastNotifications.addDanger({
title: 'Error in visualization',
text: error.message,
});
if (!canSkipResponseHandler) {
this.visData = await Promise.resolve(
this.responseHandler(requestHandlerResponse, this.vis.params.dimensions)
);
}
return {
as: 'visualization',
value: {
visType: this.vis.type.name,
visData: this.visData,
visConfig: this.vis.params,
params: {},
},
};
}
}