diff --git a/src/legacy/core_plugins/input_control_vis/public/control/control.js b/src/legacy/core_plugins/input_control_vis/public/control/control.js index 9a27a3d3f1dbd..f9566a76180f0 100644 --- a/src/legacy/core_plugins/input_control_vis/public/control/control.js +++ b/src/legacy/core_plugins/input_control_vis/public/control/control.js @@ -60,6 +60,10 @@ export class Control { throw new Error('fetch method not defined, subclass are required to implement'); } + destroy() { + throw new Error('destroy method not defined, subclass are required to implement'); + } + format = (value) => { const field = this.filterManager.getField(); if (field) { diff --git a/src/legacy/core_plugins/input_control_vis/public/control/list_control_factory.js b/src/legacy/core_plugins/input_control_vis/public/control/list_control_factory.js index f93f5e4801301..105bbe5dad501 100644 --- a/src/legacy/core_plugins/input_control_vis/public/control/list_control_factory.js +++ b/src/legacy/core_plugins/input_control_vis/public/control/list_control_factory.js @@ -68,6 +68,12 @@ const termsAgg = ({ field, size, direction, query }) => { class ListControl extends Control { fetch = async (query) => { + // Abort any in-progress fetch + if (this.abortController) { + this.abortController.abort(); + } + this.abortController = new AbortController(); + const indexPattern = this.filterManager.getIndexPattern(); if (!indexPattern) { this.disable(noIndexPatternMsg(this.controlParams.indexPattern)); @@ -114,13 +120,18 @@ class ListControl extends Control { indexPattern, aggs, this.useTimeFilter, - ancestorFilters); + ancestorFilters + ); + this.abortController.signal.addEventListener('abort', () => searchSource.cancelQueued()); this.lastQuery = query; let resp; try { resp = await searchSource.fetch(); } catch(error) { + // If the fetch was aborted then no need to surface this error in the UI + if (error.name === 'AbortError') return; + this.disable(i18n.translate('inputControl.listControl.unableToFetchTooltip', { defaultMessage: 'Unable to fetch terms, error: {errorMessage}', values: { errorMessage: error.message } @@ -148,6 +159,10 @@ class ListControl extends Control { this.disabledReason = ''; } + destroy() { + if (this.abortController) this.abortController.abort(); + } + hasValue() { return typeof this.value !== 'undefined' && this.value.length > 0; } diff --git a/src/legacy/core_plugins/input_control_vis/public/control/range_control_factory.js b/src/legacy/core_plugins/input_control_vis/public/control/range_control_factory.js index 78a0e42771c2b..8a157769165ca 100644 --- a/src/legacy/core_plugins/input_control_vis/public/control/range_control_factory.js +++ b/src/legacy/core_plugins/input_control_vis/public/control/range_control_factory.js @@ -50,6 +50,12 @@ const minMaxAgg = (field) => { class RangeControl extends Control { async fetch() { + // Abort any in-progress fetch + if (this.abortController) { + this.abortController.abort(); + } + this.abortController = new AbortController(); + const indexPattern = this.filterManager.getIndexPattern(); if (!indexPattern) { this.disable(noIndexPatternMsg(this.controlParams.indexPattern)); @@ -60,11 +66,15 @@ class RangeControl extends Control { const aggs = minMaxAgg(indexPattern.fields.byName[fieldName]); const searchSource = createSearchSource(this.kbnApi, null, indexPattern, aggs, this.useTimeFilter); + this.abortController.signal.addEventListener('abort', () => searchSource.cancelQueued()); let resp; try { resp = await searchSource.fetch(); } catch(error) { + // If the fetch was aborted then no need to surface this error in the UI + if (error.name === 'AbortError') return; + this.disable(i18n.translate('inputControl.rangeControl.unableToFetchTooltip', { defaultMessage: 'Unable to fetch range min and max, error: {errorMessage}', values: { errorMessage: error.message } @@ -84,6 +94,10 @@ class RangeControl extends Control { this.max = max; this.enable = true; } + + destroy() { + if (this.abortController) this.abortController.abort(); + } } export async function rangeControlFactory(controlParams, kbnApi, useTimeFilter) { diff --git a/src/legacy/core_plugins/input_control_vis/public/vis_controller.js b/src/legacy/core_plugins/input_control_vis/public/vis_controller.js index 07ec0a690b4da..9777b9c1570d5 100644 --- a/src/legacy/core_plugins/input_control_vis/public/vis_controller.js +++ b/src/legacy/core_plugins/input_control_vis/public/vis_controller.js @@ -42,14 +42,13 @@ class VisController { this.controls = []; this.controls = await this.initControls(); this.drawVis(); - return; } - return; } destroy() { this.updateSubsciption.unsubscribe(); unmountComponentAtNode(this.el); + this.controls.forEach(control => control.destroy()); } drawVis = () => { diff --git a/src/legacy/core_plugins/interpreter/public/functions/esaggs.ts b/src/legacy/core_plugins/interpreter/public/functions/esaggs.ts index bcc2951580d86..4438256c91228 100644 --- a/src/legacy/core_plugins/interpreter/public/functions/esaggs.ts +++ b/src/legacy/core_plugins/interpreter/public/functions/esaggs.ts @@ -87,7 +87,7 @@ export const esaggs = (): ExpressionFunction { + columns: response.columns.map((column: any) => { const cleanedColumn: KibanaDatatableColumn = { id: column.id, name: column.name, diff --git a/src/legacy/core_plugins/kibana/public/discover/controllers/discover.js b/src/legacy/core_plugins/kibana/public/discover/controllers/discover.js index dd1da717b4a82..5753fdda780d9 100644 --- a/src/legacy/core_plugins/kibana/public/discover/controllers/discover.js +++ b/src/legacy/core_plugins/kibana/public/discover/controllers/discover.js @@ -761,6 +761,9 @@ function discoverController( }) .then(onResults) .catch((error) => { + // If the request was aborted then no need to surface this error in the UI + if (error instanceof Error && error.name === 'AbortError') return; + const fetchError = getPainlessError(error); if (fetchError) { diff --git a/src/legacy/core_plugins/kibana/public/discover/embeddable/search_embeddable.ts b/src/legacy/core_plugins/kibana/public/discover/embeddable/search_embeddable.ts index afbbcf79173db..92fad1713177a 100644 --- a/src/legacy/core_plugins/kibana/public/discover/embeddable/search_embeddable.ts +++ b/src/legacy/core_plugins/kibana/public/discover/embeddable/search_embeddable.ts @@ -191,6 +191,7 @@ export class SearchEmbeddable extends Embeddable if (this.autoRefreshFetchSubscription) { this.autoRefreshFetchSubscription.unsubscribe(); } + this.savedSearch.searchSource.cancelQueued(); } private initializeSearchScope() { @@ -270,6 +271,10 @@ export class SearchEmbeddable extends Embeddable if (!this.searchScope) return; const { searchSource } = this.savedSearch; + + // Abort any in-progress requests + searchSource.cancelQueued(); + searchSource.setField('size', config.get('discover:sampleSize')); searchSource.setField('sort', getSort(this.searchScope.sort, this.searchScope.indexPattern)); @@ -304,6 +309,9 @@ export class SearchEmbeddable extends Embeddable this.searchScope!.totalHitCount = resp.hits.total; }); } catch (error) { + // If the fetch was aborted, no need to surface this in the UI + if (error.name === 'AbortError') return; + toastNotifications.addError(error, { title: i18n.translate('kbn.embeddable.errorTitle', { defaultMessage: 'Error fetching data', diff --git a/src/legacy/ui/public/agg_types/buckets/histogram.js b/src/legacy/ui/public/agg_types/buckets/histogram.js index 5f5b1d2e9e95b..418604e48dc57 100644 --- a/src/legacy/ui/public/agg_types/buckets/histogram.js +++ b/src/legacy/ui/public/agg_types/buckets/histogram.js @@ -76,13 +76,13 @@ export const histogramBucketAgg = new BucketAggType({ { name: 'interval', editorComponent: NumberIntervalParamEditor, - modifyAggConfigOnSearchRequestStart(aggConfig, searchSource) { + modifyAggConfigOnSearchRequestStart(aggConfig, searchSource, searchRequest) { const field = aggConfig.getField(); const aggBody = field.scripted ? { script: { source: field.script, lang: field.lang } } : { field: field.name }; - return searchSource + const childSearchSource = searchSource .createChild() .setField('size', 0) .setField('aggs', { @@ -92,15 +92,19 @@ export const histogramBucketAgg = new BucketAggType({ minAgg: { min: aggBody } - }) - .fetch() + }); + + searchRequest.whenAborted(() => childSearchSource.cancelQueued()); + + return childSearchSource.fetch() .then((resp) => { aggConfig.setAutoBounds({ min: _.get(resp, 'aggregations.minAgg.value'), max: _.get(resp, 'aggregations.maxAgg.value') }); }) - .catch(() => { + .catch(e => { + if (e.name === 'AbortError') return; toastNotifications.addWarning(i18n.translate('common.ui.aggTypes.histogram.missingMaxMinValuesWarning', { // eslint-disable-next-line max-len defaultMessage: 'Unable to retrieve max and min values to auto-scale histogram buckets. This may lead to poor visualization performance.' diff --git a/src/legacy/ui/public/agg_types/buckets/terms.js b/src/legacy/ui/public/agg_types/buckets/terms.js index 48580cb721971..2450bb6466404 100644 --- a/src/legacy/ui/public/agg_types/buckets/terms.js +++ b/src/legacy/ui/public/agg_types/buckets/terms.js @@ -76,12 +76,15 @@ export const termsBucketAgg = new BucketAggType({ }; }, createFilter: createFilterTerms, - postFlightRequest: async (resp, aggConfigs, aggConfig, searchSource, inspectorAdapters) => { + postFlightRequest: async (resp, aggConfigs, aggConfig, searchSource, inspectorAdapters, abortSignal) => { if (!resp.aggregations) return resp; const nestedSearchSource = searchSource.createChild(); if (aggConfig.params.otherBucket) { const filterAgg = buildOtherBucketAgg(aggConfigs, aggConfig, resp); if (!filterAgg) return resp; + if (abortSignal) { + abortSignal.addEventListener('abort', () => nestedSearchSource.cancelQueued()); + } nestedSearchSource.setField('aggs', filterAgg); diff --git a/src/legacy/ui/public/courier/fetch/request/search_request/search_request.js b/src/legacy/ui/public/courier/fetch/request/search_request/search_request.js index 0af741d0216bb..a6ce562e462d8 100644 --- a/src/legacy/ui/public/courier/fetch/request/search_request/search_request.js +++ b/src/legacy/ui/public/courier/fetch/request/search_request/search_request.js @@ -168,10 +168,13 @@ export function SearchRequestProvider(Promise) { abort() { this._markStopped(); - this.defer = null; this.aborted = true; - this.abortedDefer.resolve(); + const error = new Error('The request was aborted.'); + error.name = 'AbortError'; + this.abortedDefer.resolve(error); this.abortedDefer = null; + this.defer.reject(error); + this.defer = null; } whenAborted(cb) { diff --git a/src/legacy/ui/public/vis/request_handlers/courier.js b/src/legacy/ui/public/vis/request_handlers/courier.js index 5cf5bf005d0b2..8ece5b11d6a42 100644 --- a/src/legacy/ui/public/vis/request_handlers/courier.js +++ b/src/legacy/ui/public/vis/request_handlers/courier.js @@ -40,7 +40,8 @@ const CourierRequestHandlerProvider = function () { partialRows, metricsAtAllLevels, inspectorAdapters, - queryFilter + queryFilter, + abortSignal, }) { // Create a new search source that inherits the original search source // but has the appropriate timeRange applied via a filter. @@ -101,6 +102,11 @@ const CourierRequestHandlerProvider = function () { request.stats(getRequestInspectorStats(requestSearchSource)); try { + // Abort any in-progress requests before fetching again + if (abortSignal) { + abortSignal.addEventListener('abort', () => requestSearchSource.cancelQueued()); + } + const response = await requestSearchSource.fetch(); searchSource.lastQuery = queryHash; @@ -133,7 +139,8 @@ const CourierRequestHandlerProvider = function () { aggs, agg, requestSearchSource, - inspectorAdapters + inspectorAdapters, + abortSignal, ); } } diff --git a/src/legacy/ui/public/vis/request_handlers/request_handlers.d.ts b/src/legacy/ui/public/vis/request_handlers/request_handlers.d.ts index 476b3fa68473e..631dfe2d7325b 100644 --- a/src/legacy/ui/public/vis/request_handlers/request_handlers.d.ts +++ b/src/legacy/ui/public/vis/request_handlers/request_handlers.d.ts @@ -40,6 +40,7 @@ export interface RequestHandlerParams { inspectorAdapters?: Adapters; metricsAtAllLevels?: boolean; visParams?: any; + abortSignal?: AbortSignal; } export type RequestHandler = (params: RequestHandlerParams) => T; diff --git a/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.test.ts b/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.test.ts index 9a16405398efb..ba809ae86d7d6 100644 --- a/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.test.ts +++ b/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.test.ts @@ -153,21 +153,30 @@ describe('EmbeddedVisualizeHandler', () => { const params = { timeRange: { foo: 'bar' } }; handler.update(params); jest.runAllTimers(); - expect(mockDataLoaderFetch).toHaveBeenCalledWith({ ...dataLoaderParams, ...params }); + expect(mockDataLoaderFetch).toHaveBeenCalled(); + const { abortSignal, ...otherParams } = mockDataLoaderFetch.mock.calls[0][0]; + expect(abortSignal).toBeInstanceOf(AbortSignal); + expect(otherParams).toEqual({ ...dataLoaderParams, ...params }); }); it('should call dataLoader.render with updated filters', () => { const params = { filters: [{ meta: { disabled: false } }] }; handler.update(params); jest.runAllTimers(); - expect(mockDataLoaderFetch).toHaveBeenCalledWith({ ...dataLoaderParams, ...params }); + expect(mockDataLoaderFetch).toHaveBeenCalled(); + const { abortSignal, ...otherParams } = mockDataLoaderFetch.mock.calls[0][0]; + expect(abortSignal).toBeInstanceOf(AbortSignal); + expect(otherParams).toEqual({ ...dataLoaderParams, ...params }); }); it('should call dataLoader.render with updated query', () => { const params = { query: { foo: 'bar' } }; handler.update(params); jest.runAllTimers(); - expect(mockDataLoaderFetch).toHaveBeenCalledWith({ ...dataLoaderParams, ...params }); + expect(mockDataLoaderFetch).toHaveBeenCalled(); + const { abortSignal, ...otherParams } = mockDataLoaderFetch.mock.calls[0][0]; + expect(abortSignal).toBeInstanceOf(AbortSignal); + expect(otherParams).toEqual({ ...dataLoaderParams, ...params }); }); }); @@ -197,6 +206,13 @@ describe('EmbeddedVisualizeHandler', () => { handler.destroy(); expect(spy).toHaveBeenCalledTimes(1); }); + + it('should call abort on controller', () => { + handler.abortController = new AbortController(); + const spy = jest.spyOn(handler.abortController, 'abort'); + handler.destroy(); + expect(spy).toHaveBeenCalled(); + }); }); describe('openInspector', () => { diff --git a/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.ts b/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.ts index 1a0155246e87f..2d50831a21041 100644 --- a/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.ts +++ b/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.ts @@ -105,6 +105,7 @@ export class EmbeddedVisualizeHandler { private actions: any = {}; private events$: Rx.Observable; private autoFetch: boolean; + private abortController?: AbortController; private autoRefreshFetchSubscription: Rx.Subscription | undefined; constructor( @@ -268,6 +269,7 @@ export class EmbeddedVisualizeHandler { */ public destroy(): void { this.destroyed = true; + this.cancel(); this.debouncedFetchAndRender.cancel(); if (this.autoFetch) { if (this.autoRefreshFetchSubscription) this.autoRefreshFetchSubscription.unsubscribe(); @@ -443,7 +445,14 @@ export class EmbeddedVisualizeHandler { this.fetchAndRender(); }; + private cancel = () => { + if (this.abortController) this.abortController.abort(); + }; + private fetch = (forceFetch: boolean = false) => { + this.cancel(); + this.abortController = new AbortController(); + this.dataLoaderParams.abortSignal = this.abortController.signal; this.dataLoaderParams.aggs = this.vis.getAggConfig(); this.dataLoaderParams.forceFetch = forceFetch; this.dataLoaderParams.inspectorAdapters = this.inspectorAdapters; @@ -452,23 +461,27 @@ export class EmbeddedVisualizeHandler { this.vis.requestError = undefined; this.vis.showRequestError = false; - 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); + return ( + this.dataLoader + // Don't pass in this.dataLoaderParams directly because it may be modified async in another + // call to fetch before the previous one has completed + .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) + ); }; /** @@ -478,6 +491,9 @@ export class EmbeddedVisualizeHandler { * frequently encountered by users. */ private handleDataLoaderError = (error: any): void => { + // If the data loader was aborted then no need to surface this error in the UI + if (error && error.name === 'AbortError') return; + // TODO: come up with a general way to cancel execution of pipeline expressions. if (this.dataLoaderParams.searchSource && this.dataLoaderParams.searchSource.cancelQueued) { this.dataLoaderParams.searchSource.cancelQueued(); diff --git a/src/legacy/ui/public/visualize/loader/pipeline_data_loader.ts b/src/legacy/ui/public/visualize/loader/pipeline_data_loader.ts index e9ccaf75e63f9..41bee473937de 100644 --- a/src/legacy/ui/public/visualize/loader/pipeline_data_loader.ts +++ b/src/legacy/ui/public/visualize/loader/pipeline_data_loader.ts @@ -26,7 +26,7 @@ export class PipelineDataLoader { public async fetch(params: RequestHandlerParams): Promise { this.vis.pipelineExpression = await buildPipeline(this.vis, params); - return await runPipeline( + return runPipeline( this.vis.pipelineExpression, {}, { @@ -39,6 +39,7 @@ export class PipelineDataLoader { : undefined, }), inspectorAdapters: params.inspectorAdapters, + abortSignal: params.abortSignal, } ); } diff --git a/src/legacy/ui/public/visualize/loader/pipeline_helpers/build_pipeline.ts b/src/legacy/ui/public/visualize/loader/pipeline_helpers/build_pipeline.ts index 4825235ea953e..b11e663808b35 100644 --- a/src/legacy/ui/public/visualize/loader/pipeline_helpers/build_pipeline.ts +++ b/src/legacy/ui/public/visualize/loader/pipeline_helpers/build_pipeline.ts @@ -416,7 +416,7 @@ const buildVisConfig: BuildVisConfigFunction = { export const buildVislibDimensions = async ( vis: any, - params: { searchSource: any; timeRange?: any } + params: { searchSource: any; timeRange?: any; abortSignal?: AbortSignal } ) => { const schemas = getSchemas(vis, params.timeRange); const dimensions = { @@ -439,7 +439,18 @@ export const buildVislibDimensions = async ( } else if (xAgg.type.name === 'histogram') { const intervalParam = xAgg.type.params.byName.interval; const output = { params: {} as any }; - await intervalParam.modifyAggConfigOnSearchRequestStart(xAgg, params.searchSource); + const searchRequest = { + whenAborted: (fn: any) => { + if (params.abortSignal) { + params.abortSignal.addEventListener('abort', fn); + } + }, + }; + await intervalParam.modifyAggConfigOnSearchRequestStart( + xAgg, + params.searchSource, + searchRequest + ); intervalParam.write(xAgg, output); dimensions.x.params.interval = output.params.interval; } @@ -453,7 +464,7 @@ export const buildVislibDimensions = async ( // take a Vis object and decorate it with the necessary params (dimensions, bucket, metric, etc) export const getVisParams = async ( vis: Vis, - params: { searchSource: SearchSource; timeRange?: any } + params: { searchSource: SearchSource; timeRange?: any; abortSignal?: AbortSignal } ) => { const schemas = getSchemas(vis, params.timeRange); let visConfig = cloneDeep(vis.params); diff --git a/src/legacy/ui/public/visualize/loader/pipeline_helpers/run_pipeline.ts b/src/legacy/ui/public/visualize/loader/pipeline_helpers/run_pipeline.ts index 1551ab755ea5b..43dbb06f8c973 100644 --- a/src/legacy/ui/public/visualize/loader/pipeline_helpers/run_pipeline.ts +++ b/src/legacy/ui/public/visualize/loader/pipeline_helpers/run_pipeline.ts @@ -30,6 +30,7 @@ type getInitialContextFunction = () => KibanaContext; export interface RunPipelineHandlers { getInitialContext: getInitialContextFunction; inspectorAdapters?: Adapters; + abortSignal?: AbortSignal; } export const runPipeline = async ( diff --git a/src/legacy/ui/public/visualize/loader/visualize_data_loader.ts b/src/legacy/ui/public/visualize/loader/visualize_data_loader.ts index 589b5f2723112..28bba324d63ce 100644 --- a/src/legacy/ui/public/visualize/loader/visualize_data_loader.ts +++ b/src/legacy/ui/public/visualize/loader/visualize_data_loader.ts @@ -70,6 +70,7 @@ export class VisualizeDataLoader { const visParams = await getVisParams(this.vis, { searchSource: params.searchSource, timeRange: params.timeRange, + abortSignal: params.abortSignal, }); const filters = params.filters || []; @@ -85,6 +86,7 @@ export class VisualizeDataLoader { ...params, query, filters: filters.concat(savedFilters).filter(f => !f.meta.disabled), + abortSignal: params.abortSignal, }); // No need to call the response handler when there have been no data nor has there been changes diff --git a/src/plugins/data/common/expressions/interpreter_provider.ts b/src/plugins/data/common/expressions/interpreter_provider.ts index cb025e22131c5..7a9309a2a42f2 100644 --- a/src/plugins/data/common/expressions/interpreter_provider.ts +++ b/src/plugins/data/common/expressions/interpreter_provider.ts @@ -52,6 +52,14 @@ export function interpreterProvider(config: any) { async function invokeChain(chainArr: any, context: any): Promise { if (!chainArr.length) return Promise.resolve(context); + // if execution was aborted return error + if (handlers.abortSignal && handlers.abortSignal.aborted) { + return createError({ + message: 'The expression was aborted.', + name: 'AbortError', + }); + } + const chain = clone(chainArr); const link = chain.shift(); // Every thing in the chain will always be a function right? const { function: fnName, arguments: fnArgs } = link; @@ -71,14 +79,6 @@ export function interpreterProvider(config: any) { // if something failed, just return the failure if (getType(newContext) === 'error') return newContext; - // if execution was aborted return error - if (handlers.abortSignal && handlers.abortSignal.aborted) { - return createError({ - message: 'The expression was aborted.', - name: 'AbortError', - }); - } - // Continue re-invoking chain until it's empty return invokeChain(chain, newContext); } catch (e) {