Skip to content

Commit

Permalink
[Search Sessions] Improve search session errors (#88613)
Browse files Browse the repository at this point in the history
* Detect ESError correctly
Fix bfetch error (was recognized as unknown error)
Make sure handleSearchError always returns an error object.

* fix tests and improve types

* type

* normalize search error response format for search and bsearch

* type

* Added es search exception examples

* Normalize and validate errors thrown from oss es_search_strategy
Validate abort

* Added tests for search service error handling

* Update msearch tests to test for errors

* Moved bsearch route to routes folder
Adjusted bsearch response format
Added verification of error's root cause

* Align painless error object

* eslint

* Add to seach interceptor tests

* add json to tsconfig

* docs

* updated xpack search strategy tests

* oops

* license header

* Add test for xpack painless error format

* doc

* Fix bsearch test potential flakiness

* code review

* fix

* code review 2

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
lizozom and kibanamachine authored Jan 31, 2021
1 parent 52f5403 commit 841ab70
Show file tree
Hide file tree
Showing 42 changed files with 1,499 additions and 295 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,14 @@
<b>Signature:</b>

```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 | <code>any</code> | |
| e | <code>KibanaServerError &#124; AbortError</code> | |
| timeoutSignal | <code>AbortSignal</code> | |
| options | <code>ISearchOptions</code> | |

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,13 @@ Constructs a new instance of the `SearchTimeoutError` class
<b>Signature:</b>

```typescript
constructor(err: Error, mode: TimeoutErrorMode);
constructor(err: Record<string, any>, mode: TimeoutErrorMode);
```

## Parameters

| Parameter | Type | Description |
| --- | --- | --- |
| err | <code>Error</code> | |
| err | <code>Record&lt;string, any&gt;</code> | |
| mode | <code>TimeoutErrorMode</code> | |

Original file line number Diff line number Diff line change
@@ -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
}

Original file line number Diff line number Diff line change
@@ -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
}
14 changes: 14 additions & 0 deletions src/plugins/data/common/search/test_data/json_e_o_f_exception.json
Original file line number Diff line number Diff line change
@@ -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
}

17 changes: 17 additions & 0 deletions src/plugins/data/common/search/test_data/parsing_exception.json
Original file line number Diff line number Diff line change
@@ -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
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"error" : {
"root_cause" : [
{
"type" : "resource_not_found_exception",
"reason" : "FlZlSXp6dkd3UXdHZjhsalVtVHBnYkEdYjNIWDhDOTZRN3ExemdmVkx4RXNQQToxMjc2ODk="
}
],
"type" : "resource_not_found_exception",
"reason" : "FlZlSXp6dkd3UXdHZjhsalVtVHBnYkEdYjNIWDhDOTZRN3ExemdmVkx4RXNQQToxMjc2ODk="
},
"status" : 404
}
Original file line number Diff line number Diff line change
@@ -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
}
Original file line number Diff line number Diff line change
@@ -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
}
7 changes: 5 additions & 2 deletions src/plugins/data/public/public.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -2282,8 +2282,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<number>;
// @internal (undocumented)
Expand Down Expand Up @@ -2453,7 +2456,7 @@ export interface SearchSourceFields {
//
// @public
export class SearchTimeoutError extends KbnError {
constructor(err: Error, mode: TimeoutErrorMode);
constructor(err: Record<string, any>, mode: TimeoutErrorMode);
// (undocumented)
getErrorMessage(application: ApplicationStart): JSX.Element;
// (undocumented)
Expand Down
19 changes: 9 additions & 10 deletions src/plugins/data/public/search/errors/es_error.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
8 changes: 4 additions & 4 deletions src/plugins/data/public/search/errors/es_error.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down
42 changes: 42 additions & 0 deletions src/plugins/data/public/search/errors/painless_error.test.tsx
Original file line number Diff line number Diff line change
@@ -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);
});
});
10 changes: 6 additions & 4 deletions src/plugins/data/public/search/errors/painless_error.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
})}
<EuiText data-test-subj="painlessScript">
{i18n.translate('data.painlessError.painlessScriptedFieldErrorMessage', {
defaultMessage: "Error executing Painless script: '{script}'",
values: { script: rootCause?.script },
})}
</EuiText>
<EuiSpacer size="s" />
<EuiSpacer size="s" />
{painlessStack ? (
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/data/public/search/errors/timeout_error.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export enum TimeoutErrorMode {
*/
export class SearchTimeoutError extends KbnError {
public mode: TimeoutErrorMode;
constructor(err: Error, mode: TimeoutErrorMode) {
constructor(err: Record<string, any>, mode: TimeoutErrorMode) {
super(`Request timeout: ${JSON.stringify(err?.message)}`);
this.mode = mode;
}
Expand Down
Loading

0 comments on commit 841ab70

Please sign in to comment.