diff --git a/src/platform/packages/shared/kbn-unified-histogram/components/chart/hooks/use_total_hits.ts b/src/platform/packages/shared/kbn-unified-histogram/components/chart/hooks/use_total_hits.ts index 564b9a98d51d6..3a9ded1e1bed6 100644 --- a/src/platform/packages/shared/kbn-unified-histogram/components/chart/hooks/use_total_hits.ts +++ b/src/platform/packages/shared/kbn-unified-histogram/components/chart/hooks/use_total_hits.ts @@ -61,8 +61,8 @@ export const useTotalHits = ({ return () => subscription.unsubscribe(); }, [fetch, fetch$]); - const onAbort = useCallback(() => { - abortController.current?.abort(); + const onAbort = useCallback((e: Event) => { + abortController.current?.abort((e.target as AbortSignal)?.reason); }, []); useEffect(() => { diff --git a/src/platform/plugins/shared/dashboard/public/dashboard_app/no_data/dashboard_app_no_data.tsx b/src/platform/plugins/shared/dashboard/public/dashboard_app/no_data/dashboard_app_no_data.tsx index d898e8d798e51..9d7b4f818f5d5 100644 --- a/src/platform/plugins/shared/dashboard/public/dashboard_app/no_data/dashboard_app_no_data.tsx +++ b/src/platform/plugins/shared/dashboard/public/dashboard_app/no_data/dashboard_app_no_data.tsx @@ -19,6 +19,7 @@ import { import { withSuspense } from '@kbn/shared-ux-utility'; import type { LensSerializedState } from '@kbn/lens-plugin/public'; import { getLensAttributesFromSuggestion } from '@kbn/visualization-utils'; +import { AbortReason } from '@kbn/kibana-utils-plugin/common'; import { coreServices, dataService, @@ -59,12 +60,12 @@ export const DashboardAppNoDataPage = ({ useEffect(() => { return () => { - abortController?.abort(); + abortController?.abort(AbortReason.CLEANUP); }; }, [abortController]); const onTryESQL = useCallback(async () => { - abortController?.abort(); + abortController?.abort(AbortReason.REPLACED); if (lensHelpersAsync.value) { const abc = new AbortController(); const { dataViews } = dataService; diff --git a/src/platform/plugins/shared/data/common/search/poll_search.test.ts b/src/platform/plugins/shared/data/common/search/poll_search.test.ts index 1254a3dc5f684..9a75f9ba8970e 100644 --- a/src/platform/plugins/shared/data/common/search/poll_search.test.ts +++ b/src/platform/plugins/shared/data/common/search/poll_search.test.ts @@ -8,7 +8,7 @@ */ import { pollSearch } from './poll_search'; -import { AbortError } from '@kbn/kibana-utils-plugin/common'; +import { AbortError, AbortReason } from '@kbn/kibana-utils-plugin/common'; describe('pollSearch', () => { function getMockedSearch$(resolveOnI = 1) { @@ -86,6 +86,29 @@ describe('pollSearch', () => { expect(cancelFn).toBeCalledTimes(1); }); + test('Does not throw or cancel if abort reason is CANCELED', async () => { + const searchFn = jest.fn().mockResolvedValue({ + isRunning: false, + isPartial: false, + rawResponse: {}, + }); + const cancelFn = jest.fn(); + + const abortController = new AbortController(); + setTimeout(() => abortController.abort(AbortReason.CANCELED), 100); + + // Poll should complete without throwing AbortError and without calling cancel + await expect( + pollSearch(searchFn, cancelFn, { abortSignal: abortController.signal }).toPromise() + ).resolves.toEqual({ + isRunning: false, + isPartial: false, + rawResponse: {}, + }); + + expect(cancelFn).not.toHaveBeenCalled(); + }); + test('Does not leak unresolved promises on cancel', async () => { const searchFn = getMockedSearch$(20); const cancelFn = jest.fn().mockRejectedValueOnce({ error: 'Oh no!' }); diff --git a/src/platform/plugins/shared/data/common/search/poll_search.ts b/src/platform/plugins/shared/data/common/search/poll_search.ts index d2129461a315d..c0c13e9d5b385 100644 --- a/src/platform/plugins/shared/data/common/search/poll_search.ts +++ b/src/platform/plugins/shared/data/common/search/poll_search.ts @@ -8,8 +8,19 @@ */ import type { Observable } from 'rxjs'; -import { from, timer, defer, fromEvent, EMPTY } from 'rxjs'; -import { expand, map, switchMap, takeUntil, takeWhile, tap } from 'rxjs'; +import { + defer, + EMPTY, + expand, + from, + fromEvent, + switchMap, + takeUntil, + takeWhile, + tap, + throwError, + timer, +} from 'rxjs'; import { AbortError } from '@kbn/kibana-utils-plugin/common'; import type { IKibanaSearchResponse } from '@kbn/search-types'; import type { IAsyncSearchOptions } from '..'; @@ -54,9 +65,7 @@ export const pollSearch = ( } const aborted$ = (abortSignal ? fromEvent(abortSignal, 'abort') : EMPTY).pipe( - map(() => { - throw new AbortError(); - }) + switchMap((e) => throwError(() => new AbortError((e.target as AbortSignal)?.reason))) ); return from(search()).pipe( diff --git a/src/platform/plugins/shared/data/public/search/search_interceptor/search_abort_controller.test.ts b/src/platform/plugins/shared/data/public/search/search_interceptor/search_abort_controller.test.ts index 99922c094fb5e..a6ec14b2785b2 100644 --- a/src/platform/plugins/shared/data/public/search/search_interceptor/search_abort_controller.test.ts +++ b/src/platform/plugins/shared/data/public/search/search_interceptor/search_abort_controller.test.ts @@ -7,6 +7,7 @@ * License v3.0 only", or the "Server Side Public License, v 1". */ +import { AbortReason } from '@kbn/kibana-utils-plugin/common'; import { SearchAbortController } from './search_abort_controller'; const timeTravel = (msToRun = 0) => { @@ -45,10 +46,26 @@ describe('search abort controller', () => { const controller2 = new AbortController(); sac.addAbortSignal(controller2.signal); expect(sac.getSignal().aborted).toBe(false); - controller.abort(); + controller.abort(AbortReason.CANCELED); expect(sac.getSignal().aborted).toBe(false); - controller2.abort(); - expect(sac.getSignal().aborted).toBe(true); + controller2.abort(AbortReason.CANCELED); + const signal = sac.getSignal(); + expect(signal.aborted).toBe(true); + expect(signal.reason).toBe(AbortReason.CANCELED); + }); + + test('when the abort reason is CANCELED', () => { + const sac = new SearchAbortController(); + sac.abort(AbortReason.CANCELED); + expect(sac.isCanceled()).toBe(true); + expect(sac.isTimeout()).toBe(false); + }); + + test('when the abort reason is TIMEOUT', () => { + const sac = new SearchAbortController(); + sac.abort(AbortReason.TIMEOUT); + expect(sac.isTimeout()).toBe(true); + expect(sac.isCanceled()).toBe(false); }); test('aborts explicitly even if all inputs are not aborted', () => { diff --git a/src/platform/plugins/shared/data/public/search/search_interceptor/search_abort_controller.ts b/src/platform/plugins/shared/data/public/search/search_interceptor/search_abort_controller.ts index 07e4980ac011a..4692157dad8d1 100644 --- a/src/platform/plugins/shared/data/public/search/search_interceptor/search_abort_controller.ts +++ b/src/platform/plugins/shared/data/public/search/search_interceptor/search_abort_controller.ts @@ -9,23 +9,18 @@ import type { Subscription } from 'rxjs'; import { timer } from 'rxjs'; - -export enum AbortReason { - Timeout = 'timeout', -} +import { AbortReason } from '@kbn/kibana-utils-plugin/common'; export class SearchAbortController { private inputAbortSignals: AbortSignal[] = new Array(); private abortController: AbortController = new AbortController(); private timeoutSub?: Subscription; private destroyed = false; - private reason?: AbortReason; constructor(timeout?: number) { if (timeout) { this.timeoutSub = timer(timeout).subscribe(() => { - this.reason = AbortReason.Timeout; - this.abortController.abort(); + this.abortController.abort(AbortReason.TIMEOUT); this.timeoutSub!.unsubscribe(); }); } @@ -34,7 +29,7 @@ export class SearchAbortController { private abortHandler = () => { const allAborted = this.inputAbortSignals.every((signal) => signal.aborted); if (allAborted) { - this.abortController.abort(); + this.abortController.abort(this.inputAbortSignals[0].reason); this.cleanup(); } }; @@ -67,12 +62,16 @@ export class SearchAbortController { return this.abortController.signal; } - public abort() { + public abort(reason?: AbortReason) { this.cleanup(); - this.abortController.abort(); + this.abortController.abort(reason); } public isTimeout() { - return this.reason === AbortReason.Timeout; + return this.abortController.signal.reason === AbortReason.TIMEOUT; + } + + public isCanceled() { + return this.abortController.signal.reason === AbortReason.CANCELED; } } diff --git a/src/platform/plugins/shared/data/public/search/search_interceptor/search_interceptor.test.ts b/src/platform/plugins/shared/data/public/search/search_interceptor/search_interceptor.test.ts index a2859710a1fbc..9f75874068a1d 100644 --- a/src/platform/plugins/shared/data/public/search/search_interceptor/search_interceptor.test.ts +++ b/src/platform/plugins/shared/data/public/search/search_interceptor/search_interceptor.test.ts @@ -17,16 +17,15 @@ import { AbortError } from '@kbn/kibana-utils-plugin/public'; import { EsError, type IEsError } from '@kbn/search-errors'; import type { ISessionService } from '..'; import { SearchSessionState } from '..'; - import * as searchPhaseException from '../../../common/search/test_data/search_phase_execution_exception.json'; import * as resourceNotFoundException from '../../../common/search/test_data/resource_not_found_exception.json'; import { BehaviorSubject } from 'rxjs'; import { dataPluginMock } from '../../mocks'; +import { AbortReason } from '@kbn/kibana-utils-plugin/common'; import { ESQL_ASYNC_SEARCH_STRATEGY, UI_SETTINGS } from '../../../common'; import type { SearchServiceStartDependencies } from '../search_service'; import type { Start as InspectorStart } from '@kbn/inspector-plugin/public'; import { SearchTimeoutError, TimeoutErrorMode } from './timeout_error'; - import { SearchSessionIncompleteWarning } from './search_session_incomplete_warning'; import { getMockSearchConfig } from '../../../config.mock'; @@ -145,9 +144,9 @@ describe('SearchInterceptor', () => { } }); - next.mockClear(); - error.mockClear(); - complete.mockClear(); + next.mockReset(); + error.mockReset(); + complete.mockReset(); jest.clearAllTimers(); jest.clearAllMocks(); @@ -1050,7 +1049,6 @@ describe('SearchInterceptor', () => { afterEach(() => { const sessionServiceMock = sessionService as jest.Mocked; sessionServiceMock.getSearchOptions.mockReset(); - mockCoreSetup.http.post.mockReset(); }); test('gets session search options from session service', async () => { @@ -1359,7 +1357,7 @@ describe('SearchInterceptor', () => { const abort = sessionService.trackSearch.mock.calls[0][0].abort; expect(abort).toBeInstanceOf(Function); - abort(); + abort(AbortReason.REPLACED); await timeTravel(10); @@ -2129,5 +2127,111 @@ describe('SearchInterceptor', () => { response.subscribe({ error }); }); }); + + describe('partial results', () => { + beforeEach(() => { + mockCoreSetup.http.post.mockResolvedValue( + getMockSearchResponse({ + id: '1', + isPartial: true, + isRunning: true, + rawResponse: {}, + }) + ); + }); + + test('should request partial results and throw error if timed out', async () => { + const abortController = new AbortController(); + setTimeout(() => { + abortController.abort(AbortReason.TIMEOUT); + }, 50); + + const response = searchInterceptor.search( + {}, + { abortSignal: abortController.signal, pollInterval: 100 } + ); + response.subscribe({ next, error }); + + await timeTravel(); // Run first request/response + + expect(next).toHaveBeenCalled(); + expect(error).not.toHaveBeenCalled(); + + await timeTravel(50); // Run until abort + + expect(mockCoreSetup.http.post).toHaveBeenCalledTimes(2); + expect(mockCoreSetup.http.post.mock.calls[1]).toMatchInlineSnapshot(` + Array [ + "/internal/search/ese/1", + Object { + "asResponse": true, + "body": "{\\"id\\":\\"1\\",\\"params\\":{},\\"retrieveResults\\":true,\\"stream\\":true}", + "context": undefined, + "signal": AbortSignal {}, + "version": "1", + }, + ] + `); + expect(error).toHaveBeenCalled(); + }); + + test('should request partial results and not throw error if canceled', async () => { + const abortController = new AbortController(); + setTimeout(() => { + abortController.abort(AbortReason.CANCELED); + }, 50); + + const response = searchInterceptor.search( + {}, + { abortSignal: abortController.signal, pollInterval: 100 } + ); + response.subscribe({ next, error }); + + await timeTravel(); // Run first request/response + + expect(next).toHaveBeenCalled(); + expect(error).not.toHaveBeenCalled(); + + await timeTravel(50); // Run until abort + + expect(mockCoreSetup.http.post).toHaveBeenCalledTimes(2); + expect(mockCoreSetup.http.post.mock.calls[1]).toMatchInlineSnapshot(` + Array [ + "/internal/search/ese/1", + Object { + "asResponse": true, + "body": "{\\"id\\":\\"1\\",\\"params\\":{},\\"retrieveResults\\":true,\\"stream\\":true}", + "context": undefined, + "signal": AbortSignal {}, + "version": "1", + }, + ] + `); + expect(error).not.toHaveBeenCalled(); + }); + + test('should not request partial results and throw error if canceled for a reason other than CANCELED/TIMEOUT', async () => { + const abortController = new AbortController(); + setTimeout(() => { + abortController.abort(AbortReason.CLEANUP); + }, 50); + + const response = searchInterceptor.search( + {}, + { abortSignal: abortController.signal, pollInterval: 100 } + ); + response.subscribe({ next, error }); + + await timeTravel(); // Run first request/response + + expect(next).toHaveBeenCalled(); + expect(error).not.toHaveBeenCalled(); + + await timeTravel(50); // Run until abort + + expect(mockCoreSetup.http.post).toHaveBeenCalledTimes(1); + expect(error).toHaveBeenCalled(); + }); + }); }); }); diff --git a/src/platform/plugins/shared/data/public/search/search_interceptor/search_interceptor.ts b/src/platform/plugins/shared/data/public/search/search_interceptor/search_interceptor.ts index 0136c13409a20..c2a0fd70372af 100644 --- a/src/platform/plugins/shared/data/public/search/search_interceptor/search_interceptor.ts +++ b/src/platform/plugins/shared/data/public/search/search_interceptor/search_interceptor.ts @@ -10,18 +10,23 @@ import { v4 as uuidv4 } from 'uuid'; import { memoize, once } from 'lodash'; import type { Observable, Subscription } from 'rxjs'; -import { BehaviorSubject, EMPTY, from, fromEvent, of, throwError } from 'rxjs'; import { + BehaviorSubject, catchError, + EMPTY, filter, finalize, + from, + fromEvent, map, + of, shareReplay, skip, switchMap, take, takeUntil, tap, + throwError, } from 'rxjs'; import type { estypes } from '@elastic/elasticsearch'; import type { @@ -51,13 +56,14 @@ import { toMountPoint } from '@kbn/react-kibana-mount'; import type { KibanaServerError } from '@kbn/kibana-utils-plugin/public'; import { AbortError } from '@kbn/kibana-utils-plugin/public'; import type { - SanitizedConnectionRequestParams, IKibanaSearchRequest, + IKibanaSearchResponse, + ISearchOptions, ISearchOptionsSerializable, + SanitizedConnectionRequestParams, } from '@kbn/search-types'; import { createEsError, isEsError, renderSearchError } from '@kbn/search-errors'; -import type { IKibanaSearchResponse, ISearchOptions } from '@kbn/search-types'; -import { defaultFreeze } from '@kbn/kibana-utils-plugin/common'; +import { AbortReason, defaultFreeze } from '@kbn/kibana-utils-plugin/common'; import { EVENT_TYPE_DATA_SEARCH_TIMEOUT, EVENT_PROPERTY_SEARCH_TIMEOUT_MS, @@ -318,7 +324,7 @@ export class SearchInterceptor { const searchTracker = this.deps.session.isCurrentSession(sessionId) ? this.deps.session.trackSearch({ - abort: () => searchAbortController.abort(), + abort: (reason?: AbortReason) => searchAbortController.abort(reason), poll: async (abortSignal) => { if (id) { await search({ abortSignal }); @@ -356,8 +362,14 @@ export class SearchInterceptor { ); const cancel = async () => { - // If the request times out, we handle cancellation after we make the last call to retrieve the results - if (!id || isSavedToBackground || searchAbortController.isTimeout()) return; + // If the request times out/is canceled, we handle cancellation after we make the last call to retrieve the results + if ( + !id || + isSavedToBackground || + searchAbortController.isTimeout() || + searchAbortController.isCanceled() + ) + return; try { await sendCancelRequest(); } catch (e) { @@ -399,23 +411,31 @@ export class SearchInterceptor { : response; }), catchError((e: Error) => { - // If we aborted (search:timeout advanced setting) and there was a partial response, return it instead of just erroring out - if (searchAbortController.isTimeout()) { - this.startRenderServices.analytics.reportEvent(EVENT_TYPE_DATA_SEARCH_TIMEOUT, { - [EVENT_PROPERTY_SEARCH_TIMEOUT_MS]: this.searchTimeout, - [EVENT_PROPERTY_EXECUTION_CONTEXT]: options.executionContext, - }); + // If we aborted (search:timeout advanced setting) or the user canceled and there was a partial response, return it instead of just erroring out + if (searchAbortController.isTimeout() || searchAbortController.isCanceled()) { + if (searchAbortController.isTimeout()) { + this.startRenderServices.analytics.reportEvent(EVENT_TYPE_DATA_SEARCH_TIMEOUT, { + [EVENT_PROPERTY_SEARCH_TIMEOUT_MS]: this.searchTimeout, + [EVENT_PROPERTY_EXECUTION_CONTEXT]: options.executionContext, + }); + } return from( - this.runSearch({ id, ...request }, { ...options, retrieveResults: true }) + this.runSearch( + { id, ...request }, + { ...options, abortSignal: new AbortController().signal, retrieveResults: true } + ) ).pipe( map((response) => options.strategy === ENHANCED_ES_SEARCH_STRATEGY ? toPartialResponseAfterTimeout(response) : response ), - tap(async () => { + tap(async (response) => { + id = id ?? response.id; await sendCancelRequest(); - this.handleSearchError(e, request?.params?.body ?? {}, options, true); + if (searchAbortController.isTimeout()) { + this.handleSearchError(e, request?.params?.body ?? {}, options, true); + } }) ); } else { @@ -606,9 +626,11 @@ export class SearchInterceptor { // Abort the replay if the abortSignal is aborted. // The underlaying search will not abort unless searchAbortController fires. const aborted$ = (abortSignal ? fromEvent(abortSignal, 'abort') : EMPTY).pipe( - map(() => { - throw new AbortError(); - }) + switchMap((e) => + (e.target as AbortSignal)?.reason === AbortReason.CANCELED + ? EMPTY + : throwError(new AbortError()) + ) ); return response$.pipe( diff --git a/src/platform/plugins/shared/data/public/search/session/session_service.ts b/src/platform/plugins/shared/data/public/search/session/session_service.ts index 86c4730d559a6..0252ade399621 100644 --- a/src/platform/plugins/shared/data/public/search/session/session_service.ts +++ b/src/platform/plugins/shared/data/public/search/session/session_service.ts @@ -9,19 +9,26 @@ import type { PublicContract, SerializableRecord } from '@kbn/utility-types'; import { + BehaviorSubject, + combineLatest, distinctUntilChanged, + EMPTY, filter, + from, map, mapTo, + merge, mergeMap, + type Observable, + of, repeat, startWith, + Subscription, switchMap, takeUntil, tap, + timer, } from 'rxjs'; -import type { Observable } from 'rxjs'; -import { BehaviorSubject, combineLatest, EMPTY, from, merge, of, Subscription, timer } from 'rxjs'; import type { PluginInitializerContext, StartServicesAccessor, @@ -32,6 +39,7 @@ import moment from 'moment'; import type { IKibanaSearchResponse, ISearchOptions } from '@kbn/search-types'; import { LRUCache } from 'lru-cache'; import type { Logger } from '@kbn/logging'; +import { AbortReason } from '@kbn/kibana-utils-plugin/common'; import type { SearchUsageCollector } from '../..'; import type { ConfigSchema } from '../../../server/config'; import type { SessionMeta, SessionStateContainer } from './search_session_state'; @@ -68,7 +76,7 @@ interface TrackSearchDescriptor { /** * Cancel the search */ - abort: () => void; + abort: (reason: AbortReason) => void; /** * Used for polling after running in background (to ensure the search makes it into the background search saved @@ -585,7 +593,7 @@ export class SessionService { state.trackedSearches .filter((s) => s.state === TrackedSearchState.InProgress) .forEach((s) => { - s.searchDescriptor.abort(); + s.searchDescriptor.abort(AbortReason.CANCELED); }); this.state.transitions.cancel(); if (isStoredSession) { diff --git a/src/platform/plugins/shared/discover/public/application/main/components/layout/discover_layout.tsx b/src/platform/plugins/shared/discover/public/application/main/components/layout/discover_layout.tsx index e637e303e21a1..ad3bc8fb19056 100644 --- a/src/platform/plugins/shared/discover/public/application/main/components/layout/discover_layout.tsx +++ b/src/platform/plugins/shared/discover/public/application/main/components/layout/discover_layout.tsx @@ -56,7 +56,6 @@ import { addLog } from '../../../../utils/add_log'; import { DiscoverResizableLayout } from './discover_resizable_layout'; import type { PanelsToggleProps } from '../../../../components/panels_toggle'; import { PanelsToggle } from '../../../../components/panels_toggle'; -import { sendErrorMsg } from '../../hooks/use_saved_search_messages'; import { useIsEsqlMode } from '../../hooks/use_is_esql_mode'; import { internalStateActions, @@ -394,8 +393,6 @@ export function DiscoverLayout({ stateContainer }: DiscoverLayoutProps) { const onCancelClick = useCallback(() => { stateContainer.dataState.cancel(); - sendErrorMsg(stateContainer.dataState.data$.documents$); - sendErrorMsg(stateContainer.dataState.data$.main$); }, [stateContainer.dataState]); const layoutUiState = useCurrentTabSelector((state) => state.uiState.layout); diff --git a/src/platform/plugins/shared/discover/public/application/main/components/no_results/no_results_suggestions/use_fetch_occurances_range.ts b/src/platform/plugins/shared/discover/public/application/main/components/no_results/no_results_suggestions/use_fetch_occurances_range.ts index beb8e49877739..83ee1a802efbc 100644 --- a/src/platform/plugins/shared/discover/public/application/main/components/no_results/no_results_suggestions/use_fetch_occurances_range.ts +++ b/src/platform/plugins/shared/discover/public/application/main/components/no_results/no_results_suggestions/use_fetch_occurances_range.ts @@ -10,12 +10,13 @@ import { useCallback, useEffect, useRef } from 'react'; import { lastValueFrom } from 'rxjs'; import type { DataView } from '@kbn/data-plugin/common'; +import { getEsQueryConfig } from '@kbn/data-plugin/common'; import type { AggregateQuery, Filter, Query } from '@kbn/es-query'; +import { buildEsQuery } from '@kbn/es-query'; import type { DataPublicPluginStart } from '@kbn/data-plugin/public'; import type { IUiSettingsClient } from '@kbn/core-ui-settings-browser'; import type { AggregationsSingleMetricAggregateBase } from '@elastic/elasticsearch/lib/api/types'; -import { buildEsQuery } from '@kbn/es-query'; -import { getEsQueryConfig } from '@kbn/data-plugin/common'; +import { AbortReason } from '@kbn/kibana-utils-plugin/common'; export interface Params { dataView?: DataView; @@ -61,7 +62,7 @@ export const useFetchOccurrencesRange = (params: Params): Result => { let occurrencesRangeResult = { status: TimeRangeExtendingStatus.failed }; if (dataView?.isTimeBased() && query && mountedRef.current) { - abortControllerRef.current?.abort(); + abortControllerRef.current?.abort(AbortReason.REPLACED); abortControllerRef.current = new AbortController(); try { @@ -93,7 +94,7 @@ export const useFetchOccurrencesRange = (params: Params): Result => { useEffect(() => { return () => { mountedRef.current = false; - abortControllerRef.current?.abort(); + abortControllerRef.current?.abort(AbortReason.CLEANUP); }; }, [abortControllerRef, mountedRef]); diff --git a/src/platform/plugins/shared/discover/public/application/main/data_fetching/fetch_esql.ts b/src/platform/plugins/shared/discover/public/application/main/data_fetching/fetch_esql.ts index 4282a51cc62eb..7f72a168ceb22 100644 --- a/src/platform/plugins/shared/discover/public/application/main/data_fetching/fetch_esql.ts +++ b/src/platform/plugins/shared/discover/public/application/main/data_fetching/fetch_esql.ts @@ -79,7 +79,9 @@ export function fetchEsql({ }, searchSessionId, }); - abortSignal?.addEventListener('abort', contract.cancel); + abortSignal?.addEventListener('abort', (e) => { + contract.cancel((e.target as AbortSignal)?.reason); + }); const execution = contract.getData(); let finalData: DataTableRecord[] = []; let esqlQueryColumns: Datatable['columns'] | undefined; diff --git a/src/platform/plugins/shared/discover/public/application/main/state_management/discover_data_state_container.ts b/src/platform/plugins/shared/discover/public/application/main/state_management/discover_data_state_container.ts index aec021dbb9234..6fca5296102b1 100644 --- a/src/platform/plugins/shared/discover/public/application/main/state_management/discover_data_state_container.ts +++ b/src/platform/plugins/shared/discover/public/application/main/state_management/discover_data_state_container.ts @@ -29,6 +29,7 @@ import type { SearchResponseWarning } from '@kbn/search-response-warnings'; import type { DataTableRecord } from '@kbn/discover-utils/types'; import { DEFAULT_COLUMNS_SETTING, SEARCH_ON_PAGE_LOAD_SETTING } from '@kbn/discover-utils'; import { getTimeDifferenceInSeconds } from '@kbn/timerange'; +import { AbortReason } from '@kbn/kibana-utils-plugin/common'; import { getEsqlDataView } from './utils/get_esql_data_view'; import type { DiscoverServices } from '../../../build_services'; import type { DiscoverSearchSessionManager } from './discover_search_session'; @@ -285,8 +286,8 @@ export function getDataStateContainer({ getCurrentTab, }; - abortController?.abort(); - abortControllerFetchMore?.abort(); + abortController?.abort(AbortReason.REPLACED); + abortControllerFetchMore?.abort(AbortReason.REPLACED); if (options.fetchMore) { abortControllerFetchMore = new AbortController(); @@ -460,8 +461,8 @@ export function getDataStateContainer({ }; const cancel = () => { - abortController?.abort(); - abortControllerFetchMore?.abort(); + abortController?.abort(AbortReason.CANCELED); + abortControllerFetchMore?.abort(AbortReason.CANCELED); }; const getAbortController = () => { diff --git a/src/platform/plugins/shared/discover/public/context_awareness/profiles_manager/profiles_manager.ts b/src/platform/plugins/shared/discover/public/context_awareness/profiles_manager/profiles_manager.ts index 4d81db191eb4f..da1eb7bb80dc7 100644 --- a/src/platform/plugins/shared/discover/public/context_awareness/profiles_manager/profiles_manager.ts +++ b/src/platform/plugins/shared/discover/public/context_awareness/profiles_manager/profiles_manager.ts @@ -9,6 +9,7 @@ import { isEqual } from 'lodash'; import { BehaviorSubject, skip } from 'rxjs'; +import { AbortReason } from '@kbn/kibana-utils-plugin/common'; import type { RootProfileService, DataSourceProfileService, @@ -78,7 +79,7 @@ export class ProfilesManager { } const abortController = new AbortController(); - this.rootProfileAbortController?.abort(); + this.rootProfileAbortController?.abort(AbortReason.REPLACED); this.rootProfileAbortController = abortController; let context = this.rootProfileService.defaultContext; diff --git a/src/platform/plugins/shared/discover/public/context_awareness/profiles_manager/scoped_profiles_manager.ts b/src/platform/plugins/shared/discover/public/context_awareness/profiles_manager/scoped_profiles_manager.ts index 8f840ee00e046..f3cc830407a1a 100644 --- a/src/platform/plugins/shared/discover/public/context_awareness/profiles_manager/scoped_profiles_manager.ts +++ b/src/platform/plugins/shared/discover/public/context_awareness/profiles_manager/scoped_profiles_manager.ts @@ -11,6 +11,7 @@ import { BehaviorSubject, combineLatest, map, skip } from 'rxjs'; import type { DataTableRecord } from '@kbn/discover-utils'; import { isEqual } from 'lodash'; import { isOfAggregateQueryType } from '@kbn/es-query'; +import { AbortReason } from '@kbn/kibana-utils-plugin/common'; import type { ContextWithProfileId } from '../profile_service'; import type { DataSourceContext, @@ -89,7 +90,7 @@ export class ScopedProfilesManager { } const abortController = new AbortController(); - this.dataSourceProfileAbortController?.abort(); + this.dataSourceProfileAbortController?.abort(AbortReason.REPLACED); this.dataSourceProfileAbortController = abortController; let context = this.dataSourceProfileService.defaultContext; diff --git a/src/platform/plugins/shared/discover/public/embeddable/initialize_fetch.ts b/src/platform/plugins/shared/discover/public/embeddable/initialize_fetch.ts index 74ce4d8189077..9e6d0a02382bc 100644 --- a/src/platform/plugins/shared/discover/public/embeddable/initialize_fetch.ts +++ b/src/platform/plugins/shared/discover/public/embeddable/initialize_fetch.ts @@ -36,7 +36,7 @@ import type { SavedSearch } from '@kbn/saved-search-plugin/public'; import type { SearchResponseWarning } from '@kbn/search-response-warnings'; import type { SearchResponseIncompleteWarning } from '@kbn/search-response-warnings/src/types'; import { getTextBasedColumnsMeta } from '@kbn/unified-data-table'; - +import { AbortReason } from '@kbn/kibana-utils-plugin/common'; import { fetchEsql } from '../application/main/data_fetching/fetch_esql'; import type { DiscoverServices } from '../build_services'; import { getAllowedSampleSize } from '../utils/get_allowed_sample_size'; @@ -173,7 +173,7 @@ export function initializeFetch({ tap(() => { // abort any in-progress requests if (abortController) { - abortController.abort(); + abortController.abort(AbortReason.REPLACED); abortController = undefined; } }), diff --git a/src/platform/plugins/shared/expressions/common/execution/execution.test.ts b/src/platform/plugins/shared/expressions/common/execution/execution.test.ts index e2c7d3f22a494..6bf71b2d37032 100644 --- a/src/platform/plugins/shared/expressions/common/execution/execution.test.ts +++ b/src/platform/plugins/shared/expressions/common/execution/execution.test.ts @@ -7,8 +7,7 @@ * License v3.0 only", or the "Server Side Public License, v 1". */ -import { lastValueFrom, of } from 'rxjs'; -import { scan } from 'rxjs'; +import { lastValueFrom, of, scan } from 'rxjs'; import { TestScheduler } from 'rxjs/testing'; import { Execution } from './execution'; import type { ExpressionAstExpression } from '../ast'; @@ -17,6 +16,7 @@ import { createUnitTestExecutor } from '../test_helpers'; import type { ExpressionFunctionDefinition } from '..'; import { ExecutionContract } from './execution_contract'; import type { ExpressionValueBoxed } from '../expression_types'; +import { AbortReason } from '@kbn/kibana-utils-plugin/common'; beforeAll(() => { if (typeof performance === 'undefined') { @@ -365,6 +365,22 @@ describe('Execution', () => { expect(result).toHaveProperty('result.aborted', false); }); + + test('does not throw error when aborted with AbortReason.CANCELED', async () => { + const execution = createExecution('add val=1'); + execution.start(1); + + // Simulate user cancellation + execution.cancel(AbortReason.CANCELED); + + const { result } = await lastValueFrom(execution.result); + + // Should return an abort error value, not throw + expect(result).toEqual({ + type: 'num', + value: 2, + }); + }); }); describe('expression execution', () => { diff --git a/src/platform/plugins/shared/expressions/common/execution/execution.ts b/src/platform/plugins/shared/expressions/common/execution/execution.ts index 67bcc37986ab5..b2e1034c2992b 100644 --- a/src/platform/plugins/shared/expressions/common/execution/execution.ts +++ b/src/platform/plugins/shared/expressions/common/execution/execution.ts @@ -14,21 +14,28 @@ import type { ObservableLike, UnwrapObservable } from '@kbn/utility-types'; import { keys, last as lastOf, mapValues, reduce, zipObject } from 'lodash'; import type { Subscription } from 'rxjs'; import { + catchError, combineLatest, defer, + finalize, from, identity, isObservable, last, + map, + Observable, of, + pluck, + ReplaySubject, + shareReplay, + switchMap, takeWhile, + tap, throwError, timer, - Observable, - ReplaySubject, } from 'rxjs'; -import { catchError, finalize, map, pluck, shareReplay, switchMap, tap } from 'rxjs'; -import { now, AbortError, calculateObjectHash } from '@kbn/kibana-utils-plugin/common'; +import { AbortReason } from '@kbn/kibana-utils-plugin/common'; +import { AbortError, calculateObjectHash, now } from '@kbn/kibana-utils-plugin/common'; import type { Adapters } from '@kbn/inspector-plugin/common'; import type { Executor } from '../executor'; import type { ExecutionContainer } from './container'; @@ -42,8 +49,8 @@ import type { ExpressionAstFunction, ExpressionAstNode, } from '../ast'; -import { parse, formatExpression, parseExpression } from '../ast'; -import type { ExecutionContext, DefaultInspectorAdapters } from './types'; +import { formatExpression, parse, parseExpression } from '../ast'; +import type { DefaultInspectorAdapters, ExecutionContext } from './types'; import type { Datatable } from '../expression_types'; import { getType } from '../expression_types'; import type { ExpressionFunction, ExpressionFunctionParameter } from '../expression_functions'; @@ -171,8 +178,11 @@ function throttle(timeout: number) { function takeUntilAborted(signal: AbortSignal) { return (source: Observable) => new Observable((subscriber) => { - const throwAbortError = () => { - subscriber.error(new AbortError()); + const throwAbortError = (e?: Event) => { + // If the execution was aborted due to end user cancellation, we still want to let + // the execution complete and handle the partial results + if ((e?.target as AbortSignal)?.reason !== AbortReason.CANCELED) + subscriber.error(new AbortError()); }; subscriber.add(source.subscribe(subscriber)); @@ -318,7 +328,9 @@ export class Execution< ), catchError((error) => { if (this.abortController.signal.aborted) { - this.childExecutions.forEach((childExecution) => childExecution.cancel()); + this.childExecutions.forEach((childExecution) => + childExecution.cancel(this.abortController.signal.reason) + ); return of({ result: createAbortErrorValue(), partial: false }); } @@ -339,8 +351,8 @@ export class Execution< /** * Stop execution of expression. */ - cancel() { - this.abortController.abort(); + cancel(reason?: AbortReason) { + this.abortController.abort(reason); } /** diff --git a/src/platform/plugins/shared/expressions/common/execution/execution_contract.ts b/src/platform/plugins/shared/expressions/common/execution/execution_contract.ts index cf378dbc22ef9..4f66ff3f212e7 100644 --- a/src/platform/plugins/shared/expressions/common/execution/execution_contract.ts +++ b/src/platform/plugins/shared/expressions/common/execution/execution_contract.ts @@ -11,6 +11,7 @@ import type { Observable } from 'rxjs'; import { of } from 'rxjs'; import { catchError } from 'rxjs'; import type { Adapters } from '@kbn/inspector-plugin/common/adapters'; +import type { AbortReason } from '@kbn/kibana-utils-plugin/common'; import type { Execution, ExecutionResult } from './execution'; import type { ExpressionValueError } from '../expression_types/specs'; import type { ExpressionAstExpression } from '../ast'; @@ -41,8 +42,8 @@ export class ExecutionContract< * (available in execution context) to aborted state, letting expression * functions to stop their execution. */ - cancel = () => { - this.execution.cancel(); + cancel = (reason?: AbortReason) => { + this.execution.cancel(reason); }; /** diff --git a/src/platform/plugins/shared/expressions/public/loader.ts b/src/platform/plugins/shared/expressions/public/loader.ts index 23bef6ed37cf2..87e82811480e1 100644 --- a/src/platform/plugins/shared/expressions/public/loader.ts +++ b/src/platform/plugins/shared/expressions/public/loader.ts @@ -8,11 +8,11 @@ */ import type { Observable, Subscription } from 'rxjs'; -import { BehaviorSubject, Subject } from 'rxjs'; -import { delay, filter, map, shareReplay } from 'rxjs'; +import { BehaviorSubject, delay, filter, map, shareReplay, Subject } from 'rxjs'; import { defaults } from 'lodash'; import type { SerializableRecord, UnwrapObservable } from '@kbn/utility-types'; import type { Adapters } from '@kbn/inspector-plugin/public'; +import { AbortReason } from '@kbn/kibana-utils-plugin/common'; import type { IExpressionLoaderParams } from './types'; import type { ExpressionAstExpression } from '../common'; import type { ExecutionContract } from '../common/execution/execution_contract'; @@ -97,12 +97,12 @@ export class ExpressionLoader { this.dataSubject.complete(); this.loadingSubject.complete(); this.renderHandler.destroy(); - this.cancel(); + this.cancel(AbortReason.CLEANUP); this.subscription?.unsubscribe(); } - cancel() { - this.execution?.cancel(); + cancel(reason?: AbortReason) { + this.execution?.cancel(reason); } getExpression(): string | undefined { @@ -138,7 +138,7 @@ export class ExpressionLoader { ) => { this.subscription?.unsubscribe(); if (this.execution && this.execution.isPending) { - this.execution.cancel(); + this.execution.cancel(AbortReason.REPLACED); } this.setParams(params); this.execution = getExpressionsService().execute(expression, params.context, { diff --git a/src/platform/plugins/shared/expressions/public/react_expression_renderer/use_expression_renderer.ts b/src/platform/plugins/shared/expressions/public/react_expression_renderer/use_expression_renderer.ts index 2f5458daea45d..e35641c98e79a 100644 --- a/src/platform/plugins/shared/expressions/public/react_expression_renderer/use_expression_renderer.ts +++ b/src/platform/plugins/shared/expressions/public/react_expression_renderer/use_expression_renderer.ts @@ -84,7 +84,7 @@ export function useExpressionRenderer( useEffect(() => { if (abortController?.signal) abortController.signal.onabort = () => { - expressionLoaderRef.current?.cancel(); + expressionLoaderRef.current?.cancel(abortController.signal.reason); }; }, [abortController]); diff --git a/src/platform/plugins/shared/kibana_utils/common/abort_utils.ts b/src/platform/plugins/shared/kibana_utils/common/abort_utils.ts index 8b155002a335c..074d282ac3cbe 100644 --- a/src/platform/plugins/shared/kibana_utils/common/abort_utils.ts +++ b/src/platform/plugins/shared/kibana_utils/common/abort_utils.ts @@ -7,6 +7,28 @@ * License v3.0 only", or the "Server Side Public License, v 1". */ +export enum AbortReason { + /** + * The request was aborted due to reaching the `search:timeout` advanced setting. + */ + TIMEOUT = 'timeout', + + /** + * The request was aborted because the data was replaced by a new request (refreshed/re-fetched). + */ + REPLACED = 'replaced', + + /** + * The request was aborted because the component unmounted or the execution context was destroyed. + */ + CLEANUP = 'cleanup', + + /** + * The request was aborted because the user explicitly canceled it. + */ + CANCELED = 'canceled', +} + /** * Class used to signify that something was aborted. Useful for applications to conditionally handle * this type of error differently than other errors. diff --git a/src/platform/plugins/shared/kibana_utils/common/index.ts b/src/platform/plugins/shared/kibana_utils/common/index.ts index 60bac7f100021..74e4dd27d2b70 100644 --- a/src/platform/plugins/shared/kibana_utils/common/index.ts +++ b/src/platform/plugins/shared/kibana_utils/common/index.ts @@ -50,7 +50,7 @@ export { InvalidJSONProperty, DuplicateField, } from './errors'; -export { AbortError, abortSignalToPromise } from './abort_utils'; +export { AbortError, AbortReason, abortSignalToPromise } from './abort_utils'; export type { Get, Set } from './create_getter_setter'; export { createGetterSetter } from './create_getter_setter'; export { distinctUntilChangedWithInitialValue } from './distinct_until_changed_with_initial_value'; diff --git a/src/platform/test/functional/apps/discover/ccs_compatibility/_cancel_results.ts b/src/platform/test/functional/apps/discover/ccs_compatibility/_cancel_results.ts new file mode 100644 index 0000000000000..16c9adbf8a823 --- /dev/null +++ b/src/platform/test/functional/apps/discover/ccs_compatibility/_cancel_results.ts @@ -0,0 +1,151 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +import expect from '@kbn/expect'; +import type { FtrProviderContext } from '../ftr_provider_context'; + +export default function ({ getService, getPageObjects }: FtrProviderContext) { + const filterBar = getService('filterBar'); + const kibanaServer = getService('kibanaServer'); + const retry = getService('retry'); + const testSubjects = getService('testSubjects'); + const toasts = getService('toasts'); + const { common, discover, header, timePicker } = getPageObjects([ + 'common', + 'discover', + 'header', + 'timePicker', + ]); + const dataViews = getService('dataViews'); + const monacoEditor = getService('monacoEditor'); + + const esArchiver = getService('esArchiver'); + const remoteEsArchiver = getService('remoteEsArchiver' as 'esArchiver'); + + describe('discover search CCS cancel', () => { + before(async () => { + await esArchiver.loadIfNeeded( + 'src/platform/test/functional/fixtures/es_archiver/logstash_functional' + ); + await remoteEsArchiver.loadIfNeeded( + 'src/platform/test/functional/fixtures/es_archiver/logstash_functional' + ); + await kibanaServer.importExport.load( + 'src/platform/test/functional/fixtures/kbn_archiver/discover.json' + ); + }); + + after(async () => { + await esArchiver.unload( + 'src/platform/test/functional/fixtures/es_archiver/logstash_functional' + ); + await remoteEsArchiver.unload( + 'src/platform/test/functional/fixtures/es_archiver/logstash_functional' + ); + await kibanaServer.importExport.unload( + 'src/platform/test/functional/fixtures/kbn_archiver/discover.json' + ); + }); + + describe('classic mode', () => { + it('should show warning and results', async () => { + await common.navigateToApp('discover'); + await dataViews.createFromSearchBar({ + name: 'ftr-remote:logstash-*,logstash-*', + hasTimeField: false, + adHoc: true, + }); + + // Add a stall time to the remote indices + await filterBar.addDslFilter( + ` + { + "query": { + "error_query": { + "indices": [ + { + "name": "*:*", + "error_type": "exception", + "message": "'Watch out!'", + "stall_time_seconds": 5 + } + ] + } + } + }`, + false + ); + await new Promise((resolve) => setTimeout(resolve, 1000)); + await testSubjects.exists('queryCancelButton'); + await testSubjects.click('queryCancelButton'); + + // Warning callout is shown + await testSubjects.exists('searchResponseWarningsCallout'); + + // No "timed out" error notification is shown + await toasts.assertCount(0); + + // View cluster details shows timed out + await testSubjects.click('searchResponseWarningsViewDetails'); + + // If both requests have already completed, it will show a context menu first, otherwise it + // will go directly to the details + if (await testSubjects.exists('viewDetailsContextMenu')) { + await testSubjects.click('viewDetailsContextMenu'); + } + + await testSubjects.click('inspectorRequestToggleClusterDetailsftr-remote'); + const txt = await testSubjects.getVisibleText('inspectorRequestClustersDetails'); + expect(txt).to.contain('Results may be incomplete or empty.'); + + // Ensure documents are still returned for the successful shards + await retry.try(async function tryingForTime() { + const hitCount = await discover.getHitCount(); + expect(hitCount).to.be('14,004'); + }); + }); + }); + + describe('esql mode', () => { + it('should show warning and results', async () => { + await common.navigateToApp('discover'); + await discover.selectTextBaseLang(); + await monacoEditor.setCodeEditorValue(`FROM logstash-*, ftr-remote:logstash-* METADATA _index + | EVAL buckets = DATE_TRUNC(5 minute, @timestamp), delay = TO_STRING(CASE(STARTS_WITH(_index, "ftr-remote"), DELAY(10ms), false)) + | STATS count = COUNT(*) BY buckets, delay`); + await timePicker.setDefaultAbsoluteRange(); + await new Promise((resolve) => setTimeout(resolve, 1000)); + await testSubjects.exists('queryCancelButton'); + await testSubjects.click('queryCancelButton'); + + await header.waitUntilLoadingHasFinished(); + // Warning callout is shown + await testSubjects.exists('searchResponseWarningsCallout'); + + // No "timed out" error notification is shown + await toasts.assertCount(0); + + // View cluster details shows timed out + await testSubjects.click('searchResponseWarningsViewDetails'); + + await testSubjects.click('inspectorRequestToggleClusterDetailsftr-remote'); + const txt = await testSubjects.getVisibleText( + 'inspectorRequestClustersTableCell-Status-ftr-remote' + ); + expect(txt).to.be('partial'); + + // Ensure documents are still returned for the successful shards + await retry.try(async () => { + const hitCount = await discover.getHitCount({ isPartial: true }); + expect(hitCount).to.be('746'); + }); + }); + }); + }); +} diff --git a/src/platform/test/functional/apps/discover/ccs_compatibility/index.ts b/src/platform/test/functional/apps/discover/ccs_compatibility/index.ts index 8d382f323348c..38a866824f2f5 100644 --- a/src/platform/test/functional/apps/discover/ccs_compatibility/index.ts +++ b/src/platform/test/functional/apps/discover/ccs_compatibility/index.ts @@ -29,6 +29,9 @@ export default function ({ getService, loadTestFile }: FtrProviderContext) { loadTestFile(require.resolve('./_data_view_editor')); loadTestFile(require.resolve('./_saved_queries')); loadTestFile(require.resolve('./_search_errors')); - if (isCcsTest) loadTestFile(require.resolve('./_timeout_results')); + if (isCcsTest) { + loadTestFile(require.resolve('./_timeout_results')); + loadTestFile(require.resolve('./_cancel_results')); + } }); } diff --git a/src/platform/test/functional/apps/discover/group7/_request_cancellation.ts b/src/platform/test/functional/apps/discover/group7/_request_cancellation.ts index 415118d81497f..55b7e3ffc10df 100644 --- a/src/platform/test/functional/apps/discover/group7/_request_cancellation.ts +++ b/src/platform/test/functional/apps/discover/group7/_request_cancellation.ts @@ -76,7 +76,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { }); await testSubjects.click('queryCancelButton'); await retry.try(async () => { - expect(await discover.hasNoResults()).to.be(true); + expect(await testSubjects.exists('searchResponseWarningsEmptyPrompt')).to.be(true); await testSubjects.existOrFail('querySubmitButton'); await testSubjects.missingOrFail('queryCancelButton'); });