From 802f608bb7802de068d9e19ca518b9cc0724b03e Mon Sep 17 00:00:00 2001 From: ppisljar Date: Wed, 3 Jul 2019 10:37:39 +0200 Subject: [PATCH 01/23] add AbortSignal to interpreter --- src/plugins/data/common/expressions/interpreter_provider.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/plugins/data/common/expressions/interpreter_provider.ts b/src/plugins/data/common/expressions/interpreter_provider.ts index ed26701de5d87..94cc73b22380a 100644 --- a/src/plugins/data/common/expressions/interpreter_provider.ts +++ b/src/plugins/data/common/expressions/interpreter_provider.ts @@ -71,6 +71,11 @@ 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(`abort was requested`); + } + // Continue re-invoking chain until it's empty return await invokeChain(chain, newContext); } catch (e) { From daf72d292913a3edd4b6157806ee34be1b107bca Mon Sep 17 00:00:00 2001 From: ppisljar Date: Wed, 3 Jul 2019 10:38:43 +0200 Subject: [PATCH 02/23] adding AbortSignal to visualize_loader --- .../ui/public/vis/request_handlers/request_handlers.d.ts | 1 + .../ui/public/visualize/loader/embedded_visualize_handler.ts | 3 +++ .../ui/public/visualize/loader/pipeline_data_loader.ts | 1 + .../public/visualize/loader/pipeline_helpers/run_pipeline.ts | 1 + src/legacy/ui/public/visualize/loader/types.ts | 5 +++++ 5 files changed, 11 insertions(+) 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 f2475065a869c..71c99cc1d2b2b 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.ts b/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.ts index 431c5ae9be6a1..00bfb754085c4 100644 --- a/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.ts +++ b/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.ts @@ -55,6 +55,7 @@ interface EmbeddedVisualizeHandlerParams extends VisualizeLoaderParams { queryFilter: any; autoFetch?: boolean; pipelineDataLoader?: boolean; + abortSignal?: AbortSignal; } const RENDER_COMPLETE_EVENT = 'render_complete'; @@ -122,6 +123,7 @@ export class EmbeddedVisualizeHandler { autoFetch = true, pipelineDataLoader = false, Private, + abortSignal, } = params; this.dataLoaderParams = { @@ -133,6 +135,7 @@ export class EmbeddedVisualizeHandler { uiState, aggs: vis.getAggConfig(), forceFetch: false, + abortSignal, }; this.pipelineDataLoader = pipelineDataLoader; 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..df78fc440b81e 100644 --- a/src/legacy/ui/public/visualize/loader/pipeline_data_loader.ts +++ b/src/legacy/ui/public/visualize/loader/pipeline_data_loader.ts @@ -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/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/types.ts b/src/legacy/ui/public/visualize/loader/types.ts index 4f34f6219621f..fb7261670a051 100644 --- a/src/legacy/ui/public/visualize/loader/types.ts +++ b/src/legacy/ui/public/visualize/loader/types.ts @@ -98,6 +98,11 @@ export interface VisualizeLoaderParams { * cycle happens. Default value: `true` */ autoFetch?: boolean; + + /** + * AbortSignal can be provided which allows canceling ongoing process + */ + abortSignal?: AbortSignal; } /** From 332dae1ed1c883f7f6118952ee6dc794e7f209e8 Mon Sep 17 00:00:00 2001 From: ppisljar Date: Wed, 3 Jul 2019 10:41:44 +0200 Subject: [PATCH 03/23] adding AbortSignal to embeddables and dashboard --- .../kibana/public/dashboard/dashboard_app.html | 1 + .../core_plugins/kibana/public/dashboard/dashboard_app.tsx | 1 + .../kibana/public/dashboard/dashboard_app_controller.tsx | 3 +++ .../kibana/public/dashboard/grid/dashboard_grid.tsx | 2 ++ .../kibana/public/dashboard/panel/dashboard_panel.tsx | 4 +++- .../public/dashboard/viewport/dashboard_viewport.tsx | 3 +++ .../dashboard/viewport/dashboard_viewport_provider.js | 1 + .../public/visualize/embeddable/visualize_embeddable.ts | 5 +++++ .../visualize/embeddable/visualize_embeddable_factory.ts | 4 +++- src/legacy/ui/public/embeddable/embeddable_factory.ts | 7 ++++++- 10 files changed, 28 insertions(+), 3 deletions(-) diff --git a/src/legacy/core_plugins/kibana/public/dashboard/dashboard_app.html b/src/legacy/core_plugins/kibana/public/dashboard/dashboard_app.html index d73e15287077c..30223c6d41761 100644 --- a/src/legacy/core_plugins/kibana/public/dashboard/dashboard_app.html +++ b/src/legacy/core_plugins/kibana/public/dashboard/dashboard_app.html @@ -120,6 +120,7 @@ diff --git a/src/legacy/core_plugins/kibana/public/dashboard/dashboard_app.tsx b/src/legacy/core_plugins/kibana/public/dashboard/dashboard_app.tsx index 68a293de167f2..5f127e865451b 100644 --- a/src/legacy/core_plugins/kibana/public/dashboard/dashboard_app.tsx +++ b/src/legacy/core_plugins/kibana/public/dashboard/dashboard_app.tsx @@ -103,6 +103,7 @@ export interface DashboardAppScope extends ng.IScope { getEmbeddableFactory: (type: string) => EmbeddableFactory; getDashboardState: () => DashboardStateManager; refresh: () => void; + abortSignal?: AbortSignal; } const app = uiModules.get('app/dashboard', [ diff --git a/src/legacy/core_plugins/kibana/public/dashboard/dashboard_app_controller.tsx b/src/legacy/core_plugins/kibana/public/dashboard/dashboard_app_controller.tsx index 557c780f6192e..cf0fd66cfe114 100644 --- a/src/legacy/core_plugins/kibana/public/dashboard/dashboard_app_controller.tsx +++ b/src/legacy/core_plugins/kibana/public/dashboard/dashboard_app_controller.tsx @@ -128,6 +128,8 @@ export class DashboardAppController { const panelActionsRegistry = Private(ContextMenuActionsRegistryProvider); const getUnhashableStates = Private(getUnhashableStatesProvider); const shareContextMenuExtensions = Private(ShareContextMenuExtensionsRegistryProvider); + const abortController = new AbortController(); + $scope.abortSignal = abortController.signal; // @ts-ignore This code is going away shortly. panelActionsStore.initializeFromRegistry(panelActionsRegistry); @@ -567,6 +569,7 @@ export class DashboardAppController { updateSubscription.unsubscribe(); fetchSubscription.unsubscribe(); dashboardStateManager.destroy(); + abortController.abort(); }); if ( diff --git a/src/legacy/core_plugins/kibana/public/dashboard/grid/dashboard_grid.tsx b/src/legacy/core_plugins/kibana/public/dashboard/grid/dashboard_grid.tsx index 4ff38fd0fbfd3..77708dfa3cf01 100644 --- a/src/legacy/core_plugins/kibana/public/dashboard/grid/dashboard_grid.tsx +++ b/src/legacy/core_plugins/kibana/public/dashboard/grid/dashboard_grid.tsx @@ -121,6 +121,7 @@ interface Props extends ReactIntl.InjectedIntlProps { onPanelsUpdated: (updatedPanels: SavedDashboardPanelMap) => void; maximizedPanelId?: string; useMargins: boolean; + abortSignal?: AbortSignal; } interface State { @@ -251,6 +252,7 @@ class DashboardGridUi extends React.Component { diff --git a/src/legacy/core_plugins/kibana/public/dashboard/panel/dashboard_panel.tsx b/src/legacy/core_plugins/kibana/public/dashboard/panel/dashboard_panel.tsx index ec65a059236a8..237cb06e69b73 100644 --- a/src/legacy/core_plugins/kibana/public/dashboard/panel/dashboard_panel.tsx +++ b/src/legacy/core_plugins/kibana/public/dashboard/panel/dashboard_panel.tsx @@ -51,6 +51,7 @@ export interface DashboardPanelProps { initialized: boolean; panel: SavedDashboardPanel; className?: string; + abortSignal?: AbortSignal; } export interface DashboardPanelUiProps extends DashboardPanelProps { @@ -89,13 +90,14 @@ class DashboardPanelUi extends React.Component { embeddableStateChanged, embeddableIsInitialized, embeddableError, + abortSignal, } = this.props; if (!initialized) { embeddableIsInitializing(); embeddableFactory // @ts-ignore -- going away with Embeddable V2 - .create(panel, embeddableStateChanged) + .create(panel, embeddableStateChanged, abortSignal) .then((embeddable: Embeddable) => { if (this.mounted) { this.embeddable = embeddable; diff --git a/src/legacy/core_plugins/kibana/public/dashboard/viewport/dashboard_viewport.tsx b/src/legacy/core_plugins/kibana/public/dashboard/viewport/dashboard_viewport.tsx index 8e34b50a0383d..0eb72c03c6ebc 100644 --- a/src/legacy/core_plugins/kibana/public/dashboard/viewport/dashboard_viewport.tsx +++ b/src/legacy/core_plugins/kibana/public/dashboard/viewport/dashboard_viewport.tsx @@ -31,6 +31,7 @@ export function DashboardViewport({ useMargins, isFullScreenMode, onExitFullScreenMode, + abortSignal, }: { maximizedPanelId: string; getEmbeddableFactory: (panelType: string) => EmbeddableFactory; @@ -40,6 +41,7 @@ export function DashboardViewport({ useMargins: boolean; isFullScreenMode: boolean; onExitFullScreenMode: () => void; + abortSignal?: AbortSignal; }) { return (
); diff --git a/src/legacy/core_plugins/kibana/public/dashboard/viewport/dashboard_viewport_provider.js b/src/legacy/core_plugins/kibana/public/dashboard/viewport/dashboard_viewport_provider.js index a219f763dac28..37a5a01632802 100644 --- a/src/legacy/core_plugins/kibana/public/dashboard/viewport/dashboard_viewport_provider.js +++ b/src/legacy/core_plugins/kibana/public/dashboard/viewport/dashboard_viewport_provider.js @@ -33,4 +33,5 @@ export function DashboardViewportProvider(props) { DashboardViewportProvider.propTypes = { getEmbeddableFactory: PropTypes.func.isRequired, + abortSignal: PropTypes.object, }; diff --git a/src/legacy/core_plugins/kibana/public/visualize/embeddable/visualize_embeddable.ts b/src/legacy/core_plugins/kibana/public/visualize/embeddable/visualize_embeddable.ts index 9c3a2cbe7e407..366744b4af3a6 100644 --- a/src/legacy/core_plugins/kibana/public/visualize/embeddable/visualize_embeddable.ts +++ b/src/legacy/core_plugins/kibana/public/visualize/embeddable/visualize_embeddable.ts @@ -41,6 +41,7 @@ export interface VisualizeEmbeddableConfiguration { editUrl?: string; editable: boolean; loader: VisualizeLoader; + abortSignal?: AbortSignal; } export class VisualizeEmbeddable extends Embeddable { @@ -54,6 +55,7 @@ export class VisualizeEmbeddable extends Embeddable { private timeRange?: TimeRange; private query?: Query; private filters?: Filter[]; + private abortSignal?: AbortSignal; constructor({ onEmbeddableStateChanged, @@ -62,6 +64,7 @@ export class VisualizeEmbeddable extends Embeddable { editUrl, editable, loader, + abortSignal, }: VisualizeEmbeddableConfiguration) { super({ title: savedVisualization.title, @@ -75,6 +78,7 @@ export class VisualizeEmbeddable extends Embeddable { this.onEmbeddableStateChanged = onEmbeddableStateChanged; this.savedVisualization = savedVisualization; this.loader = loader; + this.abortSignal = abortSignal; const parsedUiState = savedVisualization.uiStateJSON ? JSON.parse(savedVisualization.uiStateJSON) @@ -184,6 +188,7 @@ export class VisualizeEmbeddable extends Embeddable { filters: containerState.filters, cssClass: `embPanel__content embPanel__content--fullWidth`, dataAttrs, + abortSignal: this.abortSignal, }; this.handler = this.loader.embedVisualizationWithSavedObject( diff --git a/src/legacy/core_plugins/kibana/public/visualize/embeddable/visualize_embeddable_factory.ts b/src/legacy/core_plugins/kibana/public/visualize/embeddable/visualize_embeddable_factory.ts index 910a99a703ab7..e35c7bbd065af 100644 --- a/src/legacy/core_plugins/kibana/public/visualize/embeddable/visualize_embeddable_factory.ts +++ b/src/legacy/core_plugins/kibana/public/visualize/embeddable/visualize_embeddable_factory.ts @@ -96,7 +96,8 @@ export class VisualizeEmbeddableFactory extends EmbeddableFactory { public readonly name: string; public readonly savedObjectMetaData?: SavedObjectMetaData; + protected abortSignal?: AbortSignal; /** * @@ -42,12 +43,15 @@ export abstract class EmbeddableFactory; + abortSignal?: AbortSignal; }) { this.name = name; this.savedObjectMetaData = savedObjectMetaData; + this.abortSignal = abortSignal; } /** @@ -61,6 +65,7 @@ export abstract class EmbeddableFactory; } From 04e1240fd3ab07290e88fac455b4d0f476fc693f Mon Sep 17 00:00:00 2001 From: ppisljar Date: Wed, 3 Jul 2019 10:42:03 +0200 Subject: [PATCH 04/23] passing AbortSignal to courier request handler --- src/legacy/core_plugins/interpreter/public/functions/esaggs.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/legacy/core_plugins/interpreter/public/functions/esaggs.ts b/src/legacy/core_plugins/interpreter/public/functions/esaggs.ts index 3bf414cf02211..db968b6c8c8b9 100644 --- a/src/legacy/core_plugins/interpreter/public/functions/esaggs.ts +++ b/src/legacy/core_plugins/interpreter/public/functions/esaggs.ts @@ -110,6 +110,7 @@ export const esaggs = (): ExpressionFunction Date: Tue, 9 Jul 2019 12:19:34 -0700 Subject: [PATCH 05/23] Remove abort signal from dashboard and move to handler, and abort fetches when necessary --- .../kibana/public/dashboard/dashboard_app.tsx | 1 - .../dashboard/dashboard_app_controller.tsx | 13 +++------ .../ui/public/vis/editors/default/default.js | 13 ++++----- .../ui/public/vis/request_handlers/courier.js | 9 ++++-- .../loader/embedded_visualize_handler.ts | 28 ++++++++----------- .../expressions/interpreter_provider.ts | 2 +- 6 files changed, 28 insertions(+), 38 deletions(-) diff --git a/src/legacy/core_plugins/kibana/public/dashboard/dashboard_app.tsx b/src/legacy/core_plugins/kibana/public/dashboard/dashboard_app.tsx index 5f127e865451b..68a293de167f2 100644 --- a/src/legacy/core_plugins/kibana/public/dashboard/dashboard_app.tsx +++ b/src/legacy/core_plugins/kibana/public/dashboard/dashboard_app.tsx @@ -103,7 +103,6 @@ export interface DashboardAppScope extends ng.IScope { getEmbeddableFactory: (type: string) => EmbeddableFactory; getDashboardState: () => DashboardStateManager; refresh: () => void; - abortSignal?: AbortSignal; } const app = uiModules.get('app/dashboard', [ diff --git a/src/legacy/core_plugins/kibana/public/dashboard/dashboard_app_controller.tsx b/src/legacy/core_plugins/kibana/public/dashboard/dashboard_app_controller.tsx index cf0fd66cfe114..e3d1f14a1616e 100644 --- a/src/legacy/core_plugins/kibana/public/dashboard/dashboard_app_controller.tsx +++ b/src/legacy/core_plugins/kibana/public/dashboard/dashboard_app_controller.tsx @@ -128,8 +128,6 @@ export class DashboardAppController { const panelActionsRegistry = Private(ContextMenuActionsRegistryProvider); const getUnhashableStates = Private(getUnhashableStatesProvider); const shareContextMenuExtensions = Private(ShareContextMenuExtensionsRegistryProvider); - const abortController = new AbortController(); - $scope.abortSignal = abortController.signal; // @ts-ignore This code is going away shortly. panelActionsStore.initializeFromRegistry(panelActionsRegistry); @@ -251,17 +249,15 @@ export class DashboardAppController { !dashboardConfig.getHideWriteControls(); $scope.updateQueryAndFetch = function({ query, dateRange }) { - if (dateRange) { - timefilter.setTime(dateRange); - } - - const oldQuery = $scope.model.query; - if (_.isEqual(oldQuery, query)) { + if (_.isEqual($scope.model.query, query) && _.isEqual($scope.model.timeRange, dateRange)) { // The user can still request a reload in the query bar, even if the // query is the same, and in that case, we have to explicitly ask for // a reload, since no state changes will cause it. dashboardStateManager.requestReload(); } else { + if (dateRange) { + timefilter.setTime(dateRange); + } $scope.model.query = query; dashboardStateManager.applyFilters($scope.model.query, $scope.model.filters); } @@ -569,7 +565,6 @@ export class DashboardAppController { updateSubscription.unsubscribe(); fetchSubscription.unsubscribe(); dashboardStateManager.destroy(); - abortController.abort(); }); if ( diff --git a/src/legacy/ui/public/vis/editors/default/default.js b/src/legacy/ui/public/vis/editors/default/default.js index a58263d3e5b62..18afcbb58bac3 100644 --- a/src/legacy/ui/public/vis/editors/default/default.js +++ b/src/legacy/ui/public/vis/editors/default/default.js @@ -164,18 +164,15 @@ const defaultEditor = function ($rootScope, $compile) { } this._loader = loader; this._handler = this._loader.embedVisualizationWithSavedObject(visualizationEl, this.savedObj, { - uiState: uiState, listenOnChange: false, - timeRange: timeRange, - filters: filters, - appState: appState, + uiState, + timeRange, + filters, + appState, }); }); } else { - this._handler.update({ - timeRange: timeRange, - filters: filters, - }); + this._handler.update({ timeRange, filters }); } }); diff --git a/src/legacy/ui/public/vis/request_handlers/courier.js b/src/legacy/ui/public/vis/request_handlers/courier.js index 80f8425340197..32d006441ff01 100644 --- a/src/legacy/ui/public/vis/request_handlers/courier.js +++ b/src/legacy/ui/public/vis/request_handlers/courier.js @@ -40,9 +40,9 @@ 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. // This is a temporary solution until we properly pass down all required @@ -102,6 +102,11 @@ const CourierRequestHandlerProvider = function () { request.stats(getRequestInspectorStats(requestSearchSource)); try { + if (abortSignal) { + abortSignal.addEventListener('abort', () => { + requestSearchSource.cancelQueued(); + }); + } const response = await requestSearchSource.fetch(); searchSource.lastQuery = queryHash; 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 00bfb754085c4..9987451859206 100644 --- a/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.ts +++ b/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.ts @@ -55,7 +55,6 @@ interface EmbeddedVisualizeHandlerParams extends VisualizeLoaderParams { queryFilter: any; autoFetch?: boolean; pipelineDataLoader?: boolean; - abortSignal?: AbortSignal; } const RENDER_COMPLETE_EVENT = 'render_complete'; @@ -94,6 +93,12 @@ export class EmbeddedVisualizeHandler { const forceFetch = this.shouldForceNextFetch; this.shouldForceNextFetch = false; + + // Abort any in-progress requests + if (this.abortController) this.abortController.abort(); + this.abortController = new AbortController(); + this.dataLoaderParams.abortSignal = this.abortController.signal; + this.fetch(forceFetch).then(this.render); }, 100); @@ -105,6 +110,7 @@ export class EmbeddedVisualizeHandler { private actions: any = {}; private events$: Rx.Observable; private autoFetch: boolean; + private abortController?: AbortController; constructor( private readonly element: HTMLElement, @@ -123,7 +129,6 @@ export class EmbeddedVisualizeHandler { autoFetch = true, pipelineDataLoader = false, Private, - abortSignal, } = params; this.dataLoaderParams = { @@ -135,7 +140,6 @@ export class EmbeddedVisualizeHandler { uiState, aggs: vis.getAggConfig(), forceFetch: false, - abortSignal, }; this.pipelineDataLoader = pipelineDataLoader; @@ -226,7 +230,7 @@ export class EmbeddedVisualizeHandler { */ public update(params: VisualizeUpdateParams = {}) { // Apply data- attributes to the element if specified - const dataAttrs = params.dataAttrs; + const { dataAttrs, ...otherParams } = params; if (dataAttrs) { Object.keys(dataAttrs).forEach(key => { if (dataAttrs[key] === null) { @@ -238,19 +242,8 @@ export class EmbeddedVisualizeHandler { }); } - let fetchRequired = false; - if (params.hasOwnProperty('timeRange')) { - fetchRequired = true; - this.dataLoaderParams.timeRange = params.timeRange; - } - if (params.hasOwnProperty('filters')) { - fetchRequired = true; - this.dataLoaderParams.filters = params.filters; - } - if (params.hasOwnProperty('query')) { - fetchRequired = true; - this.dataLoaderParams.query = params.query; - } + const fetchRequired = ['timeRange', 'filters', 'query'].some(key => params.hasOwnProperty(key)); + this.dataLoaderParams = { ...this.dataLoaderParams, ...otherParams }; if (fetchRequired) { this.fetchAndRender(); @@ -263,6 +256,7 @@ export class EmbeddedVisualizeHandler { */ public destroy(): void { this.destroyed = true; + if (this.abortController) this.abortController.abort(); this.debouncedFetchAndRender.cancel(); if (this.autoFetch) { timefilter.off('autoRefreshFetch', this.reload); diff --git a/src/plugins/data/common/expressions/interpreter_provider.ts b/src/plugins/data/common/expressions/interpreter_provider.ts index 94cc73b22380a..3f1d8ab7091a2 100644 --- a/src/plugins/data/common/expressions/interpreter_provider.ts +++ b/src/plugins/data/common/expressions/interpreter_provider.ts @@ -73,7 +73,7 @@ export function interpreterProvider(config: any) { // if execution was aborted return error if (handlers.abortSignal && handlers.abortSignal.aborted) { - return createError(`abort was requested`); + return new Error(`abort was requested`); } // Continue re-invoking chain until it's empty From 095860eae53c3f1e4f8e97f8d4fdddf29b5fa7e7 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Tue, 9 Jul 2019 12:33:23 -0700 Subject: [PATCH 06/23] Remove the rest of the references to abort signal in dashboard --- .../kibana/public/dashboard/dashboard_app.html | 1 - .../kibana/public/dashboard/grid/dashboard_grid.tsx | 2 -- .../kibana/public/dashboard/panel/dashboard_panel.tsx | 4 +--- .../public/dashboard/viewport/dashboard_viewport.tsx | 3 --- .../dashboard/viewport/dashboard_viewport_provider.js | 1 - .../public/visualize/embeddable/visualize_embeddable.ts | 5 ----- .../visualize/embeddable/visualize_embeddable_factory.ts | 4 +--- src/legacy/ui/public/embeddable/embeddable_factory.ts | 7 +------ 8 files changed, 3 insertions(+), 24 deletions(-) diff --git a/src/legacy/core_plugins/kibana/public/dashboard/dashboard_app.html b/src/legacy/core_plugins/kibana/public/dashboard/dashboard_app.html index 30223c6d41761..d73e15287077c 100644 --- a/src/legacy/core_plugins/kibana/public/dashboard/dashboard_app.html +++ b/src/legacy/core_plugins/kibana/public/dashboard/dashboard_app.html @@ -120,7 +120,6 @@ diff --git a/src/legacy/core_plugins/kibana/public/dashboard/grid/dashboard_grid.tsx b/src/legacy/core_plugins/kibana/public/dashboard/grid/dashboard_grid.tsx index 77708dfa3cf01..4ff38fd0fbfd3 100644 --- a/src/legacy/core_plugins/kibana/public/dashboard/grid/dashboard_grid.tsx +++ b/src/legacy/core_plugins/kibana/public/dashboard/grid/dashboard_grid.tsx @@ -121,7 +121,6 @@ interface Props extends ReactIntl.InjectedIntlProps { onPanelsUpdated: (updatedPanels: SavedDashboardPanelMap) => void; maximizedPanelId?: string; useMargins: boolean; - abortSignal?: AbortSignal; } interface State { @@ -252,7 +251,6 @@ class DashboardGridUi extends React.Component { diff --git a/src/legacy/core_plugins/kibana/public/dashboard/panel/dashboard_panel.tsx b/src/legacy/core_plugins/kibana/public/dashboard/panel/dashboard_panel.tsx index 237cb06e69b73..ec65a059236a8 100644 --- a/src/legacy/core_plugins/kibana/public/dashboard/panel/dashboard_panel.tsx +++ b/src/legacy/core_plugins/kibana/public/dashboard/panel/dashboard_panel.tsx @@ -51,7 +51,6 @@ export interface DashboardPanelProps { initialized: boolean; panel: SavedDashboardPanel; className?: string; - abortSignal?: AbortSignal; } export interface DashboardPanelUiProps extends DashboardPanelProps { @@ -90,14 +89,13 @@ class DashboardPanelUi extends React.Component { embeddableStateChanged, embeddableIsInitialized, embeddableError, - abortSignal, } = this.props; if (!initialized) { embeddableIsInitializing(); embeddableFactory // @ts-ignore -- going away with Embeddable V2 - .create(panel, embeddableStateChanged, abortSignal) + .create(panel, embeddableStateChanged) .then((embeddable: Embeddable) => { if (this.mounted) { this.embeddable = embeddable; diff --git a/src/legacy/core_plugins/kibana/public/dashboard/viewport/dashboard_viewport.tsx b/src/legacy/core_plugins/kibana/public/dashboard/viewport/dashboard_viewport.tsx index 0eb72c03c6ebc..8e34b50a0383d 100644 --- a/src/legacy/core_plugins/kibana/public/dashboard/viewport/dashboard_viewport.tsx +++ b/src/legacy/core_plugins/kibana/public/dashboard/viewport/dashboard_viewport.tsx @@ -31,7 +31,6 @@ export function DashboardViewport({ useMargins, isFullScreenMode, onExitFullScreenMode, - abortSignal, }: { maximizedPanelId: string; getEmbeddableFactory: (panelType: string) => EmbeddableFactory; @@ -41,7 +40,6 @@ export function DashboardViewport({ useMargins: boolean; isFullScreenMode: boolean; onExitFullScreenMode: () => void; - abortSignal?: AbortSignal; }) { return (
); diff --git a/src/legacy/core_plugins/kibana/public/dashboard/viewport/dashboard_viewport_provider.js b/src/legacy/core_plugins/kibana/public/dashboard/viewport/dashboard_viewport_provider.js index 37a5a01632802..a219f763dac28 100644 --- a/src/legacy/core_plugins/kibana/public/dashboard/viewport/dashboard_viewport_provider.js +++ b/src/legacy/core_plugins/kibana/public/dashboard/viewport/dashboard_viewport_provider.js @@ -33,5 +33,4 @@ export function DashboardViewportProvider(props) { DashboardViewportProvider.propTypes = { getEmbeddableFactory: PropTypes.func.isRequired, - abortSignal: PropTypes.object, }; diff --git a/src/legacy/core_plugins/kibana/public/visualize/embeddable/visualize_embeddable.ts b/src/legacy/core_plugins/kibana/public/visualize/embeddable/visualize_embeddable.ts index 366744b4af3a6..9c3a2cbe7e407 100644 --- a/src/legacy/core_plugins/kibana/public/visualize/embeddable/visualize_embeddable.ts +++ b/src/legacy/core_plugins/kibana/public/visualize/embeddable/visualize_embeddable.ts @@ -41,7 +41,6 @@ export interface VisualizeEmbeddableConfiguration { editUrl?: string; editable: boolean; loader: VisualizeLoader; - abortSignal?: AbortSignal; } export class VisualizeEmbeddable extends Embeddable { @@ -55,7 +54,6 @@ export class VisualizeEmbeddable extends Embeddable { private timeRange?: TimeRange; private query?: Query; private filters?: Filter[]; - private abortSignal?: AbortSignal; constructor({ onEmbeddableStateChanged, @@ -64,7 +62,6 @@ export class VisualizeEmbeddable extends Embeddable { editUrl, editable, loader, - abortSignal, }: VisualizeEmbeddableConfiguration) { super({ title: savedVisualization.title, @@ -78,7 +75,6 @@ export class VisualizeEmbeddable extends Embeddable { this.onEmbeddableStateChanged = onEmbeddableStateChanged; this.savedVisualization = savedVisualization; this.loader = loader; - this.abortSignal = abortSignal; const parsedUiState = savedVisualization.uiStateJSON ? JSON.parse(savedVisualization.uiStateJSON) @@ -188,7 +184,6 @@ export class VisualizeEmbeddable extends Embeddable { filters: containerState.filters, cssClass: `embPanel__content embPanel__content--fullWidth`, dataAttrs, - abortSignal: this.abortSignal, }; this.handler = this.loader.embedVisualizationWithSavedObject( diff --git a/src/legacy/core_plugins/kibana/public/visualize/embeddable/visualize_embeddable_factory.ts b/src/legacy/core_plugins/kibana/public/visualize/embeddable/visualize_embeddable_factory.ts index e35c7bbd065af..910a99a703ab7 100644 --- a/src/legacy/core_plugins/kibana/public/visualize/embeddable/visualize_embeddable_factory.ts +++ b/src/legacy/core_plugins/kibana/public/visualize/embeddable/visualize_embeddable_factory.ts @@ -96,8 +96,7 @@ export class VisualizeEmbeddableFactory extends EmbeddableFactory { public readonly name: string; public readonly savedObjectMetaData?: SavedObjectMetaData; - protected abortSignal?: AbortSignal; /** * @@ -43,15 +42,12 @@ export abstract class EmbeddableFactory; - abortSignal?: AbortSignal; }) { this.name = name; this.savedObjectMetaData = savedObjectMetaData; - this.abortSignal = abortSignal; } /** @@ -65,7 +61,6 @@ export abstract class EmbeddableFactory; } From 3c7e3441a20bf322ea5c7d6ea293ee23e09f9c20 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Wed, 10 Jul 2019 08:36:44 -0700 Subject: [PATCH 07/23] Revert changes to dashboard_app --- .../public/dashboard/dashboard_app_controller.tsx | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/legacy/core_plugins/kibana/public/dashboard/dashboard_app_controller.tsx b/src/legacy/core_plugins/kibana/public/dashboard/dashboard_app_controller.tsx index e3d1f14a1616e..557c780f6192e 100644 --- a/src/legacy/core_plugins/kibana/public/dashboard/dashboard_app_controller.tsx +++ b/src/legacy/core_plugins/kibana/public/dashboard/dashboard_app_controller.tsx @@ -249,15 +249,17 @@ export class DashboardAppController { !dashboardConfig.getHideWriteControls(); $scope.updateQueryAndFetch = function({ query, dateRange }) { - if (_.isEqual($scope.model.query, query) && _.isEqual($scope.model.timeRange, dateRange)) { + if (dateRange) { + timefilter.setTime(dateRange); + } + + const oldQuery = $scope.model.query; + if (_.isEqual(oldQuery, query)) { // The user can still request a reload in the query bar, even if the // query is the same, and in that case, we have to explicitly ask for // a reload, since no state changes will cause it. dashboardStateManager.requestReload(); } else { - if (dateRange) { - timefilter.setTime(dateRange); - } $scope.model.query = query; dashboardStateManager.applyFilters($scope.model.query, $scope.model.filters); } From 4af0b6403675335510044d3d3ca0b149dcbe4849 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Tue, 16 Jul 2019 09:14:09 -0700 Subject: [PATCH 08/23] Remove code related to canceling visualize requests and only keep stuff for canceling interpreter --- .../interpreter/public/functions/esaggs.ts | 1 - .../ui/public/vis/editors/default/default.js | 13 ++++++---- .../ui/public/vis/request_handlers/courier.js | 8 +----- .../request_handlers/request_handlers.d.ts | 1 - .../loader/embedded_visualize_handler.ts | 25 +++++++++++-------- .../visualize/loader/pipeline_data_loader.ts | 1 - .../loader/pipeline_helpers/run_pipeline.ts | 1 - .../ui/public/visualize/loader/types.ts | 5 ---- .../expressions/interpreter_provider.ts | 2 +- 9 files changed, 24 insertions(+), 33 deletions(-) diff --git a/src/legacy/core_plugins/interpreter/public/functions/esaggs.ts b/src/legacy/core_plugins/interpreter/public/functions/esaggs.ts index db968b6c8c8b9..3bf414cf02211 100644 --- a/src/legacy/core_plugins/interpreter/public/functions/esaggs.ts +++ b/src/legacy/core_plugins/interpreter/public/functions/esaggs.ts @@ -110,7 +110,6 @@ export const esaggs = (): ExpressionFunction { - requestSearchSource.cancelQueued(); - }); - } const response = await requestSearchSource.fetch(); searchSource.lastQuery = queryHash; 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 71c99cc1d2b2b..f2475065a869c 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,7 +40,6 @@ 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.ts b/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.ts index 9987451859206..431c5ae9be6a1 100644 --- a/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.ts +++ b/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.ts @@ -93,12 +93,6 @@ export class EmbeddedVisualizeHandler { const forceFetch = this.shouldForceNextFetch; this.shouldForceNextFetch = false; - - // Abort any in-progress requests - if (this.abortController) this.abortController.abort(); - this.abortController = new AbortController(); - this.dataLoaderParams.abortSignal = this.abortController.signal; - this.fetch(forceFetch).then(this.render); }, 100); @@ -110,7 +104,6 @@ export class EmbeddedVisualizeHandler { private actions: any = {}; private events$: Rx.Observable; private autoFetch: boolean; - private abortController?: AbortController; constructor( private readonly element: HTMLElement, @@ -230,7 +223,7 @@ export class EmbeddedVisualizeHandler { */ public update(params: VisualizeUpdateParams = {}) { // Apply data- attributes to the element if specified - const { dataAttrs, ...otherParams } = params; + const dataAttrs = params.dataAttrs; if (dataAttrs) { Object.keys(dataAttrs).forEach(key => { if (dataAttrs[key] === null) { @@ -242,8 +235,19 @@ export class EmbeddedVisualizeHandler { }); } - const fetchRequired = ['timeRange', 'filters', 'query'].some(key => params.hasOwnProperty(key)); - this.dataLoaderParams = { ...this.dataLoaderParams, ...otherParams }; + let fetchRequired = false; + if (params.hasOwnProperty('timeRange')) { + fetchRequired = true; + this.dataLoaderParams.timeRange = params.timeRange; + } + if (params.hasOwnProperty('filters')) { + fetchRequired = true; + this.dataLoaderParams.filters = params.filters; + } + if (params.hasOwnProperty('query')) { + fetchRequired = true; + this.dataLoaderParams.query = params.query; + } if (fetchRequired) { this.fetchAndRender(); @@ -256,7 +260,6 @@ export class EmbeddedVisualizeHandler { */ public destroy(): void { this.destroyed = true; - if (this.abortController) this.abortController.abort(); this.debouncedFetchAndRender.cancel(); if (this.autoFetch) { timefilter.off('autoRefreshFetch', this.reload); 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 df78fc440b81e..e9ccaf75e63f9 100644 --- a/src/legacy/ui/public/visualize/loader/pipeline_data_loader.ts +++ b/src/legacy/ui/public/visualize/loader/pipeline_data_loader.ts @@ -39,7 +39,6 @@ export class PipelineDataLoader { : undefined, }), inspectorAdapters: params.inspectorAdapters, - abortSignal: params.abortSignal, } ); } 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 43dbb06f8c973..1551ab755ea5b 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,7 +30,6 @@ 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/types.ts b/src/legacy/ui/public/visualize/loader/types.ts index fb7261670a051..4f34f6219621f 100644 --- a/src/legacy/ui/public/visualize/loader/types.ts +++ b/src/legacy/ui/public/visualize/loader/types.ts @@ -98,11 +98,6 @@ export interface VisualizeLoaderParams { * cycle happens. Default value: `true` */ autoFetch?: boolean; - - /** - * AbortSignal can be provided which allows canceling ongoing process - */ - abortSignal?: AbortSignal; } /** diff --git a/src/plugins/data/common/expressions/interpreter_provider.ts b/src/plugins/data/common/expressions/interpreter_provider.ts index 3f1d8ab7091a2..80ce5cebbd5f2 100644 --- a/src/plugins/data/common/expressions/interpreter_provider.ts +++ b/src/plugins/data/common/expressions/interpreter_provider.ts @@ -73,7 +73,7 @@ export function interpreterProvider(config: any) { // if execution was aborted return error if (handlers.abortSignal && handlers.abortSignal.aborted) { - return new Error(`abort was requested`); + return new Error(`Expression was aborted.`); } // Continue re-invoking chain until it's empty From d8f5208efcc84af036df9de7c4aeec190cd3b2dd Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Thu, 25 Jul 2019 12:44:35 -0700 Subject: [PATCH 09/23] Use createError --- .../data/public/expressions/expressions_service.ts | 1 + src/plugins/data/common/expressions/create_error.ts | 1 + .../data/common/expressions/interpreter_provider.ts | 7 +++++-- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/legacy/core_plugins/data/public/expressions/expressions_service.ts b/src/legacy/core_plugins/data/public/expressions/expressions_service.ts index bb2ca806bbac3..043e1fc74c530 100644 --- a/src/legacy/core_plugins/data/public/expressions/expressions_service.ts +++ b/src/legacy/core_plugins/data/public/expressions/expressions_service.ts @@ -41,6 +41,7 @@ export type getInitialContextFunction = () => InitialContextObject; export interface Handlers { getInitialContext: getInitialContextFunction; inspectorAdapters?: Adapters; + abortSignal?: AbortSignal; } type Context = object; diff --git a/src/plugins/data/common/expressions/create_error.ts b/src/plugins/data/common/expressions/create_error.ts index 78a12cb07c663..cee288e5e1b35 100644 --- a/src/plugins/data/common/expressions/create_error.ts +++ b/src/plugins/data/common/expressions/create_error.ts @@ -22,5 +22,6 @@ export const createError = (err: any) => ({ error: { stack: process.env.NODE_ENV === 'production' ? undefined : err.stack, message: typeof err === 'string' ? err : err.message, + name: (err && err.name) || 'Error', }, }); diff --git a/src/plugins/data/common/expressions/interpreter_provider.ts b/src/plugins/data/common/expressions/interpreter_provider.ts index 80ce5cebbd5f2..cb025e22131c5 100644 --- a/src/plugins/data/common/expressions/interpreter_provider.ts +++ b/src/plugins/data/common/expressions/interpreter_provider.ts @@ -73,11 +73,14 @@ export function interpreterProvider(config: any) { // if execution was aborted return error if (handlers.abortSignal && handlers.abortSignal.aborted) { - return new Error(`Expression was aborted.`); + return createError({ + message: 'The expression was aborted.', + name: 'AbortError', + }); } // Continue re-invoking chain until it's empty - return await invokeChain(chain, newContext); + return invokeChain(chain, newContext); } catch (e) { // Everything that throws from a function will hit this // The interpreter should *never* fail. It should always return a `{type: error}` on failure From 00a672d56f24c97be26950d854ea5cfbae2349e2 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Thu, 25 Jul 2019 16:40:59 -0700 Subject: [PATCH 10/23] Cancel in-progress fetches when a new one is requested or when leaving the page --- .../core_plugins/interpreter/public/functions/esaggs.ts | 5 +++-- src/legacy/ui/public/vis/request_handlers/courier.js | 8 +++++++- .../ui/public/vis/request_handlers/request_handlers.d.ts | 1 + .../public/visualize/loader/embedded_visualize_handler.ts | 7 +++++++ .../ui/public/visualize/loader/pipeline_data_loader.ts | 1 + .../visualize/loader/pipeline_helpers/run_pipeline.ts | 1 + 6 files changed, 20 insertions(+), 3 deletions(-) diff --git a/src/legacy/core_plugins/interpreter/public/functions/esaggs.ts b/src/legacy/core_plugins/interpreter/public/functions/esaggs.ts index 3bf414cf02211..72d398fec7d20 100644 --- a/src/legacy/core_plugins/interpreter/public/functions/esaggs.ts +++ b/src/legacy/core_plugins/interpreter/public/functions/esaggs.ts @@ -83,7 +83,7 @@ export const esaggs = (): ExpressionFunction requestSearchSource.cancelQueued()); + } + const response = await requestSearchSource.fetch(); searchSource.lastQuery = queryHash; 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 f2475065a869c..71c99cc1d2b2b 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.ts b/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.ts index 431c5ae9be6a1..50fdf79d8007d 100644 --- a/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.ts +++ b/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.ts @@ -104,6 +104,7 @@ export class EmbeddedVisualizeHandler { private actions: any = {}; private events$: Rx.Observable; private autoFetch: boolean; + private abortController?: AbortController; constructor( private readonly element: HTMLElement, @@ -260,6 +261,7 @@ export class EmbeddedVisualizeHandler { */ public destroy(): void { this.destroyed = true; + if (this.abortController) this.abortController.abort(); this.debouncedFetchAndRender.cancel(); if (this.autoFetch) { timefilter.off('autoRefreshFetch', this.reload); @@ -436,6 +438,11 @@ export class EmbeddedVisualizeHandler { }; private fetch = (forceFetch: boolean = false) => { + // Abort any in-progress requests + if (this.abortController) this.abortController.abort(); + 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; 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..df78fc440b81e 100644 --- a/src/legacy/ui/public/visualize/loader/pipeline_data_loader.ts +++ b/src/legacy/ui/public/visualize/loader/pipeline_data_loader.ts @@ -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/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 ( From b37525f3ac2eb50a61031a7cefc8ff99b07ccc8b Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Wed, 31 Jul 2019 09:05:14 -0700 Subject: [PATCH 11/23] Update with cancel methods and make visualize data loader work --- .../visualize/loader/embedded_visualize_handler.ts | 12 ++++++------ .../public/visualize/loader/pipeline_data_loader.ts | 10 +++++++++- .../public/visualize/loader/visualize_data_loader.ts | 9 +++++++++ 3 files changed, 24 insertions(+), 7 deletions(-) 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 50fdf79d8007d..7e785181fa465 100644 --- a/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.ts +++ b/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.ts @@ -104,7 +104,6 @@ export class EmbeddedVisualizeHandler { private actions: any = {}; private events$: Rx.Observable; private autoFetch: boolean; - private abortController?: AbortController; constructor( private readonly element: HTMLElement, @@ -261,7 +260,7 @@ export class EmbeddedVisualizeHandler { */ public destroy(): void { this.destroyed = true; - if (this.abortController) this.abortController.abort(); + this.cancel(); this.debouncedFetchAndRender.cancel(); if (this.autoFetch) { timefilter.off('autoRefreshFetch', this.reload); @@ -437,11 +436,12 @@ export class EmbeddedVisualizeHandler { this.fetchAndRender(); }; + private cancel = () => { + this.dataLoader.cancel(); + }; + private fetch = (forceFetch: boolean = false) => { - // Abort any in-progress requests - if (this.abortController) this.abortController.abort(); - this.abortController = new AbortController(); - this.dataLoaderParams.abortSignal = this.abortController.signal; + this.cancel(); this.dataLoaderParams.aggs = this.vis.getAggConfig(); this.dataLoaderParams.forceFetch = forceFetch; 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 df78fc440b81e..08f8eea6e4f85 100644 --- a/src/legacy/ui/public/visualize/loader/pipeline_data_loader.ts +++ b/src/legacy/ui/public/visualize/loader/pipeline_data_loader.ts @@ -21,9 +21,17 @@ import { RequestHandlerParams, Vis } from '../../vis'; import { buildPipeline, runPipeline } from './pipeline_helpers'; export class PipelineDataLoader { + private abortController?: AbortController; + constructor(private readonly vis: Vis) {} + public cancel() { + if (this.abortController) this.abortController.abort(); + } + public async fetch(params: RequestHandlerParams): Promise { + this.cancel(); + this.abortController = new AbortController(); this.vis.pipelineExpression = await buildPipeline(this.vis, params); return await runPipeline( @@ -39,7 +47,7 @@ export class PipelineDataLoader { : undefined, }), inspectorAdapters: params.inspectorAdapters, - abortSignal: params.abortSignal, + abortSignal: this.abortController.signal, } ); } 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..15ce755457df2 100644 --- a/src/legacy/ui/public/visualize/loader/visualize_data_loader.ts +++ b/src/legacy/ui/public/visualize/loader/visualize_data_loader.ts @@ -55,6 +55,7 @@ export class VisualizeDataLoader { private visData: any; private previousVisState: any; private previousRequestHandlerResponse: any; + private abortController?: AbortController; constructor(private readonly vis: Vis, Private: IPrivate) { const { requestHandler, responseHandler } = vis.type; @@ -65,7 +66,14 @@ export class VisualizeDataLoader { this.responseHandler = getHandler(responseHandlers, responseHandler); } + public cancel(): void { + if (this.abortController) this.abortController.abort(); + } + public async fetch(params: RequestHandlerParams): Promise { + this.cancel(); + this.abortController = new AbortController(); + // add necessary params to vis object (dimensions, bucket, metric, etc) const visParams = await getVisParams(this.vis, { searchSource: params.searchSource, @@ -85,6 +93,7 @@ export class VisualizeDataLoader { ...params, query, filters: filters.concat(savedFilters).filter(f => !f.meta.disabled), + abortSignal: this.abortController.signal, }); // No need to call the response handler when there have been no data nor has there been changes From dd784a3e8bf7b7394a29ce9503303ca8dcee4cf4 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Thu, 1 Aug 2019 09:01:43 -0700 Subject: [PATCH 12/23] Remove unnecessary call to --- .../ui/public/visualize/loader/embedded_visualize_handler.ts | 2 -- 1 file changed, 2 deletions(-) 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 7e785181fa465..50368e23c9ba4 100644 --- a/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.ts +++ b/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.ts @@ -441,8 +441,6 @@ export class EmbeddedVisualizeHandler { }; private fetch = (forceFetch: boolean = false) => { - this.cancel(); - this.dataLoaderParams.aggs = this.vis.getAggConfig(); this.dataLoaderParams.forceFetch = forceFetch; this.dataLoaderParams.inspectorAdapters = this.inspectorAdapters; From cb2fb2dbaa8abb360b919b63928993e03607eebe Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Fri, 2 Aug 2019 13:41:09 -0700 Subject: [PATCH 13/23] Fix tests --- .../loader/embedded_visualize_handler.test.mocks.ts | 3 +++ .../loader/embedded_visualize_handler.test.ts | 11 ++++++++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.test.mocks.ts b/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.test.mocks.ts index 6adea30c94fb4..82407e1d4feee 100644 --- a/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.test.mocks.ts +++ b/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.test.mocks.ts @@ -53,10 +53,13 @@ export const mockDataLoaderFetch = jest.fn().mockReturnValue({ params: {}, }, }); +export const mockDataLoaderCancel = jest.fn(); + const MockDataLoader = class { public async fetch(data: any) { return await mockDataLoaderFetch(data); } + public cancel = mockDataLoaderCancel; }; jest.mock('./pipeline_data_loader', () => ({ 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 4e41dd54855ea..98ccedfe916e7 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 @@ -16,7 +16,11 @@ * specific language governing permissions and limitations * under the License. */ -import { mockDataLoaderFetch, timefilter } from './embedded_visualize_handler.test.mocks'; +import { + mockDataLoaderFetch, + mockDataLoaderCancel, + timefilter, +} from './embedded_visualize_handler.test.mocks'; // @ts-ignore import MockState from '../../../../../fixtures/mock_state'; @@ -197,6 +201,11 @@ describe('EmbeddedVisualizeHandler', () => { handler.destroy(); expect(spy).toHaveBeenCalledTimes(1); }); + + it('should call cancel on data loader', () => { + handler.destroy(); + expect(mockDataLoaderCancel).toHaveBeenCalled(); + }); }); describe('openInspector', () => { From 3f0b86fc1f501eb7f0c39dd5f6e7e43feec26587 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Tue, 6 Aug 2019 18:14:21 -0700 Subject: [PATCH 14/23] Remove cancel from data loaders --- .../visualize/loader/embedded_visualize_handler.ts | 6 +++++- .../public/visualize/loader/pipeline_data_loader.ts | 12 ++---------- .../public/visualize/loader/visualize_data_loader.ts | 10 +--------- 3 files changed, 8 insertions(+), 20 deletions(-) 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 50368e23c9ba4..452ea03c9b235 100644 --- a/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.ts +++ b/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.ts @@ -104,6 +104,7 @@ export class EmbeddedVisualizeHandler { private actions: any = {}; private events$: Rx.Observable; private autoFetch: boolean; + private abortController?: AbortController; constructor( private readonly element: HTMLElement, @@ -437,10 +438,13 @@ export class EmbeddedVisualizeHandler { }; private cancel = () => { - this.dataLoader.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; 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 08f8eea6e4f85..41bee473937de 100644 --- a/src/legacy/ui/public/visualize/loader/pipeline_data_loader.ts +++ b/src/legacy/ui/public/visualize/loader/pipeline_data_loader.ts @@ -21,20 +21,12 @@ import { RequestHandlerParams, Vis } from '../../vis'; import { buildPipeline, runPipeline } from './pipeline_helpers'; export class PipelineDataLoader { - private abortController?: AbortController; - constructor(private readonly vis: Vis) {} - public cancel() { - if (this.abortController) this.abortController.abort(); - } - public async fetch(params: RequestHandlerParams): Promise { - this.cancel(); - this.abortController = new AbortController(); this.vis.pipelineExpression = await buildPipeline(this.vis, params); - return await runPipeline( + return runPipeline( this.vis.pipelineExpression, {}, { @@ -47,7 +39,7 @@ export class PipelineDataLoader { : undefined, }), inspectorAdapters: params.inspectorAdapters, - abortSignal: this.abortController.signal, + abortSignal: params.abortSignal, } ); } 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 15ce755457df2..3352e1284591c 100644 --- a/src/legacy/ui/public/visualize/loader/visualize_data_loader.ts +++ b/src/legacy/ui/public/visualize/loader/visualize_data_loader.ts @@ -55,7 +55,6 @@ export class VisualizeDataLoader { private visData: any; private previousVisState: any; private previousRequestHandlerResponse: any; - private abortController?: AbortController; constructor(private readonly vis: Vis, Private: IPrivate) { const { requestHandler, responseHandler } = vis.type; @@ -66,14 +65,7 @@ export class VisualizeDataLoader { this.responseHandler = getHandler(responseHandlers, responseHandler); } - public cancel(): void { - if (this.abortController) this.abortController.abort(); - } - public async fetch(params: RequestHandlerParams): Promise { - this.cancel(); - this.abortController = new AbortController(); - // add necessary params to vis object (dimensions, bucket, metric, etc) const visParams = await getVisParams(this.vis, { searchSource: params.searchSource, @@ -93,7 +85,7 @@ export class VisualizeDataLoader { ...params, query, filters: filters.concat(savedFilters).filter(f => !f.meta.disabled), - abortSignal: this.abortController.signal, + abortSignal: params.abortSignal, }); // No need to call the response handler when there have been no data nor has there been changes From b1ec8e41a624799c1e27b985abc689bc46cc1021 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Wed, 7 Aug 2019 17:16:04 -0700 Subject: [PATCH 15/23] Fix search source to always either resolve or reject in fetch() --- .../request/search_request/search_request.js | 4 +++- .../courier/search_source/search_source.js | 7 ++++++- .../loader/embedded_visualize_handler.ts | 3 +++ .../common/expressions/interpreter_provider.ts | 16 ++++++++-------- 4 files changed, 20 insertions(+), 10 deletions(-) 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..e44d88720353e 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 @@ -170,7 +170,9 @@ export function SearchRequestProvider(Promise) { this._markStopped(); this.defer = null; this.aborted = true; - this.abortedDefer.resolve(); + const error = new Error('The expression was aborted.'); + error.name = 'AbortError'; + this.abortedDefer.resolve(error); this.abortedDefer = null; } diff --git a/src/legacy/ui/public/courier/search_source/search_source.js b/src/legacy/ui/public/courier/search_source/search_source.js index 4a2c441012b50..92ca5bc4932c8 100644 --- a/src/legacy/ui/public/courier/search_source/search_source.js +++ b/src/legacy/ui/public/courier/search_source/search_source.js @@ -312,7 +312,12 @@ export function SearchSourceProvider(Promise, Private, config) { } fetchSoon.fetchSearchRequests([req]); - return req.getCompletePromise(); + return new Promise((resolve, reject) => { + req.getCompleteOrAbortedPromise().then(result => { + if (result instanceof Error) return reject(result); + resolve(result); + }); + }); } /** 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 452ea03c9b235..c0dd9b326d1ec 100644 --- a/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.ts +++ b/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.ts @@ -479,6 +479,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.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/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) { From 215be5dd7dcd43689f3bef8b4a8535beb44b376f Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Wed, 7 Aug 2019 17:16:59 -0700 Subject: [PATCH 16/23] Fix naming --- .../courier/fetch/request/search_request/search_request.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 e44d88720353e..3b6e88d439408 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 @@ -170,7 +170,7 @@ export function SearchRequestProvider(Promise) { this._markStopped(); this.defer = null; this.aborted = true; - const error = new Error('The expression was aborted.'); + const error = new Error('The request was aborted.'); error.name = 'AbortError'; this.abortedDefer.resolve(error); this.abortedDefer = null; From d63a824acae6ae9cf9bdae311929c5ef1f1a4395 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Thu, 8 Aug 2019 15:58:27 -0700 Subject: [PATCH 17/23] Update search request to reject on abort, and don't render errors in the UI --- .../kibana/public/discover/controllers/discover.js | 3 +++ .../courier/fetch/request/search_request/search_request.js | 3 ++- .../ui/public/courier/search_source/search_source.js | 7 +------ .../public/visualize/loader/embedded_visualize_handler.ts | 2 +- 4 files changed, 7 insertions(+), 8 deletions(-) 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 377fd72e9c771..67067debcb9c4 100644 --- a/src/legacy/core_plugins/kibana/public/discover/controllers/discover.js +++ b/src/legacy/core_plugins/kibana/public/discover/controllers/discover.js @@ -735,6 +735,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/ui/public/courier/fetch/request/search_request/search_request.js b/src/legacy/ui/public/courier/fetch/request/search_request/search_request.js index 3b6e88d439408..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,12 +168,13 @@ export function SearchRequestProvider(Promise) { abort() { this._markStopped(); - this.defer = null; this.aborted = true; 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/courier/search_source/search_source.js b/src/legacy/ui/public/courier/search_source/search_source.js index 92ca5bc4932c8..4a2c441012b50 100644 --- a/src/legacy/ui/public/courier/search_source/search_source.js +++ b/src/legacy/ui/public/courier/search_source/search_source.js @@ -312,12 +312,7 @@ export function SearchSourceProvider(Promise, Private, config) { } fetchSoon.fetchSearchRequests([req]); - return new Promise((resolve, reject) => { - req.getCompleteOrAbortedPromise().then(result => { - if (result instanceof Error) return reject(result); - resolve(result); - }); - }); + return req.getCompletePromise(); } /** 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 dd352fccbc61a..907f219d7091e 100644 --- a/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.ts +++ b/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.ts @@ -487,7 +487,7 @@ export class EmbeddedVisualizeHandler { */ private handleDataLoaderError = (error: any): void => { // If the data loader was aborted then no need to surface this error in the UI - if (error.name === 'AbortError') return; + 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) { From aeb5045875f4a949680a4d8c336d83e9b1818d98 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Fri, 9 Aug 2019 10:02:22 -0700 Subject: [PATCH 18/23] Update tests --- .../embedded_visualize_handler.test.mocks.ts | 3 --- .../loader/embedded_visualize_handler.test.ts | 27 ++++++++++++------- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.test.mocks.ts b/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.test.mocks.ts index 82407e1d4feee..6adea30c94fb4 100644 --- a/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.test.mocks.ts +++ b/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.test.mocks.ts @@ -53,13 +53,10 @@ export const mockDataLoaderFetch = jest.fn().mockReturnValue({ params: {}, }, }); -export const mockDataLoaderCancel = jest.fn(); - const MockDataLoader = class { public async fetch(data: any) { return await mockDataLoaderFetch(data); } - public cancel = mockDataLoaderCancel; }; jest.mock('./pipeline_data_loader', () => ({ 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 a67397bb4e2da..259678faca7d4 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 @@ -16,11 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import { - mockDataLoaderFetch, - mockDataLoaderCancel, - timefilter, -} from './embedded_visualize_handler.test.mocks'; +import { mockDataLoaderFetch, timefilter } from './embedded_visualize_handler.test.mocks'; // @ts-ignore import MockState from '../../../../../fixtures/mock_state'; @@ -157,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 }); }); }); @@ -202,9 +207,11 @@ describe('EmbeddedVisualizeHandler', () => { expect(spy).toHaveBeenCalledTimes(1); }); - it('should call cancel on data loader', () => { + it('should call abort on controller', () => { + handler.abortController = new AbortController(); + const spy = jest.spyOn(handler.abortController, 'abort'); handler.destroy(); - expect(mockDataLoaderCancel).toHaveBeenCalled(); + expect(spy).toHaveBeenCalled(); }); }); From 728a7b49ae76653ab8fdfda4f9a809c19c5f2594 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Tue, 27 Aug 2019 09:53:32 -0700 Subject: [PATCH 19/23] Add cancellation to input control --- .../input_control_vis/public/control/control.js | 4 ++++ .../public/control/list_control_factory.js | 17 ++++++++++++++++- .../public/control/range_control_factory.js | 14 ++++++++++++++ .../input_control_vis/public/vis_controller.js | 3 +-- 4 files changed, 35 insertions(+), 3 deletions(-) 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 = () => { From e068744883fb80a1fb9b711c9574cbdba803979e Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Tue, 27 Aug 2019 15:02:37 -0700 Subject: [PATCH 20/23] Cancel histogram fetches --- .../ui/public/agg_types/buckets/histogram.js | 14 ++++--- .../loader/embedded_visualize_handler.ts | 38 ++++++++++--------- .../loader/pipeline_helpers/build_pipeline.ts | 17 +++++++-- .../visualize/loader/visualize_data_loader.ts | 1 + 4 files changed, 45 insertions(+), 25 deletions(-) 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/visualize/loader/embedded_visualize_handler.ts b/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.ts index 567c8e6be5bcb..2d50831a21041 100644 --- a/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.ts +++ b/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.ts @@ -461,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) + ); }; /** 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/visualize_data_loader.ts b/src/legacy/ui/public/visualize/loader/visualize_data_loader.ts index 3352e1284591c..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 || []; From 72a8965c8087df464c878e4d5fd1d0d08ca83b25 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Tue, 27 Aug 2019 16:57:46 -0700 Subject: [PATCH 21/23] Update esaggs typings --- .../core_plugins/interpreter/public/functions/esaggs.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/legacy/core_plugins/interpreter/public/functions/esaggs.ts b/src/legacy/core_plugins/interpreter/public/functions/esaggs.ts index 2979476001eb4..4438256c91228 100644 --- a/src/legacy/core_plugins/interpreter/public/functions/esaggs.ts +++ b/src/legacy/core_plugins/interpreter/public/functions/esaggs.ts @@ -114,13 +114,13 @@ export const esaggs = (): ExpressionFunction { + columns: response.columns.map((column: any) => { const cleanedColumn: KibanaDatatableColumn = { id: column.id, name: column.name, From d5983e4ccfd71fb4d187498b4250504c90fd2919 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Tue, 27 Aug 2019 17:13:51 -0700 Subject: [PATCH 22/23] Add cancellation for search embeddables --- .../public/discover/embeddable/search_embeddable.ts | 8 ++++++++ 1 file changed, 8 insertions(+) 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', From 4949fb49a63d7e5187e2d96ca8f4c93fffc30cc3 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Wed, 28 Aug 2019 07:29:47 -0700 Subject: [PATCH 23/23] Abort post-flight requests --- src/legacy/ui/public/agg_types/buckets/terms.js | 5 ++++- src/legacy/ui/public/vis/request_handlers/courier.js | 3 ++- 2 files changed, 6 insertions(+), 2 deletions(-) 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/vis/request_handlers/courier.js b/src/legacy/ui/public/vis/request_handlers/courier.js index 5f4c5489ae46f..8ece5b11d6a42 100644 --- a/src/legacy/ui/public/vis/request_handlers/courier.js +++ b/src/legacy/ui/public/vis/request_handlers/courier.js @@ -139,7 +139,8 @@ const CourierRequestHandlerProvider = function () { aggs, agg, requestSearchSource, - inspectorAdapters + inspectorAdapters, + abortSignal, ); } }