diff --git a/docs/development/plugins/data/public/kibana-plugin-plugins-data-public.searchinterceptor.handlesearcherror.md b/docs/development/plugins/data/public/kibana-plugin-plugins-data-public.searchinterceptor.handlesearcherror.md index b5ac4a4e53887e..5f8966f0227ac5 100644 --- a/docs/development/plugins/data/public/kibana-plugin-plugins-data-public.searchinterceptor.handlesearcherror.md +++ b/docs/development/plugins/data/public/kibana-plugin-plugins-data-public.searchinterceptor.handlesearcherror.md @@ -7,14 +7,14 @@ Signature: ```typescript -protected handleSearchError(e: any, timeoutSignal: AbortSignal, options?: ISearchOptions): Error; +protected handleSearchError(e: KibanaServerError | AbortError, timeoutSignal: AbortSignal, options?: ISearchOptions): Error; ``` ## Parameters | Parameter | Type | Description | | --- | --- | --- | -| e | any | | +| e | KibanaServerError | AbortError | | | timeoutSignal | AbortSignal | | | options | ISearchOptions | | diff --git a/docs/development/plugins/data/public/kibana-plugin-plugins-data-public.searchtimeouterror._constructor_.md b/docs/development/plugins/data/public/kibana-plugin-plugins-data-public.searchtimeouterror._constructor_.md index 1c6370c7d03561..b4eecca665e827 100644 --- a/docs/development/plugins/data/public/kibana-plugin-plugins-data-public.searchtimeouterror._constructor_.md +++ b/docs/development/plugins/data/public/kibana-plugin-plugins-data-public.searchtimeouterror._constructor_.md @@ -9,13 +9,13 @@ Constructs a new instance of the `SearchTimeoutError` class Signature: ```typescript -constructor(err: Error, mode: TimeoutErrorMode); +constructor(err: Record, mode: TimeoutErrorMode); ``` ## Parameters | Parameter | Type | Description | | --- | --- | --- | -| err | Error | | +| err | Record<string, any> | | | mode | TimeoutErrorMode | | diff --git a/src/plugins/data/common/search/test_data/illegal_argument_exception.json b/src/plugins/data/common/search/test_data/illegal_argument_exception.json new file mode 100644 index 00000000000000..ae48468abc209d --- /dev/null +++ b/src/plugins/data/common/search/test_data/illegal_argument_exception.json @@ -0,0 +1,14 @@ +{ + "error" : { + "root_cause" : [ + { + "type" : "illegal_argument_exception", + "reason" : "failed to parse setting [timeout] with value [1] as a time value: unit is missing or unrecognized" + } + ], + "type" : "illegal_argument_exception", + "reason" : "failed to parse setting [timeout] with value [1] as a time value: unit is missing or unrecognized" + }, + "status" : 400 + } + \ No newline at end of file diff --git a/src/plugins/data/common/search/test_data/index_not_found_exception.json b/src/plugins/data/common/search/test_data/index_not_found_exception.json new file mode 100644 index 00000000000000..dc892d95ae3974 --- /dev/null +++ b/src/plugins/data/common/search/test_data/index_not_found_exception.json @@ -0,0 +1,21 @@ +{ + "error" : { + "root_cause" : [ + { + "type" : "index_not_found_exception", + "reason" : "no such index [poop]", + "resource.type" : "index_or_alias", + "resource.id" : "poop", + "index_uuid" : "_na_", + "index" : "poop" + } + ], + "type" : "index_not_found_exception", + "reason" : "no such index [poop]", + "resource.type" : "index_or_alias", + "resource.id" : "poop", + "index_uuid" : "_na_", + "index" : "poop" + }, + "status" : 404 +} diff --git a/src/plugins/data/common/search/test_data/json_e_o_f_exception.json b/src/plugins/data/common/search/test_data/json_e_o_f_exception.json new file mode 100644 index 00000000000000..88134e1c6ea03b --- /dev/null +++ b/src/plugins/data/common/search/test_data/json_e_o_f_exception.json @@ -0,0 +1,14 @@ +{ + "error" : { + "root_cause" : [ + { + "type" : "json_e_o_f_exception", + "reason" : "Unexpected end-of-input: expected close marker for Object (start marker at [Source: (org.elasticsearch.common.io.stream.InputStreamStreamInput); line: 1, column: 1])\n at [Source: (org.elasticsearch.common.io.stream.InputStreamStreamInput); line: 1, column: 2]" + } + ], + "type" : "json_e_o_f_exception", + "reason" : "Unexpected end-of-input: expected close marker for Object (start marker at [Source: (org.elasticsearch.common.io.stream.InputStreamStreamInput); line: 1, column: 1])\n at [Source: (org.elasticsearch.common.io.stream.InputStreamStreamInput); line: 1, column: 2]" + }, + "status" : 400 + } + \ No newline at end of file diff --git a/src/plugins/data/common/search/test_data/parsing_exception.json b/src/plugins/data/common/search/test_data/parsing_exception.json new file mode 100644 index 00000000000000..725a847aa0e3f5 --- /dev/null +++ b/src/plugins/data/common/search/test_data/parsing_exception.json @@ -0,0 +1,17 @@ +{ + "error" : { + "root_cause" : [ + { + "type" : "parsing_exception", + "reason" : "[terms] query does not support [ohno]", + "line" : 4, + "col" : 17 + } + ], + "type" : "parsing_exception", + "reason" : "[terms] query does not support [ohno]", + "line" : 4, + "col" : 17 + }, + "status" : 400 +} diff --git a/src/plugins/data/common/search/test_data/resource_not_found_exception.json b/src/plugins/data/common/search/test_data/resource_not_found_exception.json new file mode 100644 index 00000000000000..7f2a3b2e6e1439 --- /dev/null +++ b/src/plugins/data/common/search/test_data/resource_not_found_exception.json @@ -0,0 +1,13 @@ +{ + "error" : { + "root_cause" : [ + { + "type" : "resource_not_found_exception", + "reason" : "FlZlSXp6dkd3UXdHZjhsalVtVHBnYkEdYjNIWDhDOTZRN3ExemdmVkx4RXNQQToxMjc2ODk=" + } + ], + "type" : "resource_not_found_exception", + "reason" : "FlZlSXp6dkd3UXdHZjhsalVtVHBnYkEdYjNIWDhDOTZRN3ExemdmVkx4RXNQQToxMjc2ODk=" + }, + "status" : 404 +} diff --git a/src/plugins/data/common/search/test_data/search_phase_execution_exception.json b/src/plugins/data/common/search/test_data/search_phase_execution_exception.json new file mode 100644 index 00000000000000..ff6879f2b89609 --- /dev/null +++ b/src/plugins/data/common/search/test_data/search_phase_execution_exception.json @@ -0,0 +1,52 @@ +{ + "error" : { + "root_cause" : [ + { + "type" : "script_exception", + "reason" : "compile error", + "script_stack" : [ + "invalid", + "^---- HERE" + ], + "script" : "invalid", + "lang" : "painless", + "position" : { + "offset" : 0, + "start" : 0, + "end" : 7 + } + } + ], + "type" : "search_phase_execution_exception", + "reason" : "all shards failed", + "phase" : "query", + "grouped" : true, + "failed_shards" : [ + { + "shard" : 0, + "index" : ".kibana_11", + "node" : "b3HX8C96Q7q1zgfVLxEsPA", + "reason" : { + "type" : "script_exception", + "reason" : "compile error", + "script_stack" : [ + "invalid", + "^---- HERE" + ], + "script" : "invalid", + "lang" : "painless", + "position" : { + "offset" : 0, + "start" : 0, + "end" : 7 + }, + "caused_by" : { + "type" : "illegal_argument_exception", + "reason" : "cannot resolve symbol [invalid]" + } + } + } + ] + }, + "status" : 400 +} diff --git a/src/plugins/data/common/search/test_data/x_content_parse_exception.json b/src/plugins/data/common/search/test_data/x_content_parse_exception.json new file mode 100644 index 00000000000000..cd6e1cb2c5977d --- /dev/null +++ b/src/plugins/data/common/search/test_data/x_content_parse_exception.json @@ -0,0 +1,17 @@ +{ + "error" : { + "root_cause" : [ + { + "type" : "x_content_parse_exception", + "reason" : "[5:13] [script] failed to parse object" + } + ], + "type" : "x_content_parse_exception", + "reason" : "[5:13] [script] failed to parse object", + "caused_by" : { + "type" : "json_parse_exception", + "reason" : "Unexpected character (''' (code 39)): expected a valid value (JSON String, Number, Array, Object or token 'null', 'true' or 'false')\n at [Source: (org.elasticsearch.common.bytes.AbstractBytesReference$BytesReferenceStreamInput); line: 5, column: 24]" + } + }, + "status" : 400 +} diff --git a/src/plugins/data/public/public.api.md b/src/plugins/data/public/public.api.md index 952f6684fcae6b..8feed5c86fc78d 100644 --- a/src/plugins/data/public/public.api.md +++ b/src/plugins/data/public/public.api.md @@ -2290,8 +2290,11 @@ export class SearchInterceptor { protected readonly deps: SearchInterceptorDeps; // (undocumented) protected getTimeoutMode(): TimeoutErrorMode; + // Warning: (ae-forgotten-export) The symbol "KibanaServerError" needs to be exported by the entry point index.d.ts + // Warning: (ae-forgotten-export) The symbol "AbortError" needs to be exported by the entry point index.d.ts + // // (undocumented) - protected handleSearchError(e: any, timeoutSignal: AbortSignal, options?: ISearchOptions): Error; + protected handleSearchError(e: KibanaServerError | AbortError, timeoutSignal: AbortSignal, options?: ISearchOptions): Error; // @internal protected pendingCount$: BehaviorSubject; // @internal (undocumented) @@ -2461,7 +2464,7 @@ export interface SearchSourceFields { // // @public export class SearchTimeoutError extends KbnError { - constructor(err: Error, mode: TimeoutErrorMode); + constructor(err: Record, mode: TimeoutErrorMode); // (undocumented) getErrorMessage(application: ApplicationStart): JSX.Element; // (undocumented) diff --git a/src/plugins/data/public/search/errors/es_error.test.tsx b/src/plugins/data/public/search/errors/es_error.test.tsx index adb422c1d18e75..6a4cb9c494b4f9 100644 --- a/src/plugins/data/public/search/errors/es_error.test.tsx +++ b/src/plugins/data/public/search/errors/es_error.test.tsx @@ -7,23 +7,22 @@ */ import { EsError } from './es_error'; -import { IEsError } from './types'; describe('EsError', () => { it('contains the same body as the wrapped error', () => { const error = { - body: { - attributes: { - error: { - type: 'top_level_exception_type', - reason: 'top-level reason', - }, + statusCode: 500, + message: 'nope', + attributes: { + error: { + type: 'top_level_exception_type', + reason: 'top-level reason', }, }, - } as IEsError; + } as any; const esError = new EsError(error); - expect(typeof esError.body).toEqual('object'); - expect(esError.body).toEqual(error.body); + expect(typeof esError.attributes).toEqual('object'); + expect(esError.attributes).toEqual(error.attributes); }); }); diff --git a/src/plugins/data/public/search/errors/es_error.tsx b/src/plugins/data/public/search/errors/es_error.tsx index fff06d2e1bfb64..d241eecfd8d5dd 100644 --- a/src/plugins/data/public/search/errors/es_error.tsx +++ b/src/plugins/data/public/search/errors/es_error.tsx @@ -11,19 +11,19 @@ import { EuiCodeBlock, EuiSpacer } from '@elastic/eui'; import { ApplicationStart } from 'kibana/public'; import { KbnError } from '../../../../kibana_utils/common'; import { IEsError } from './types'; -import { getRootCause, getTopLevelCause } from './utils'; +import { getRootCause } from './utils'; export class EsError extends KbnError { - readonly body: IEsError['body']; + readonly attributes: IEsError['attributes']; constructor(protected readonly err: IEsError) { super('EsError'); - this.body = err.body; + this.attributes = err.attributes; } public getErrorMessage(application: ApplicationStart) { const rootCause = getRootCause(this.err)?.reason; - const topLevelCause = getTopLevelCause(this.err)?.reason; + const topLevelCause = this.attributes?.reason; const cause = rootCause ?? topLevelCause; return ( diff --git a/src/plugins/data/public/search/errors/painless_error.test.tsx b/src/plugins/data/public/search/errors/painless_error.test.tsx new file mode 100644 index 00000000000000..929f25e234a604 --- /dev/null +++ b/src/plugins/data/public/search/errors/painless_error.test.tsx @@ -0,0 +1,42 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * and the Server Side Public License, v 1; you may not use this file except in + * compliance with, at your election, the Elastic License or the Server Side + * Public License, v 1. + */ + +import { coreMock } from '../../../../../core/public/mocks'; +const startMock = coreMock.createStart(); + +import { mount } from 'enzyme'; +import { PainlessError } from './painless_error'; +import { findTestSubject } from '@elastic/eui/lib/test'; +import * as searchPhaseException from '../../../common/search/test_data/search_phase_execution_exception.json'; + +describe('PainlessError', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('Should show reason and code', () => { + const e = new PainlessError({ + statusCode: 400, + message: 'search_phase_execution_exception', + attributes: searchPhaseException.error, + }); + const component = mount(e.getErrorMessage(startMock.application)); + + const scriptElem = findTestSubject(component, 'painlessScript').getDOMNode(); + + const failedShards = e.attributes?.failed_shards![0]; + const script = failedShards!.reason.script; + expect(scriptElem.textContent).toBe(`Error executing Painless script: '${script}'`); + + const stackTraceElem = findTestSubject(component, 'painlessStackTrace').getDOMNode(); + const stackTrace = failedShards!.reason.script_stack!.join('\n'); + expect(stackTraceElem.textContent).toBe(stackTrace); + + expect(component.find('EuiButton').length).toBe(1); + }); +}); diff --git a/src/plugins/data/public/search/errors/painless_error.tsx b/src/plugins/data/public/search/errors/painless_error.tsx index 8a4248e48185bc..6d11f3a16b09e8 100644 --- a/src/plugins/data/public/search/errors/painless_error.tsx +++ b/src/plugins/data/public/search/errors/painless_error.tsx @@ -33,10 +33,12 @@ export class PainlessError extends EsError { return ( <> - {i18n.translate('data.painlessError.painlessScriptedFieldErrorMessage', { - defaultMessage: "Error executing Painless script: '{script}'.", - values: { script: rootCause?.script }, - })} + + {i18n.translate('data.painlessError.painlessScriptedFieldErrorMessage', { + defaultMessage: "Error executing Painless script: '{script}'", + values: { script: rootCause?.script }, + })} + {painlessStack ? ( diff --git a/src/plugins/data/public/search/errors/timeout_error.tsx b/src/plugins/data/public/search/errors/timeout_error.tsx index ee2703b888bf17..6b9ce1b422481f 100644 --- a/src/plugins/data/public/search/errors/timeout_error.tsx +++ b/src/plugins/data/public/search/errors/timeout_error.tsx @@ -24,7 +24,7 @@ export enum TimeoutErrorMode { */ export class SearchTimeoutError extends KbnError { public mode: TimeoutErrorMode; - constructor(err: Error, mode: TimeoutErrorMode) { + constructor(err: Record, mode: TimeoutErrorMode) { super(`Request timeout: ${JSON.stringify(err?.message)}`); this.mode = mode; } diff --git a/src/plugins/data/public/search/errors/types.ts b/src/plugins/data/public/search/errors/types.ts index d62cb311bf6a43..5806ef8676b9bd 100644 --- a/src/plugins/data/public/search/errors/types.ts +++ b/src/plugins/data/public/search/errors/types.ts @@ -6,57 +6,47 @@ * Public License, v 1. */ +import { KibanaServerError } from '../../../../kibana_utils/common'; + export interface FailedShard { shard: number; index: string; node: string; - reason: { + reason: Reason; +} + +export interface Reason { + type: string; + reason: string; + script_stack?: string[]; + position?: { + offset: number; + start: number; + end: number; + }; + lang?: string; + script?: string; + caused_by?: { type: string; reason: string; - script_stack: string[]; - script: string; - lang: string; - position: { - offset: number; - start: number; - end: number; - }; - caused_by: { - type: string; - reason: string; - }; }; } -export interface IEsError { - body: { - statusCode: number; - error: string; - message: string; - attributes?: { - error?: { - root_cause?: [ - { - lang: string; - script: string; - } - ]; - type: string; - reason: string; - failed_shards: FailedShard[]; - caused_by: { - type: string; - reason: string; - phase: string; - grouped: boolean; - failed_shards: FailedShard[]; - script_stack: string[]; - }; - }; - }; - }; +export interface IEsErrorAttributes { + type: string; + reason: string; + root_cause?: Reason[]; + failed_shards?: FailedShard[]; } +export type IEsError = KibanaServerError; + +/** + * Checks if a given errors originated from Elasticsearch. + * Those params are assigned to the attributes property of an error. + * + * @param e + */ export function isEsError(e: any): e is IEsError { - return !!e.body?.attributes; + return !!e.attributes; } diff --git a/src/plugins/data/public/search/errors/utils.ts b/src/plugins/data/public/search/errors/utils.ts index d140e713f9440d..7d303543a0c57d 100644 --- a/src/plugins/data/public/search/errors/utils.ts +++ b/src/plugins/data/public/search/errors/utils.ts @@ -6,19 +6,15 @@ * Public License, v 1. */ -import { IEsError } from './types'; +import { FailedShard } from './types'; +import { KibanaServerError } from '../../../../kibana_utils/common'; -export function getFailedShards(err: IEsError) { - const failedShards = - err.body?.attributes?.error?.failed_shards || - err.body?.attributes?.error?.caused_by?.failed_shards; +export function getFailedShards(err: KibanaServerError): FailedShard | undefined { + const errorInfo = err.attributes; + const failedShards = errorInfo?.failed_shards || errorInfo?.caused_by?.failed_shards; return failedShards ? failedShards[0] : undefined; } -export function getTopLevelCause(err: IEsError) { - return err.body?.attributes?.error; -} - -export function getRootCause(err: IEsError) { +export function getRootCause(err: KibanaServerError) { return getFailedShards(err)?.reason; } diff --git a/src/plugins/data/public/search/search_interceptor.test.ts b/src/plugins/data/public/search/search_interceptor.test.ts index 5ae01eccdd920c..bfd73951c31c48 100644 --- a/src/plugins/data/public/search/search_interceptor.test.ts +++ b/src/plugins/data/public/search/search_interceptor.test.ts @@ -12,12 +12,15 @@ import { coreMock } from '../../../../core/public/mocks'; import { IEsSearchRequest } from '../../common/search'; import { SearchInterceptor } from './search_interceptor'; import { AbortError } from '../../../kibana_utils/public'; -import { SearchTimeoutError, PainlessError, TimeoutErrorMode } from './errors'; +import { SearchTimeoutError, PainlessError, TimeoutErrorMode, EsError } from './errors'; import { searchServiceMock } from './mocks'; import { ISearchStart, ISessionService } from '.'; import { bfetchPluginMock } from '../../../bfetch/public/mocks'; import { BfetchPublicSetup } from 'src/plugins/bfetch/public'; +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'; + let searchInterceptor: SearchInterceptor; let mockCoreSetup: MockedKeys; let bfetchSetup: jest.Mocked; @@ -64,15 +67,9 @@ describe('SearchInterceptor', () => { test('Renders a PainlessError', async () => { searchInterceptor.showError( new PainlessError({ - body: { - attributes: { - error: { - failed_shards: { - reason: 'bananas', - }, - }, - }, - } as any, + statusCode: 400, + message: 'search_phase_execution_exception', + attributes: searchPhaseException.error, }) ); expect(mockCoreSetup.notifications.toasts.addDanger).toBeCalledTimes(1); @@ -161,10 +158,8 @@ describe('SearchInterceptor', () => { describe('Should handle Timeout errors', () => { test('Should throw SearchTimeoutError on server timeout AND show toast', async () => { const mockResponse: any = { - result: 500, - body: { - message: 'Request timed out', - }, + statusCode: 500, + message: 'Request timed out', }; fetchMock.mockRejectedValueOnce(mockResponse); const mockRequest: IEsSearchRequest = { @@ -177,10 +172,8 @@ describe('SearchInterceptor', () => { test('Timeout error should show multiple times if not in a session', async () => { const mockResponse: any = { - result: 500, - body: { - message: 'Request timed out', - }, + statusCode: 500, + message: 'Request timed out', }; fetchMock.mockRejectedValue(mockResponse); const mockRequest: IEsSearchRequest = { @@ -198,10 +191,8 @@ describe('SearchInterceptor', () => { test('Timeout error should show once per each session', async () => { const mockResponse: any = { - result: 500, - body: { - message: 'Request timed out', - }, + statusCode: 500, + message: 'Request timed out', }; fetchMock.mockRejectedValue(mockResponse); const mockRequest: IEsSearchRequest = { @@ -219,10 +210,8 @@ describe('SearchInterceptor', () => { test('Timeout error should show once in a single session', async () => { const mockResponse: any = { - result: 500, - body: { - message: 'Request timed out', - }, + statusCode: 500, + message: 'Request timed out', }; fetchMock.mockRejectedValue(mockResponse); const mockRequest: IEsSearchRequest = { @@ -240,22 +229,9 @@ describe('SearchInterceptor', () => { test('Should throw Painless error on server error with OSS format', async () => { const mockResponse: any = { - result: 500, - body: { - attributes: { - error: { - failed_shards: [ - { - reason: { - lang: 'painless', - script_stack: ['a', 'b'], - reason: 'banana', - }, - }, - ], - }, - }, - }, + statusCode: 400, + message: 'search_phase_execution_exception', + attributes: searchPhaseException.error, }; fetchMock.mockRejectedValueOnce(mockResponse); const mockRequest: IEsSearchRequest = { @@ -265,6 +241,20 @@ describe('SearchInterceptor', () => { await expect(response.toPromise()).rejects.toThrow(PainlessError); }); + test('Should throw ES error on ES server error', async () => { + const mockResponse: any = { + statusCode: 400, + message: 'resource_not_found_exception', + attributes: resourceNotFoundException.error, + }; + fetchMock.mockRejectedValueOnce(mockResponse); + const mockRequest: IEsSearchRequest = { + params: {}, + }; + const response = searchInterceptor.search(mockRequest); + await expect(response.toPromise()).rejects.toThrow(EsError); + }); + test('Observable should fail if user aborts (test merged signal)', async () => { const abortController = new AbortController(); fetchMock.mockImplementationOnce((options: any) => { diff --git a/src/plugins/data/public/search/search_interceptor.ts b/src/plugins/data/public/search/search_interceptor.ts index f6ca9ef1a993de..6dfc8faea769ea 100644 --- a/src/plugins/data/public/search/search_interceptor.ts +++ b/src/plugins/data/public/search/search_interceptor.ts @@ -6,7 +6,7 @@ * Public License, v 1. */ -import { get, memoize } from 'lodash'; +import { memoize } from 'lodash'; import { BehaviorSubject, throwError, timer, defer, from, Observable, NEVER } from 'rxjs'; import { catchError, finalize } from 'rxjs/operators'; import { PublicMethodsOf } from '@kbn/utility-types'; @@ -25,7 +25,11 @@ import { getHttpError, } from './errors'; import { toMountPoint } from '../../../kibana_react/public'; -import { AbortError, getCombinedAbortSignal } from '../../../kibana_utils/public'; +import { + AbortError, + getCombinedAbortSignal, + KibanaServerError, +} from '../../../kibana_utils/public'; import { ISessionService } from './session'; export interface SearchInterceptorDeps { @@ -87,8 +91,12 @@ export class SearchInterceptor { * @returns `Error` a search service specific error or the original error, if a specific error can't be recognized. * @internal */ - protected handleSearchError(e: any, timeoutSignal: AbortSignal, options?: ISearchOptions): Error { - if (timeoutSignal.aborted || get(e, 'body.message') === 'Request timed out') { + protected handleSearchError( + e: KibanaServerError | AbortError, + timeoutSignal: AbortSignal, + options?: ISearchOptions + ): Error { + if (timeoutSignal.aborted || e.message === 'Request timed out') { // Handle a client or a server side timeout const err = new SearchTimeoutError(e, this.getTimeoutMode()); @@ -96,7 +104,7 @@ export class SearchInterceptor { // The timeout error is shown any time a request times out, or once per session, if the request is part of a session. this.showTimeoutError(err, options?.sessionId); return err; - } else if (options?.abortSignal?.aborted) { + } else if (e instanceof AbortError) { // In the case an application initiated abort, throw the existing AbortError. return e; } else if (isEsError(e)) { @@ -106,12 +114,13 @@ export class SearchInterceptor { return new EsError(e); } } else { - return e; + return e instanceof Error ? e : new Error(e.message); } } /** * @internal + * @throws `AbortError` | `ErrorLike` */ protected runSearch( request: IKibanaSearchRequest, @@ -234,7 +243,7 @@ export class SearchInterceptor { }); this.pendingCount$.next(this.pendingCount$.getValue() + 1); return from(this.runSearch(request, { ...options, abortSignal: combinedSignal })).pipe( - catchError((e: Error) => { + catchError((e: Error | AbortError) => { return throwError(this.handleSearchError(e, timeoutSignal, options)); }), finalize(() => { diff --git a/src/plugins/data/server/search/es_search/es_search_strategy.test.ts b/src/plugins/data/server/search/es_search/es_search_strategy.test.ts index 8e66729825e39c..eeef46381732e8 100644 --- a/src/plugins/data/server/search/es_search/es_search_strategy.test.ts +++ b/src/plugins/data/server/search/es_search/es_search_strategy.test.ts @@ -6,37 +6,56 @@ * Public License, v 1. */ +import { + elasticsearchClientMock, + MockedTransportRequestPromise, + // eslint-disable-next-line @kbn/eslint/no-restricted-paths +} from '../../../../../core/server/elasticsearch/client/mocks'; import { pluginInitializerContextConfigMock } from '../../../../../core/server/mocks'; import { esSearchStrategyProvider } from './es_search_strategy'; import { SearchStrategyDependencies } from '../types'; +import * as indexNotFoundException from '../../../common/search/test_data/index_not_found_exception.json'; +import { ElasticsearchClientError, ResponseError } from '@elastic/elasticsearch/lib/errors'; +import { KbnServerError } from '../../../../kibana_utils/server'; + describe('ES search strategy', () => { + const successBody = { + _shards: { + total: 10, + failed: 1, + skipped: 2, + successful: 7, + }, + }; + let mockedApiCaller: MockedTransportRequestPromise; + let mockApiCaller: jest.Mock<() => MockedTransportRequestPromise>; const mockLogger: any = { debug: () => {}, }; - const mockApiCaller = jest.fn().mockResolvedValue({ - body: { - _shards: { - total: 10, - failed: 1, - skipped: 2, - successful: 7, - }, - }, - }); - const mockDeps = ({ - uiSettingsClient: { - get: () => {}, - }, - esClient: { asCurrentUser: { search: mockApiCaller } }, - } as unknown) as SearchStrategyDependencies; + function getMockedDeps(err?: Record) { + mockApiCaller = jest.fn().mockImplementation(() => { + if (err) { + mockedApiCaller = elasticsearchClientMock.createErrorTransportRequestPromise(err); + } else { + mockedApiCaller = elasticsearchClientMock.createSuccessTransportRequestPromise( + successBody, + { statusCode: 200 } + ); + } + return mockedApiCaller; + }); - const mockConfig$ = pluginInitializerContextConfigMock({}).legacy.globalConfig$; + return ({ + uiSettingsClient: { + get: () => {}, + }, + esClient: { asCurrentUser: { search: mockApiCaller } }, + } as unknown) as SearchStrategyDependencies; + } - beforeEach(() => { - mockApiCaller.mockClear(); - }); + const mockConfig$ = pluginInitializerContextConfigMock({}).legacy.globalConfig$; it('returns a strategy with `search`', async () => { const esSearch = await esSearchStrategyProvider(mockConfig$, mockLogger); @@ -48,7 +67,7 @@ describe('ES search strategy', () => { const params = { index: 'logstash-*' }; await esSearchStrategyProvider(mockConfig$, mockLogger) - .search({ params }, {}, mockDeps) + .search({ params }, {}, getMockedDeps()) .subscribe(() => { expect(mockApiCaller).toBeCalled(); expect(mockApiCaller.mock.calls[0][0]).toEqual({ @@ -64,7 +83,7 @@ describe('ES search strategy', () => { const params = { index: 'logstash-*', ignore_unavailable: false, timeout: '1000ms' }; await esSearchStrategyProvider(mockConfig$, mockLogger) - .search({ params }, {}, mockDeps) + .search({ params }, {}, getMockedDeps()) .subscribe(() => { expect(mockApiCaller).toBeCalled(); expect(mockApiCaller.mock.calls[0][0]).toEqual({ @@ -82,13 +101,109 @@ describe('ES search strategy', () => { params: { index: 'logstash-*' }, }, {}, - mockDeps + getMockedDeps() ) .subscribe((data) => { expect(data.isRunning).toBe(false); expect(data.isPartial).toBe(false); expect(data).toHaveProperty('loaded'); expect(data).toHaveProperty('rawResponse'); + expect(mockedApiCaller.abort).not.toBeCalled(); done(); })); + + it('can be aborted', async () => { + const params = { index: 'logstash-*', ignore_unavailable: false, timeout: '1000ms' }; + + const abortController = new AbortController(); + abortController.abort(); + + await esSearchStrategyProvider(mockConfig$, mockLogger) + .search({ params }, { abortSignal: abortController.signal }, getMockedDeps()) + .toPromise(); + + expect(mockApiCaller).toBeCalled(); + expect(mockApiCaller.mock.calls[0][0]).toEqual({ + ...params, + track_total_hits: true, + }); + expect(mockedApiCaller.abort).toBeCalled(); + }); + + it('throws normalized error if ResponseError is thrown', async (done) => { + const params = { index: 'logstash-*', ignore_unavailable: false, timeout: '1000ms' }; + const errResponse = new ResponseError({ + body: indexNotFoundException, + statusCode: 404, + headers: {}, + warnings: [], + meta: {} as any, + }); + + try { + await esSearchStrategyProvider(mockConfig$, mockLogger) + .search({ params }, {}, getMockedDeps(errResponse)) + .toPromise(); + } catch (e) { + expect(mockApiCaller).toBeCalled(); + expect(e).toBeInstanceOf(KbnServerError); + expect(e.statusCode).toBe(404); + expect(e.message).toBe(errResponse.message); + expect(e.errBody).toBe(indexNotFoundException); + done(); + } + }); + + it('throws normalized error if ElasticsearchClientError is thrown', async (done) => { + const params = { index: 'logstash-*', ignore_unavailable: false, timeout: '1000ms' }; + const errResponse = new ElasticsearchClientError('This is a general ESClient error'); + + try { + await esSearchStrategyProvider(mockConfig$, mockLogger) + .search({ params }, {}, getMockedDeps(errResponse)) + .toPromise(); + } catch (e) { + expect(mockApiCaller).toBeCalled(); + expect(e).toBeInstanceOf(KbnServerError); + expect(e.statusCode).toBe(500); + expect(e.message).toBe(errResponse.message); + expect(e.errBody).toBe(undefined); + done(); + } + }); + + it('throws normalized error if ESClient throws unknown error', async (done) => { + const params = { index: 'logstash-*', ignore_unavailable: false, timeout: '1000ms' }; + const errResponse = new Error('ESClient error'); + + try { + await esSearchStrategyProvider(mockConfig$, mockLogger) + .search({ params }, {}, getMockedDeps(errResponse)) + .toPromise(); + } catch (e) { + expect(mockApiCaller).toBeCalled(); + expect(e).toBeInstanceOf(KbnServerError); + expect(e.statusCode).toBe(500); + expect(e.message).toBe(errResponse.message); + expect(e.errBody).toBe(undefined); + done(); + } + }); + + it('throws KbnServerError for unknown index type', async (done) => { + const params = { index: 'logstash-*', ignore_unavailable: false, timeout: '1000ms' }; + + try { + await esSearchStrategyProvider(mockConfig$, mockLogger) + .search({ indexType: 'banana', params }, {}, getMockedDeps()) + .toPromise(); + } catch (e) { + expect(mockApiCaller).not.toBeCalled(); + expect(e).toBeInstanceOf(KbnServerError); + expect(e.message).toBe('Unsupported index pattern type banana'); + expect(e.statusCode).toBe(400); + expect(e.errBody).toBe(undefined); + done(); + } + }); }); diff --git a/src/plugins/data/server/search/es_search/es_search_strategy.ts b/src/plugins/data/server/search/es_search/es_search_strategy.ts index a11bbe11f3f959..c176a50627b92e 100644 --- a/src/plugins/data/server/search/es_search/es_search_strategy.ts +++ b/src/plugins/data/server/search/es_search/es_search_strategy.ts @@ -15,13 +15,20 @@ import type { SearchUsage } from '../collectors'; import { getDefaultSearchParams, getShardTimeout, shimAbortSignal } from './request_utils'; import { toKibanaSearchResponse } from './response_utils'; import { searchUsageObserver } from '../collectors/usage'; -import { KbnServerError } from '../../../../kibana_utils/server'; +import { getKbnServerError, KbnServerError } from '../../../../kibana_utils/server'; export const esSearchStrategyProvider = ( config$: Observable, logger: Logger, usage?: SearchUsage ): ISearchStrategy => ({ + /** + * @param request + * @param options + * @param deps + * @throws `KbnServerError` + * @returns `Observable>` + */ search: (request, { abortSignal }, { esClient, uiSettingsClient }) => { // Only default index pattern type is supported here. // See data_enhanced for other type support. @@ -30,15 +37,19 @@ export const esSearchStrategyProvider = ( } const search = async () => { - const config = await config$.pipe(first()).toPromise(); - const params = { - ...(await getDefaultSearchParams(uiSettingsClient)), - ...getShardTimeout(config), - ...request.params, - }; - const promise = esClient.asCurrentUser.search>(params); - const { body } = await shimAbortSignal(promise, abortSignal); - return toKibanaSearchResponse(body); + try { + const config = await config$.pipe(first()).toPromise(); + const params = { + ...(await getDefaultSearchParams(uiSettingsClient)), + ...getShardTimeout(config), + ...request.params, + }; + const promise = esClient.asCurrentUser.search>(params); + const { body } = await shimAbortSignal(promise, abortSignal); + return toKibanaSearchResponse(body); + } catch (e) { + throw getKbnServerError(e); + } }; return from(search()).pipe(tap(searchUsageObserver(logger, usage))); diff --git a/src/plugins/data/server/search/routes/bsearch.ts b/src/plugins/data/server/search/routes/bsearch.ts new file mode 100644 index 00000000000000..e30b7bdaa84022 --- /dev/null +++ b/src/plugins/data/server/search/routes/bsearch.ts @@ -0,0 +1,65 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * and the Server Side Public License, v 1; you may not use this file except in + * compliance with, at your election, the Elastic License or the Server Side + * Public License, v 1. + */ + +import { catchError, first, map } from 'rxjs/operators'; +import { CoreStart, KibanaRequest } from 'src/core/server'; +import { BfetchServerSetup } from 'src/plugins/bfetch/server'; +import { + IKibanaSearchRequest, + IKibanaSearchResponse, + ISearchClient, + ISearchOptions, +} from '../../../common/search'; +import { shimHitsTotal } from './shim_hits_total'; + +type GetScopedProider = (coreStart: CoreStart) => (request: KibanaRequest) => ISearchClient; + +export function registerBsearchRoute( + bfetch: BfetchServerSetup, + coreStartPromise: Promise<[CoreStart, {}, {}]>, + getScopedProvider: GetScopedProider +): void { + bfetch.addBatchProcessingRoute< + { request: IKibanaSearchRequest; options?: ISearchOptions }, + IKibanaSearchResponse + >('/internal/bsearch', (request) => { + return { + /** + * @param requestOptions + * @throws `KibanaServerError` + */ + onBatchItem: async ({ request: requestData, options }) => { + const coreStart = await coreStartPromise; + const search = getScopedProvider(coreStart[0])(request); + return search + .search(requestData, options) + .pipe( + first(), + map((response) => { + return { + ...response, + ...{ + rawResponse: shimHitsTotal(response.rawResponse), + }, + }; + }), + catchError((err) => { + // Re-throw as object, to get attributes passed to the client + // eslint-disable-next-line no-throw-literal + throw { + message: err.message, + statusCode: err.statusCode, + attributes: err.errBody?.error, + }; + }) + ) + .toPromise(); + }, + }; + }); +} diff --git a/src/plugins/data/server/search/routes/call_msearch.ts b/src/plugins/data/server/search/routes/call_msearch.ts index 6578774f65a3c8..fc30e2f29c3ef2 100644 --- a/src/plugins/data/server/search/routes/call_msearch.ts +++ b/src/plugins/data/server/search/routes/call_msearch.ts @@ -8,12 +8,12 @@ import { Observable } from 'rxjs'; import { first } from 'rxjs/operators'; -import { ApiResponse } from '@elastic/elasticsearch'; import { SearchResponse } from 'elasticsearch'; import { IUiSettingsClient, IScopedClusterClient, SharedGlobalConfig } from 'src/core/server'; import type { MsearchRequestBody, MsearchResponse } from '../../../common/search/search_source'; import { shimHitsTotal } from './shim_hits_total'; +import { getKbnServerError } from '../../../../kibana_utils/server'; import { getShardTimeout, getDefaultSearchParams, shimAbortSignal } from '..'; /** @internal */ @@ -48,6 +48,9 @@ interface CallMsearchDependencies { * @internal */ export function getCallMsearch(dependencies: CallMsearchDependencies) { + /** + * @throws KbnServerError + */ return async (params: { body: MsearchRequestBody; signal?: AbortSignal; @@ -61,28 +64,29 @@ export function getCallMsearch(dependencies: CallMsearchDependencies) { // trackTotalHits is not supported by msearch const { track_total_hits: _, ...defaultParams } = await getDefaultSearchParams(uiSettings); - const body = convertRequestBody(params.body, timeout); - - const promise = shimAbortSignal( - esClient.asCurrentUser.msearch( + try { + const promise = esClient.asCurrentUser.msearch( { - body, + body: convertRequestBody(params.body, timeout), }, { querystring: defaultParams, } - ), - params.signal - ); - const response = (await promise) as ApiResponse<{ responses: Array> }>; + ); + const response = await shimAbortSignal(promise, params.signal); - return { - body: { - ...response, + return { body: { - responses: response.body.responses?.map((r: SearchResponse) => shimHitsTotal(r)), + ...response, + body: { + responses: response.body.responses?.map((r: SearchResponse) => + shimHitsTotal(r) + ), + }, }, - }, - }; + }; + } catch (e) { + throw getKbnServerError(e); + } }; } diff --git a/src/plugins/data/server/search/routes/msearch.test.ts b/src/plugins/data/server/search/routes/msearch.test.ts index 02f200d5435dda..a847931a49123d 100644 --- a/src/plugins/data/server/search/routes/msearch.test.ts +++ b/src/plugins/data/server/search/routes/msearch.test.ts @@ -24,6 +24,8 @@ import { convertRequestBody } from './call_msearch'; import { registerMsearchRoute } from './msearch'; import { DataPluginStart } from '../../plugin'; import { dataPluginMock } from '../../mocks'; +import * as jsonEofException from '../../../common/search/test_data/json_e_o_f_exception.json'; +import { ResponseError } from '@elastic/elasticsearch/lib/errors'; describe('msearch route', () => { let mockDataStart: MockedKeys; @@ -76,15 +78,52 @@ describe('msearch route', () => { }); }); - it('handler throws an error if the search throws an error', async () => { - const response = { - message: 'oh no', - body: { - error: 'oops', + it('handler returns an error response if the search throws an error', async () => { + const rejectedValue = Promise.reject( + new ResponseError({ + body: jsonEofException, + statusCode: 400, + meta: {} as any, + headers: [], + warnings: [], + }) + ); + const mockClient = { + msearch: jest.fn().mockReturnValue(rejectedValue), + }; + const mockContext = { + core: { + elasticsearch: { client: { asCurrentUser: mockClient } }, + uiSettings: { client: { get: jest.fn() } }, }, }; + const mockBody = { searches: [{ header: {}, body: {} }] }; + const mockQuery = {}; + const mockRequest = httpServerMock.createKibanaRequest({ + body: mockBody, + query: mockQuery, + }); + const mockResponse = httpServerMock.createResponseFactory(); + + registerMsearchRoute(mockCoreSetup.http.createRouter(), { getStartServices, globalConfig$ }); + + const mockRouter = mockCoreSetup.http.createRouter.mock.results[0].value; + const handler = mockRouter.post.mock.calls[0][1]; + await handler((mockContext as unknown) as RequestHandlerContext, mockRequest, mockResponse); + + expect(mockClient.msearch).toBeCalledTimes(1); + expect(mockResponse.customError).toBeCalled(); + + const error: any = mockResponse.customError.mock.calls[0][0]; + expect(error.statusCode).toBe(400); + expect(error.body.message).toBe('json_e_o_f_exception'); + expect(error.body.attributes).toBe(jsonEofException.error); + }); + + it('handler returns an error response if the search throws a general error', async () => { + const rejectedValue = Promise.reject(new Error('What happened?')); const mockClient = { - msearch: jest.fn().mockReturnValue(Promise.reject(response)), + msearch: jest.fn().mockReturnValue(rejectedValue), }; const mockContext = { core: { @@ -106,11 +145,12 @@ describe('msearch route', () => { const handler = mockRouter.post.mock.calls[0][1]; await handler((mockContext as unknown) as RequestHandlerContext, mockRequest, mockResponse); - expect(mockClient.msearch).toBeCalled(); + expect(mockClient.msearch).toBeCalledTimes(1); expect(mockResponse.customError).toBeCalled(); const error: any = mockResponse.customError.mock.calls[0][0]; - expect(error.body.message).toBe('oh no'); - expect(error.body.attributes.error).toBe('oops'); + expect(error.statusCode).toBe(500); + expect(error.body.message).toBe('What happened?'); + expect(error.body.attributes).toBe(undefined); }); }); diff --git a/src/plugins/data/server/search/routes/search.test.ts b/src/plugins/data/server/search/routes/search.test.ts index f47a42cf9d82b4..2cde6d19e4c187 100644 --- a/src/plugins/data/server/search/routes/search.test.ts +++ b/src/plugins/data/server/search/routes/search.test.ts @@ -12,11 +12,27 @@ import { CoreSetup, RequestHandlerContext } from 'src/core/server'; import { coreMock, httpServerMock } from '../../../../../../src/core/server/mocks'; import { registerSearchRoute } from './search'; import { DataPluginStart } from '../../plugin'; +import * as searchPhaseException from '../../../common/search/test_data/search_phase_execution_exception.json'; +import * as indexNotFoundException from '../../../common/search/test_data/index_not_found_exception.json'; +import { KbnServerError } from '../../../../kibana_utils/server'; describe('Search service', () => { let mockCoreSetup: MockedKeys>; + function mockEsError(message: string, statusCode: number, attributes?: Record) { + return new KbnServerError(message, statusCode, attributes); + } + + async function runMockSearch(mockContext: any, mockRequest: any, mockResponse: any) { + registerSearchRoute(mockCoreSetup.http.createRouter()); + + const mockRouter = mockCoreSetup.http.createRouter.mock.results[0].value; + const handler = mockRouter.post.mock.calls[0][1]; + await handler((mockContext as unknown) as RequestHandlerContext, mockRequest, mockResponse); + } + beforeEach(() => { + jest.clearAllMocks(); mockCoreSetup = coreMock.createSetup(); }); @@ -54,11 +70,7 @@ describe('Search service', () => { }); const mockResponse = httpServerMock.createResponseFactory(); - registerSearchRoute(mockCoreSetup.http.createRouter()); - - const mockRouter = mockCoreSetup.http.createRouter.mock.results[0].value; - const handler = mockRouter.post.mock.calls[0][1]; - await handler((mockContext as unknown) as RequestHandlerContext, mockRequest, mockResponse); + await runMockSearch(mockContext, mockRequest, mockResponse); expect(mockContext.search.search).toBeCalled(); expect(mockContext.search.search.mock.calls[0][0]).toStrictEqual(mockBody); @@ -68,14 +80,9 @@ describe('Search service', () => { }); }); - it('handler throws an error if the search throws an error', async () => { + it('handler returns an error response if the search throws a painless error', async () => { const rejectedValue = from( - Promise.reject({ - message: 'oh no', - body: { - error: 'oops', - }, - }) + Promise.reject(mockEsError('search_phase_execution_exception', 400, searchPhaseException)) ); const mockContext = { @@ -84,25 +91,69 @@ describe('Search service', () => { }, }; - const mockBody = { id: undefined, params: {} }; - const mockParams = { strategy: 'foo' }; const mockRequest = httpServerMock.createKibanaRequest({ - body: mockBody, - params: mockParams, + body: { id: undefined, params: {} }, + params: { strategy: 'foo' }, }); const mockResponse = httpServerMock.createResponseFactory(); - registerSearchRoute(mockCoreSetup.http.createRouter()); + await runMockSearch(mockContext, mockRequest, mockResponse); - const mockRouter = mockCoreSetup.http.createRouter.mock.results[0].value; - const handler = mockRouter.post.mock.calls[0][1]; - await handler((mockContext as unknown) as RequestHandlerContext, mockRequest, mockResponse); + // verify error + expect(mockResponse.customError).toBeCalled(); + const error: any = mockResponse.customError.mock.calls[0][0]; + expect(error.statusCode).toBe(400); + expect(error.body.message).toBe('search_phase_execution_exception'); + expect(error.body.attributes).toBe(searchPhaseException.error); + }); + + it('handler returns an error response if the search throws an index not found error', async () => { + const rejectedValue = from( + Promise.reject(mockEsError('index_not_found_exception', 404, indexNotFoundException)) + ); + + const mockContext = { + search: { + search: jest.fn().mockReturnValue(rejectedValue), + }, + }; + + const mockRequest = httpServerMock.createKibanaRequest({ + body: { id: undefined, params: {} }, + params: { strategy: 'foo' }, + }); + const mockResponse = httpServerMock.createResponseFactory(); + + await runMockSearch(mockContext, mockRequest, mockResponse); + + expect(mockResponse.customError).toBeCalled(); + const error: any = mockResponse.customError.mock.calls[0][0]; + expect(error.statusCode).toBe(404); + expect(error.body.message).toBe('index_not_found_exception'); + expect(error.body.attributes).toBe(indexNotFoundException.error); + }); + + it('handler returns an error response if the search throws a general error', async () => { + const rejectedValue = from(Promise.reject(new Error('This is odd'))); + + const mockContext = { + search: { + search: jest.fn().mockReturnValue(rejectedValue), + }, + }; + + const mockRequest = httpServerMock.createKibanaRequest({ + body: { id: undefined, params: {} }, + params: { strategy: 'foo' }, + }); + const mockResponse = httpServerMock.createResponseFactory(); + + await runMockSearch(mockContext, mockRequest, mockResponse); - expect(mockContext.search.search).toBeCalled(); - expect(mockContext.search.search.mock.calls[0][0]).toStrictEqual(mockBody); expect(mockResponse.customError).toBeCalled(); const error: any = mockResponse.customError.mock.calls[0][0]; - expect(error.body.message).toBe('oh no'); - expect(error.body.attributes.error).toBe('oops'); + expect(error.statusCode).toBe(500); + expect(error.body.message).toBe('This is odd'); + expect(error.body.attributes).toBe(undefined); }); }); diff --git a/src/plugins/data/server/search/search_service.ts b/src/plugins/data/server/search/search_service.ts index f1a6fc09ee21f4..63593bbe84a088 100644 --- a/src/plugins/data/server/search/search_service.ts +++ b/src/plugins/data/server/search/search_service.ts @@ -6,7 +6,7 @@ * Public License, v 1. */ -import { BehaviorSubject, Observable } from 'rxjs'; +import { BehaviorSubject, Observable, throwError } from 'rxjs'; import { pick } from 'lodash'; import { CoreSetup, @@ -18,7 +18,7 @@ import { SharedGlobalConfig, StartServicesAccessor, } from 'src/core/server'; -import { catchError, first, map } from 'rxjs/operators'; +import { first } from 'rxjs/operators'; import { BfetchServerSetup } from 'src/plugins/bfetch/server'; import { ExpressionsServerSetup } from 'src/plugins/expressions/server'; import type { @@ -64,6 +64,7 @@ import { aggShardDelay } from '../../common/search/aggs/buckets/shard_delay_fn'; import { ConfigSchema } from '../../config'; import { SessionService, IScopedSessionService, ISessionService } from './session'; import { KbnServerError } from '../../../kibana_utils/server'; +import { registerBsearchRoute } from './routes/bsearch'; type StrategyMap = Record>; @@ -137,43 +138,7 @@ export class SearchService implements Plugin { ) ); - bfetch.addBatchProcessingRoute< - { request: IKibanaSearchResponse; options?: ISearchOptions }, - any - >('/internal/bsearch', (request) => { - const search = this.asScopedProvider(this.coreStart!)(request); - - return { - onBatchItem: async ({ request: requestData, options }) => { - return search - .search(requestData, options) - .pipe( - first(), - map((response) => { - return { - ...response, - ...{ - rawResponse: shimHitsTotal(response.rawResponse), - }, - }; - }), - catchError((err) => { - // eslint-disable-next-line no-throw-literal - throw { - statusCode: err.statusCode || 500, - body: { - message: err.message, - attributes: { - error: err.body?.error || err.message, - }, - }, - }; - }) - ) - .toPromise(); - }, - }; - }); + registerBsearchRoute(bfetch, core.getStartServices(), this.asScopedProvider); core.savedObjects.registerType(searchTelemetry); if (usageCollection) { @@ -285,10 +250,14 @@ export class SearchService implements Plugin { options: ISearchOptions, deps: SearchStrategyDependencies ) => { - const strategy = this.getSearchStrategy( - options.strategy - ); - return session.search(strategy, request, options, deps); + try { + const strategy = this.getSearchStrategy( + options.strategy + ); + return session.search(strategy, request, options, deps); + } catch (e) { + return throwError(e); + } }; private cancel = (id: string, options: ISearchOptions, deps: SearchStrategyDependencies) => { diff --git a/src/plugins/data/tsconfig.json b/src/plugins/data/tsconfig.json index 81bcb3b02e100e..21560b13288407 100644 --- a/src/plugins/data/tsconfig.json +++ b/src/plugins/data/tsconfig.json @@ -7,7 +7,7 @@ "declaration": true, "declarationMap": true }, - "include": ["common/**/*", "public/**/*", "server/**/*", "config.ts"], + "include": ["common/**/*", "public/**/*", "server/**/*", "config.ts", "common/**/*.json"], "references": [ { "path": "../../core/tsconfig.json" }, { "path": "../bfetch/tsconfig.json" }, diff --git a/src/plugins/kibana_utils/common/errors/index.ts b/src/plugins/kibana_utils/common/errors/index.ts index 354cf1d504b287..f859e0728269a1 100644 --- a/src/plugins/kibana_utils/common/errors/index.ts +++ b/src/plugins/kibana_utils/common/errors/index.ts @@ -7,3 +7,4 @@ */ export * from './errors'; +export * from './types'; diff --git a/src/plugins/kibana_utils/common/errors/types.ts b/src/plugins/kibana_utils/common/errors/types.ts new file mode 100644 index 00000000000000..89e83586dc1157 --- /dev/null +++ b/src/plugins/kibana_utils/common/errors/types.ts @@ -0,0 +1,12 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * and the Server Side Public License, v 1; you may not use this file except in + * compliance with, at your election, the Elastic License or the Server Side + * Public License, v 1. + */ +export interface KibanaServerError { + statusCode: number; + message: string; + attributes?: T; +} diff --git a/src/plugins/kibana_utils/server/index.ts b/src/plugins/kibana_utils/server/index.ts index f95ffe5c3d7b6f..821118ea4640dd 100644 --- a/src/plugins/kibana_utils/server/index.ts +++ b/src/plugins/kibana_utils/server/index.ts @@ -18,4 +18,4 @@ export { url, } from '../common'; -export { KbnServerError, reportServerError } from './report_server_error'; +export { KbnServerError, reportServerError, getKbnServerError } from './report_server_error'; diff --git a/src/plugins/kibana_utils/server/report_server_error.ts b/src/plugins/kibana_utils/server/report_server_error.ts index 664f34ca7ad518..01e80cfc7184d3 100644 --- a/src/plugins/kibana_utils/server/report_server_error.ts +++ b/src/plugins/kibana_utils/server/report_server_error.ts @@ -6,23 +6,42 @@ * Public License, v 1. */ +import { ResponseError } from '@elastic/elasticsearch/lib/errors'; import { KibanaResponseFactory } from 'kibana/server'; import { KbnError } from '../common'; export class KbnServerError extends KbnError { - constructor(message: string, public readonly statusCode: number) { + public errBody?: Record; + constructor(message: string, public readonly statusCode: number, errBody?: Record) { super(message); + this.errBody = errBody; } } -export function reportServerError(res: KibanaResponseFactory, err: any) { +/** + * Formats any error thrown into a standardized `KbnServerError`. + * @param e `Error` or `ElasticsearchClientError` + * @returns `KbnServerError` + */ +export function getKbnServerError(e: Error) { + return new KbnServerError( + e.message ?? 'Unknown error', + e instanceof ResponseError ? e.statusCode : 500, + e instanceof ResponseError ? e.body : undefined + ); +} + +/** + * + * @param res Formats a `KbnServerError` into a server error response + * @param err + */ +export function reportServerError(res: KibanaResponseFactory, err: KbnServerError) { return res.customError({ statusCode: err.statusCode ?? 500, body: { message: err.message, - attributes: { - error: err.body?.error || err.message, - }, + attributes: err.errBody?.error, }, }); } diff --git a/test/api_integration/apis/search/bsearch.ts b/test/api_integration/apis/search/bsearch.ts new file mode 100644 index 00000000000000..504680d28bf83a --- /dev/null +++ b/test/api_integration/apis/search/bsearch.ts @@ -0,0 +1,172 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * and the Server Side Public License, v 1; you may not use this file except in + * compliance with, at your election, the Elastic License or the Server Side + * Public License, v 1. + */ + +import expect from '@kbn/expect'; +import request from 'superagent'; +import { FtrProviderContext } from '../../ftr_provider_context'; +import { painlessErrReq } from './painless_err_req'; +import { verifyErrorResponse } from './verify_error'; + +function parseBfetchResponse(resp: request.Response): Array> { + return resp.text + .trim() + .split('\n') + .map((item) => JSON.parse(item)); +} + +export default function ({ getService }: FtrProviderContext) { + const supertest = getService('supertest'); + const esArchiver = getService('esArchiver'); + + describe('bsearch', () => { + describe('post', () => { + it('should return 200 a single response', async () => { + const resp = await supertest.post(`/internal/bsearch`).send({ + batch: [ + { + request: { + params: { + body: { + query: { + match_all: {}, + }, + }, + }, + }, + }, + ], + }); + + const jsonBody = JSON.parse(resp.text); + + expect(resp.status).to.be(200); + expect(jsonBody.id).to.be(0); + expect(jsonBody.result.isPartial).to.be(false); + expect(jsonBody.result.isRunning).to.be(false); + expect(jsonBody.result).to.have.property('rawResponse'); + }); + + it('should return a batch of successful resposes', async () => { + const resp = await supertest.post(`/internal/bsearch`).send({ + batch: [ + { + request: { + params: { + body: { + query: { + match_all: {}, + }, + }, + }, + }, + }, + { + request: { + params: { + body: { + query: { + match_all: {}, + }, + }, + }, + }, + }, + ], + }); + + expect(resp.status).to.be(200); + const parsedResponse = parseBfetchResponse(resp); + expect(parsedResponse).to.have.length(2); + parsedResponse.forEach((responseJson) => { + expect(responseJson.result.isPartial).to.be(false); + expect(responseJson.result.isRunning).to.be(false); + expect(responseJson.result).to.have.property('rawResponse'); + }); + }); + + it('should return error for not found strategy', async () => { + const resp = await supertest.post(`/internal/bsearch`).send({ + batch: [ + { + request: { + params: { + body: { + query: { + match_all: {}, + }, + }, + }, + }, + options: { + strategy: 'wtf', + }, + }, + ], + }); + + expect(resp.status).to.be(200); + parseBfetchResponse(resp).forEach((responseJson, i) => { + expect(responseJson.id).to.be(i); + verifyErrorResponse(responseJson.error, 404, 'Search strategy wtf not found'); + }); + }); + + it('should return 400 when index type is provided in OSS', async () => { + const resp = await supertest.post(`/internal/bsearch`).send({ + batch: [ + { + request: { + indexType: 'baad', + params: { + body: { + query: { + match_all: {}, + }, + }, + }, + }, + }, + ], + }); + + expect(resp.status).to.be(200); + parseBfetchResponse(resp).forEach((responseJson, i) => { + expect(responseJson.id).to.be(i); + verifyErrorResponse(responseJson.error, 400, 'Unsupported index pattern type baad'); + }); + }); + + describe('painless', () => { + before(async () => { + await esArchiver.loadIfNeeded( + '../../../functional/fixtures/es_archiver/logstash_functional' + ); + }); + + after(async () => { + await esArchiver.unload('../../../functional/fixtures/es_archiver/logstash_functional'); + }); + it('should return 400 for Painless error', async () => { + const resp = await supertest.post(`/internal/bsearch`).send({ + batch: [ + { + request: painlessErrReq, + }, + ], + }); + + expect(resp.status).to.be(200); + parseBfetchResponse(resp).forEach((responseJson, i) => { + expect(responseJson.id).to.be(i); + verifyErrorResponse(responseJson.error, 400, 'search_phase_execution_exception', true); + }); + }); + }); + }); + }); +} diff --git a/test/api_integration/apis/search/index.ts b/test/api_integration/apis/search/index.ts index 2f21825d6902f8..6e90bf0f22c51f 100644 --- a/test/api_integration/apis/search/index.ts +++ b/test/api_integration/apis/search/index.ts @@ -11,6 +11,7 @@ import { FtrProviderContext } from '../../ftr_provider_context'; export default function ({ loadTestFile }: FtrProviderContext) { describe('search', () => { loadTestFile(require.resolve('./search')); + loadTestFile(require.resolve('./bsearch')); loadTestFile(require.resolve('./msearch')); }); } diff --git a/test/api_integration/apis/search/painless_err_req.ts b/test/api_integration/apis/search/painless_err_req.ts new file mode 100644 index 00000000000000..6fbf6565d7a9e1 --- /dev/null +++ b/test/api_integration/apis/search/painless_err_req.ts @@ -0,0 +1,44 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * and the Server Side Public License, v 1; you may not use this file except in + * compliance with, at your election, the Elastic License or the Server Side + * Public License, v 1. + */ + +export const painlessErrReq = { + params: { + index: 'log*', + body: { + size: 500, + fields: ['*'], + script_fields: { + invalid_scripted_field: { + script: { + source: 'invalid', + lang: 'painless', + }, + }, + }, + stored_fields: ['*'], + query: { + bool: { + filter: [ + { + match_all: {}, + }, + { + range: { + '@timestamp': { + gte: '2015-01-19T12:27:55.047Z', + lte: '2021-01-19T12:27:55.047Z', + format: 'strict_date_optional_time', + }, + }, + }, + ], + }, + }, + }, + }, +}; diff --git a/test/api_integration/apis/search/search.ts b/test/api_integration/apis/search/search.ts index fc13189a407537..155705f81fa8a9 100644 --- a/test/api_integration/apis/search/search.ts +++ b/test/api_integration/apis/search/search.ts @@ -8,11 +8,21 @@ import expect from '@kbn/expect'; import { FtrProviderContext } from '../../ftr_provider_context'; +import { painlessErrReq } from './painless_err_req'; +import { verifyErrorResponse } from './verify_error'; export default function ({ getService }: FtrProviderContext) { const supertest = getService('supertest'); + const esArchiver = getService('esArchiver'); describe('search', () => { + before(async () => { + await esArchiver.loadIfNeeded('../../../functional/fixtures/es_archiver/logstash_functional'); + }); + + after(async () => { + await esArchiver.unload('../../../functional/fixtures/es_archiver/logstash_functional'); + }); describe('post', () => { it('should return 200 when correctly formatted searches are provided', async () => { const resp = await supertest @@ -28,13 +38,37 @@ export default function ({ getService }: FtrProviderContext) { }) .expect(200); + expect(resp.status).to.be(200); expect(resp.body.isPartial).to.be(false); expect(resp.body.isRunning).to.be(false); expect(resp.body).to.have.property('rawResponse'); }); - it('should return 404 when if no strategy is provided', async () => - await supertest + it('should return 200 if terminated early', async () => { + const resp = await supertest + .post(`/internal/search/es`) + .send({ + params: { + terminateAfter: 1, + index: 'log*', + size: 1000, + body: { + query: { + match_all: {}, + }, + }, + }, + }) + .expect(200); + + expect(resp.status).to.be(200); + expect(resp.body.isPartial).to.be(false); + expect(resp.body.isRunning).to.be(false); + expect(resp.body.rawResponse.terminated_early).to.be(true); + }); + + it('should return 404 when if no strategy is provided', async () => { + const resp = await supertest .post(`/internal/search`) .send({ body: { @@ -43,7 +77,10 @@ export default function ({ getService }: FtrProviderContext) { }, }, }) - .expect(404)); + .expect(404); + + verifyErrorResponse(resp.body, 404); + }); it('should return 404 when if unknown strategy is provided', async () => { const resp = await supertest @@ -56,6 +93,8 @@ export default function ({ getService }: FtrProviderContext) { }, }) .expect(404); + + verifyErrorResponse(resp.body, 404); expect(resp.body.message).to.contain('banana not found'); }); @@ -74,11 +113,33 @@ export default function ({ getService }: FtrProviderContext) { }) .expect(400); + verifyErrorResponse(resp.body, 400); + expect(resp.body.message).to.contain('Unsupported index pattern'); }); + it('should return 400 with illegal ES argument', async () => { + const resp = await supertest + .post(`/internal/search/es`) + .send({ + params: { + timeout: 1, // This should be a time range string! + index: 'log*', + size: 1000, + body: { + query: { + match_all: {}, + }, + }, + }, + }) + .expect(400); + + verifyErrorResponse(resp.body, 400, 'illegal_argument_exception', true); + }); + it('should return 400 with a bad body', async () => { - await supertest + const resp = await supertest .post(`/internal/search/es`) .send({ params: { @@ -89,16 +150,26 @@ export default function ({ getService }: FtrProviderContext) { }, }) .expect(400); + + verifyErrorResponse(resp.body, 400, 'parsing_exception', true); + }); + + it('should return 400 for a painless error', async () => { + const resp = await supertest.post(`/internal/search/es`).send(painlessErrReq).expect(400); + + verifyErrorResponse(resp.body, 400, 'search_phase_execution_exception', true); }); }); describe('delete', () => { it('should return 404 when no search id provided', async () => { - await supertest.delete(`/internal/search/es`).send().expect(404); + const resp = await supertest.delete(`/internal/search/es`).send().expect(404); + verifyErrorResponse(resp.body, 404); }); it('should return 400 when trying a delete on a non supporting strategy', async () => { const resp = await supertest.delete(`/internal/search/es/123`).send().expect(400); + verifyErrorResponse(resp.body, 400); expect(resp.body.message).to.contain("Search strategy es doesn't support cancellations"); }); }); diff --git a/test/api_integration/apis/search/verify_error.ts b/test/api_integration/apis/search/verify_error.ts new file mode 100644 index 00000000000000..a5754ff47973ec --- /dev/null +++ b/test/api_integration/apis/search/verify_error.ts @@ -0,0 +1,27 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * and the Server Side Public License, v 1; you may not use this file except in + * compliance with, at your election, the Elastic License or the Server Side + * Public License, v 1. + */ + +import expect from '@kbn/expect'; + +export const verifyErrorResponse = ( + r: any, + expectedCode: number, + message?: string, + shouldHaveAttrs?: boolean +) => { + expect(r.statusCode).to.be(expectedCode); + if (message) { + expect(r.message).to.be(message); + } + if (shouldHaveAttrs) { + expect(r).to.have.property('attributes'); + expect(r.attributes).to.have.property('root_cause'); + } else { + expect(r).not.to.have.property('attributes'); + } +}; diff --git a/x-pack/plugins/data_enhanced/common/search/test_data/search_phase_execution_exception.json b/x-pack/plugins/data_enhanced/common/search/test_data/search_phase_execution_exception.json new file mode 100644 index 00000000000000..b79a396445e3d8 --- /dev/null +++ b/x-pack/plugins/data_enhanced/common/search/test_data/search_phase_execution_exception.json @@ -0,0 +1,229 @@ +{ + "error": { + "root_cause": [ + { + "type": "script_exception", + "reason": "compile error", + "script_stack": [ + "invalid", + "^---- HERE" + ], + "script": "invalid", + "lang": "painless", + "position": { + "offset": 0, + "start": 0, + "end": 7 + } + }, + { + "type": "script_exception", + "reason": "compile error", + "script_stack": [ + "invalid", + "^---- HERE" + ], + "script": "invalid", + "lang": "painless", + "position": { + "offset": 0, + "start": 0, + "end": 7 + } + }, + { + "type": "parse_exception", + "reason": "failed to parse date field [2021-01-19T12:2755.047Z] with format [strict_date_optional_time]: [failed to parse date field [2021-01-19T12:2755.047Z] with format [strict_date_optional_time]]" + }, + { + "type": "script_exception", + "reason": "compile error", + "script_stack": [ + "invalid", + "^---- HERE" + ], + "script": "invalid", + "lang": "painless", + "position": { + "offset": 0, + "start": 0, + "end": 7 + } + }, + { + "type": "script_exception", + "reason": "compile error", + "script_stack": [ + "invalid", + "^---- HERE" + ], + "script": "invalid", + "lang": "painless", + "position": { + "offset": 0, + "start": 0, + "end": 7 + } + }, + { + "type": "script_exception", + "reason": "compile error", + "script_stack": [ + "invalid", + "^---- HERE" + ], + "script": "invalid", + "lang": "painless", + "position": { + "offset": 0, + "start": 0, + "end": 7 + } + } + ], + "type": "search_phase_execution_exception", + "reason": "all shards failed", + "phase": "query", + "grouped": true, + "failed_shards": [ + { + "shard": 0, + "index": ".apm-agent-configuration", + "node": "DEfMVCg5R12TRG4CYIxUgQ", + "reason": { + "type": "script_exception", + "reason": "compile error", + "script_stack": [ + "invalid", + "^---- HERE" + ], + "script": "invalid", + "lang": "painless", + "position": { + "offset": 0, + "start": 0, + "end": 7 + }, + "caused_by": { + "type": "illegal_argument_exception", + "reason": "cannot resolve symbol [invalid]" + } + } + }, + { + "shard": 0, + "index": ".apm-custom-link", + "node": "DEfMVCg5R12TRG4CYIxUgQ", + "reason": { + "type": "script_exception", + "reason": "compile error", + "script_stack": [ + "invalid", + "^---- HERE" + ], + "script": "invalid", + "lang": "painless", + "position": { + "offset": 0, + "start": 0, + "end": 7 + }, + "caused_by": { + "type": "illegal_argument_exception", + "reason": "cannot resolve symbol [invalid]" + } + } + }, + { + "shard": 0, + "index": ".kibana-event-log-8.0.0-000001", + "node": "DEfMVCg5R12TRG4CYIxUgQ", + "reason": { + "type": "parse_exception", + "reason": "failed to parse date field [2021-01-19T12:2755.047Z] with format [strict_date_optional_time]: [failed to parse date field [2021-01-19T12:2755.047Z] with format [strict_date_optional_time]]", + "caused_by": { + "type": "illegal_argument_exception", + "reason": "failed to parse date field [2021-01-19T12:2755.047Z] with format [strict_date_optional_time]", + "caused_by": { + "type": "date_time_parse_exception", + "reason": "Text '2021-01-19T12:2755.047Z' could not be parsed, unparsed text found at index 16" + } + } + } + }, + { + "shard": 0, + "index": ".kibana_1", + "node": "DEfMVCg5R12TRG4CYIxUgQ", + "reason": { + "type": "script_exception", + "reason": "compile error", + "script_stack": [ + "invalid", + "^---- HERE" + ], + "script": "invalid", + "lang": "painless", + "position": { + "offset": 0, + "start": 0, + "end": 7 + }, + "caused_by": { + "type": "illegal_argument_exception", + "reason": "cannot resolve symbol [invalid]" + } + } + }, + { + "shard": 0, + "index": ".kibana_task_manager_1", + "node": "DEfMVCg5R12TRG4CYIxUgQ", + "reason": { + "type": "script_exception", + "reason": "compile error", + "script_stack": [ + "invalid", + "^---- HERE" + ], + "script": "invalid", + "lang": "painless", + "position": { + "offset": 0, + "start": 0, + "end": 7 + }, + "caused_by": { + "type": "illegal_argument_exception", + "reason": "cannot resolve symbol [invalid]" + } + } + }, + { + "shard": 0, + "index": ".security-7", + "node": "DEfMVCg5R12TRG4CYIxUgQ", + "reason": { + "type": "script_exception", + "reason": "compile error", + "script_stack": [ + "invalid", + "^---- HERE" + ], + "script": "invalid", + "lang": "painless", + "position": { + "offset": 0, + "start": 0, + "end": 7 + }, + "caused_by": { + "type": "illegal_argument_exception", + "reason": "cannot resolve symbol [invalid]" + } + } + } + ] + }, + "status": 400 +} \ No newline at end of file diff --git a/x-pack/plugins/data_enhanced/public/search/search_interceptor.test.ts b/x-pack/plugins/data_enhanced/public/search/search_interceptor.test.ts index 1a6fc724e2cf21..22b0f3272ff7d8 100644 --- a/x-pack/plugins/data_enhanced/public/search/search_interceptor.test.ts +++ b/x-pack/plugins/data_enhanced/public/search/search_interceptor.test.ts @@ -9,10 +9,16 @@ import { EnhancedSearchInterceptor } from './search_interceptor'; import { CoreSetup, CoreStart } from 'kibana/public'; import { UI_SETTINGS } from '../../../../../src/plugins/data/common'; import { AbortError } from '../../../../../src/plugins/kibana_utils/public'; -import { ISessionService, SearchTimeoutError, SearchSessionState } from 'src/plugins/data/public'; +import { + ISessionService, + SearchTimeoutError, + SearchSessionState, + PainlessError, +} from 'src/plugins/data/public'; import { dataPluginMock } from '../../../../../src/plugins/data/public/mocks'; import { bfetchPluginMock } from '../../../../../src/plugins/bfetch/public/mocks'; import { BehaviorSubject } from 'rxjs'; +import * as xpackResourceNotFoundException from '../../common/search/test_data/search_phase_execution_exception.json'; const timeTravel = (msToRun = 0) => { jest.advanceTimersByTime(msToRun); @@ -99,6 +105,33 @@ describe('EnhancedSearchInterceptor', () => { }); }); + describe('errors', () => { + test('Should throw Painless error on server error with OSS format', async () => { + const mockResponse: any = { + statusCode: 400, + message: 'search_phase_execution_exception', + attributes: xpackResourceNotFoundException.error, + }; + fetchMock.mockRejectedValueOnce(mockResponse); + const response = searchInterceptor.search({ + params: {}, + }); + await expect(response.toPromise()).rejects.toThrow(PainlessError); + }); + + test('Renders a PainlessError', async () => { + searchInterceptor.showError( + new PainlessError({ + statusCode: 400, + message: 'search_phase_execution_exception', + attributes: xpackResourceNotFoundException.error, + }) + ); + expect(mockCoreSetup.notifications.toasts.addDanger).toBeCalledTimes(1); + expect(mockCoreSetup.notifications.toasts.addError).not.toBeCalled(); + }); + }); + describe('search', () => { test('should resolve immediately if first call returns full result', async () => { const responses = [ @@ -342,7 +375,8 @@ describe('EnhancedSearchInterceptor', () => { { time: 10, value: { - error: 'oh no', + statusCode: 500, + message: 'oh no', id: 1, }, isError: true, @@ -364,7 +398,8 @@ describe('EnhancedSearchInterceptor', () => { await timeTravel(10); expect(error).toHaveBeenCalled(); - expect(error.mock.calls[0][0]).toBe(responses[1].value); + expect(error.mock.calls[0][0]).toBeInstanceOf(Error); + expect((error.mock.calls[0][0] as Error).message).toBe('oh no'); expect(fetchMock).toHaveBeenCalledTimes(2); expect(mockCoreSetup.http.delete).toHaveBeenCalledTimes(1); }); diff --git a/x-pack/plugins/data_enhanced/server/search/es_search_strategy.test.ts b/x-pack/plugins/data_enhanced/server/search/es_search_strategy.test.ts index 3230895da77059..b2ddd0310f8f59 100644 --- a/x-pack/plugins/data_enhanced/server/search/es_search_strategy.test.ts +++ b/x-pack/plugins/data_enhanced/server/search/es_search_strategy.test.ts @@ -7,6 +7,10 @@ import { enhancedEsSearchStrategyProvider } from './es_search_strategy'; import { BehaviorSubject } from 'rxjs'; import { SearchStrategyDependencies } from '../../../../../src/plugins/data/server/search'; +import { KbnServerError } from '../../../../../src/plugins/kibana_utils/server'; +import { ElasticsearchClientError, ResponseError } from '@elastic/elasticsearch/lib/errors'; +import * as indexNotFoundException from '../../../../../src/plugins/data/common/search/test_data/index_not_found_exception.json'; +import * as xContentParseException from '../../../../../src/plugins/data/common/search/test_data/x_content_parse_exception.json'; const mockAsyncResponse = { body: { @@ -145,6 +149,54 @@ describe('ES search strategy', () => { expect(request).toHaveProperty('wait_for_completion_timeout'); expect(request).toHaveProperty('keep_alive'); }); + + it('throws normalized error if ResponseError is thrown', async () => { + const errResponse = new ResponseError({ + body: indexNotFoundException, + statusCode: 404, + headers: {}, + warnings: [], + meta: {} as any, + }); + + mockSubmitCaller.mockRejectedValue(errResponse); + + const params = { index: 'logstash-*', body: { query: {} } }; + const esSearch = await enhancedEsSearchStrategyProvider(mockConfig$, mockLogger); + + let err: KbnServerError | undefined; + try { + await esSearch.search({ params }, {}, mockDeps).toPromise(); + } catch (e) { + err = e; + } + expect(mockSubmitCaller).toBeCalled(); + expect(err).toBeInstanceOf(KbnServerError); + expect(err?.statusCode).toBe(404); + expect(err?.message).toBe(errResponse.message); + expect(err?.errBody).toBe(indexNotFoundException); + }); + + it('throws normalized error if Error is thrown', async () => { + const errResponse = new Error('not good'); + + mockSubmitCaller.mockRejectedValue(errResponse); + + const params = { index: 'logstash-*', body: { query: {} } }; + const esSearch = await enhancedEsSearchStrategyProvider(mockConfig$, mockLogger); + + let err: KbnServerError | undefined; + try { + await esSearch.search({ params }, {}, mockDeps).toPromise(); + } catch (e) { + err = e; + } + expect(mockSubmitCaller).toBeCalled(); + expect(err).toBeInstanceOf(KbnServerError); + expect(err?.statusCode).toBe(500); + expect(err?.message).toBe(errResponse.message); + expect(err?.errBody).toBe(undefined); + }); }); describe('cancel', () => { @@ -160,6 +212,33 @@ describe('ES search strategy', () => { const request = mockDeleteCaller.mock.calls[0][0]; expect(request).toEqual({ id }); }); + + it('throws normalized error on ResponseError', async () => { + const errResponse = new ResponseError({ + body: xContentParseException, + statusCode: 400, + headers: {}, + warnings: [], + meta: {} as any, + }); + mockDeleteCaller.mockRejectedValue(errResponse); + + const id = 'some_id'; + const esSearch = await enhancedEsSearchStrategyProvider(mockConfig$, mockLogger); + + let err: KbnServerError | undefined; + try { + await esSearch.cancel!(id, {}, mockDeps); + } catch (e) { + err = e; + } + + expect(mockDeleteCaller).toBeCalled(); + expect(err).toBeInstanceOf(KbnServerError); + expect(err?.statusCode).toBe(400); + expect(err?.message).toBe(errResponse.message); + expect(err?.errBody).toBe(xContentParseException); + }); }); describe('extend', () => { @@ -176,5 +255,27 @@ describe('ES search strategy', () => { const request = mockGetCaller.mock.calls[0][0]; expect(request).toEqual({ id, keep_alive: keepAlive }); }); + + it('throws normalized error on ElasticsearchClientError', async () => { + const errResponse = new ElasticsearchClientError('something is wrong with EsClient'); + mockGetCaller.mockRejectedValue(errResponse); + + const id = 'some_other_id'; + const keepAlive = '1d'; + const esSearch = await enhancedEsSearchStrategyProvider(mockConfig$, mockLogger); + + let err: KbnServerError | undefined; + try { + await esSearch.extend!(id, keepAlive, {}, mockDeps); + } catch (e) { + err = e; + } + + expect(mockGetCaller).toBeCalled(); + expect(err).toBeInstanceOf(KbnServerError); + expect(err?.statusCode).toBe(500); + expect(err?.message).toBe(errResponse.message); + expect(err?.errBody).toBe(undefined); + }); }); }); diff --git a/x-pack/plugins/data_enhanced/server/search/es_search_strategy.ts b/x-pack/plugins/data_enhanced/server/search/es_search_strategy.ts index 54ed59b30952af..694d9807b5a4d3 100644 --- a/x-pack/plugins/data_enhanced/server/search/es_search_strategy.ts +++ b/x-pack/plugins/data_enhanced/server/search/es_search_strategy.ts @@ -6,7 +6,7 @@ import type { Observable } from 'rxjs'; import type { IScopedClusterClient, Logger, SharedGlobalConfig } from 'kibana/server'; -import { first, tap } from 'rxjs/operators'; +import { catchError, first, tap } from 'rxjs/operators'; import { SearchResponse } from 'elasticsearch'; import { from } from 'rxjs'; import type { @@ -33,7 +33,7 @@ import { } from './request_utils'; import { toAsyncKibanaSearchResponse } from './response_utils'; import { AsyncSearchResponse } from './types'; -import { KbnServerError } from '../../../../../src/plugins/kibana_utils/server'; +import { getKbnServerError, KbnServerError } from '../../../../../src/plugins/kibana_utils/server'; export const enhancedEsSearchStrategyProvider = ( config$: Observable, @@ -41,7 +41,11 @@ export const enhancedEsSearchStrategyProvider = ( usage?: SearchUsage ): ISearchStrategy => { async function cancelAsyncSearch(id: string, esClient: IScopedClusterClient) { - await esClient.asCurrentUser.asyncSearch.delete({ id }); + try { + await esClient.asCurrentUser.asyncSearch.delete({ id }); + } catch (e) { + throw getKbnServerError(e); + } } function asyncSearch( @@ -70,7 +74,10 @@ export const enhancedEsSearchStrategyProvider = ( return pollSearch(search, cancel, options).pipe( tap((response) => (id = response.id)), - tap(searchUsageObserver(logger, usage)) + tap(searchUsageObserver(logger, usage)), + catchError((e) => { + throw getKbnServerError(e); + }) ); } @@ -90,40 +97,72 @@ export const enhancedEsSearchStrategyProvider = ( ...params, }; - const promise = esClient.asCurrentUser.transport.request({ - method, - path, - body, - querystring, - }); + try { + const promise = esClient.asCurrentUser.transport.request({ + method, + path, + body, + querystring, + }); - const esResponse = await shimAbortSignal(promise, options?.abortSignal); - const response = esResponse.body as SearchResponse; - return { - rawResponse: response, - ...getTotalLoaded(response), - }; + const esResponse = await shimAbortSignal(promise, options?.abortSignal); + const response = esResponse.body as SearchResponse; + return { + rawResponse: response, + ...getTotalLoaded(response), + }; + } catch (e) { + throw getKbnServerError(e); + } } return { + /** + * @param request + * @param options + * @param deps `SearchStrategyDependencies` + * @returns `Observable>` + * @throws `KbnServerError` + */ search: (request, options: IAsyncSearchOptions, deps) => { logger.debug(`search ${JSON.stringify(request.params) || request.id}`); + if (request.indexType && request.indexType !== 'rollup') { + throw new KbnServerError('Unknown indexType', 400); + } if (request.indexType === undefined) { return asyncSearch(request, options, deps); - } else if (request.indexType === 'rollup') { - return from(rollupSearch(request, options, deps)); } else { - throw new KbnServerError('Unknown indexType', 400); + return from(rollupSearch(request, options, deps)); } }, + /** + * @param id async search ID to cancel, as returned from _async_search API + * @param options + * @param deps `SearchStrategyDependencies` + * @returns `Promise` + * @throws `KbnServerError` + */ cancel: async (id, options, { esClient }) => { logger.debug(`cancel ${id}`); await cancelAsyncSearch(id, esClient); }, + /** + * + * @param id async search ID to extend, as returned from _async_search API + * @param keepAlive + * @param options + * @param deps `SearchStrategyDependencies` + * @returns `Promise` + * @throws `KbnServerError` + */ extend: async (id, keepAlive, options, { esClient }) => { logger.debug(`extend ${id} by ${keepAlive}`); - await esClient.asCurrentUser.asyncSearch.get({ id, keep_alive: keepAlive }); + try { + await esClient.asCurrentUser.asyncSearch.get({ id, keep_alive: keepAlive }); + } catch (e) { + throw getKbnServerError(e); + } }, }; }; diff --git a/x-pack/plugins/data_enhanced/tsconfig.json b/x-pack/plugins/data_enhanced/tsconfig.json index c4b09276880d99..29bfd71cb32b40 100644 --- a/x-pack/plugins/data_enhanced/tsconfig.json +++ b/x-pack/plugins/data_enhanced/tsconfig.json @@ -14,7 +14,8 @@ "config.ts", "../../../typings/**/*", // have to declare *.json explicitly due to https://github.com/microsoft/TypeScript/issues/25636 - "public/autocomplete/providers/kql_query_suggestion/__fixtures__/*.json" + "public/autocomplete/providers/kql_query_suggestion/__fixtures__/*.json", + "common/search/test_data/*.json" ], "references": [ { "path": "../../../src/core/tsconfig.json" }, diff --git a/x-pack/test/api_integration/apis/search/search.ts b/x-pack/test/api_integration/apis/search/search.ts index 0c08b834a27783..2115976bcced1a 100644 --- a/x-pack/test/api_integration/apis/search/search.ts +++ b/x-pack/test/api_integration/apis/search/search.ts @@ -6,6 +6,7 @@ import expect from '@kbn/expect'; import { FtrProviderContext } from '../../ftr_provider_context'; +import { verifyErrorResponse } from '../../../../../test/api_integration/apis/search/verify_error'; export default function ({ getService }: FtrProviderContext) { const supertest = getService('supertest'); @@ -90,6 +91,23 @@ export default function ({ getService }: FtrProviderContext) { expect(resp2.body.isRunning).to.be(false); }); + it('should fail without kbn-xref header', async () => { + const resp = await supertest + .post(`/internal/search/ese`) + .send({ + params: { + body: { + query: { + match_all: {}, + }, + }, + }, + }) + .expect(400); + + verifyErrorResponse(resp.body, 400, 'Request must contain a kbn-xsrf header.'); + }); + it('should return 400 when unknown index type is provided', async () => { const resp = await supertest .post(`/internal/search/ese`) @@ -106,7 +124,7 @@ export default function ({ getService }: FtrProviderContext) { }) .expect(400); - expect(resp.body.message).to.contain('Unknown indexType'); + verifyErrorResponse(resp.body, 400, 'Unknown indexType'); }); it('should return 400 if invalid id is provided', async () => { @@ -124,7 +142,7 @@ export default function ({ getService }: FtrProviderContext) { }) .expect(400); - expect(resp.body.message).to.contain('illegal_argument_exception'); + verifyErrorResponse(resp.body, 400, 'illegal_argument_exception', true); }); it('should return 404 if unkown id is provided', async () => { @@ -143,12 +161,11 @@ export default function ({ getService }: FtrProviderContext) { }, }) .expect(404); - - expect(resp.body.message).to.contain('resource_not_found_exception'); + verifyErrorResponse(resp.body, 404, 'resource_not_found_exception', true); }); it('should return 400 with a bad body', async () => { - await supertest + const resp = await supertest .post(`/internal/search/ese`) .set('kbn-xsrf', 'foo') .send({ @@ -160,6 +177,8 @@ export default function ({ getService }: FtrProviderContext) { }, }) .expect(400); + + verifyErrorResponse(resp.body, 400, 'parsing_exception', true); }); }); @@ -186,8 +205,7 @@ export default function ({ getService }: FtrProviderContext) { }, }) .expect(400); - - expect(resp.body.message).to.contain('illegal_argument_exception'); + verifyErrorResponse(resp.body, 400, 'illegal_argument_exception', true); }); it('should return 400 if rollup search is without non-existent index', async () => { @@ -207,7 +225,7 @@ export default function ({ getService }: FtrProviderContext) { }) .expect(400); - expect(resp.body.message).to.contain('illegal_argument_exception'); + verifyErrorResponse(resp.body, 400, 'illegal_argument_exception', true); }); it('should rollup search', async () => { @@ -241,7 +259,7 @@ export default function ({ getService }: FtrProviderContext) { .set('kbn-xsrf', 'foo') .send() .expect(400); - expect(resp.body.message).to.contain('illegal_argument_exception'); + verifyErrorResponse(resp.body, 400, 'illegal_argument_exception', true); }); it('should delete a search', async () => {