Skip to content

Commit

Permalink
[Search] Error Alignment 2 (#80965)
Browse files Browse the repository at this point in the history
* Improve the display of ES errors and Http errors

* fixes

* Improve text

* fix ts

* update limit

Co-authored-by: Kibana Machine <[email protected]>
  • Loading branch information
lizozom and kibanamachine authored Oct 26, 2020
1 parent a31027f commit 046c840
Show file tree
Hide file tree
Showing 11 changed files with 180 additions and 55 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,13 @@ Constructs a new instance of the `PainlessError` class
<b>Signature:</b>

```typescript
constructor(err: EsError, request: IKibanaSearchRequest);
constructor(err: IEsError, request: IKibanaSearchRequest);
```

## Parameters

| Parameter | Type | Description |
| --- | --- | --- |
| err | <code>EsError</code> | |
| err | <code>IEsError</code> | |
| request | <code>IKibanaSearchRequest</code> | |

Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
<b>Signature:</b>

```typescript
export declare class PainlessError extends KbnError
export declare class PainlessError extends EsError
```
## Constructors
Expand Down
2 changes: 1 addition & 1 deletion packages/kbn-optimizer/limits.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ pageLoadAssetSize:
dashboard: 374194
dashboardEnhanced: 65646
dashboardMode: 22716
data: 1170713
data: 1287839
dataEnhanced: 50420
devTools: 38637
discover: 105145
Expand Down
9 changes: 5 additions & 4 deletions src/plugins/data/public/public.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -1598,13 +1598,13 @@ export interface OptionedValueProp {
value: string;
}

// Warning: (ae-forgotten-export) The symbol "KbnError" needs to be exported by the entry point index.d.ts
// Warning: (ae-forgotten-export) The symbol "EsError" needs to be exported by the entry point index.d.ts
// Warning: (ae-missing-release-tag) "PainlessError" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal)
//
// @public (undocumented)
export class PainlessError extends KbnError {
// Warning: (ae-forgotten-export) The symbol "EsError" needs to be exported by the entry point index.d.ts
constructor(err: EsError, request: IKibanaSearchRequest);
export class PainlessError extends EsError {
// Warning: (ae-forgotten-export) The symbol "IEsError" needs to be exported by the entry point index.d.ts
constructor(err: IEsError, request: IKibanaSearchRequest);
// (undocumented)
getErrorMessage(application: ApplicationStart): JSX.Element;
// (undocumented)
Expand Down Expand Up @@ -2134,6 +2134,7 @@ export interface SearchSourceFields {
version?: boolean;
}

// Warning: (ae-forgotten-export) The symbol "KbnError" needs to be exported by the entry point index.d.ts
// Warning: (ae-missing-release-tag) "SearchTimeoutError" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal)
//
// @public
Expand Down
46 changes: 46 additions & 0 deletions src/plugins/data/public/search/errors/es_error.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import React from 'react';
import { EuiCodeBlock, EuiSpacer } from '@elastic/eui';
import { ApplicationStart } from 'kibana/public';
import { KbnError } from '../../../../kibana_utils/common';
import { IEsError } from './types';
import { getRootCause } from './utils';

export class EsError extends KbnError {
constructor(protected readonly err: IEsError) {
super('EsError');
}

public getErrorMessage(application: ApplicationStart) {
const rootCause = getRootCause(this.err)?.reason;

return (
<>
<EuiSpacer size="s" />
{rootCause ? (
<EuiCodeBlock data-test-subj="errMessage" isCopyable={true} paddingSize="s">
{rootCause}
</EuiCodeBlock>
) : null}
</>
);
}
}
38 changes: 38 additions & 0 deletions src/plugins/data/public/search/errors/http_error.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import { EuiCodeBlock, EuiSpacer } from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import React from 'react';

export function getHttpError(message: string) {
return (
<>
{i18n.translate('data.errors.fetchError', {
defaultMessage:
'Check your network and proxy configuration. If the problem persists, contact your network administrator.',
})}
<EuiSpacer size="s" />
<EuiSpacer size="s" />
<EuiCodeBlock data-test-subj="errMessage" isCopyable={true} paddingSize="s">
{message}
</EuiCodeBlock>
</>
);
}
4 changes: 4 additions & 0 deletions src/plugins/data/public/search/errors/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,9 @@
* under the License.
*/

export * from './es_error';
export * from './painless_error';
export * from './timeout_error';
export * from './utils';
export * from './types';
export * from './http_error';
46 changes: 17 additions & 29 deletions src/plugins/data/public/search/errors/painless_error.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,22 +22,15 @@ import { i18n } from '@kbn/i18n';
import { EuiButton, EuiSpacer, EuiText, EuiCodeBlock } from '@elastic/eui';
import { FormattedMessage } from '@kbn/i18n/react';
import { ApplicationStart } from 'kibana/public';
import { KbnError } from '../../../../kibana_utils/common';
import { EsError, isEsError } from './types';
import { IEsError, isEsError } from './types';
import { EsError } from './es_error';
import { getRootCause } from './utils';
import { IKibanaSearchRequest } from '..';

export class PainlessError extends KbnError {
export class PainlessError extends EsError {
painlessStack?: string;
constructor(err: EsError, request: IKibanaSearchRequest) {
const rootCause = getRootCause(err as EsError);

super(
i18n.translate('data.painlessError.painlessScriptedFieldErrorMessage', {
defaultMessage: "Error executing Painless script: '{script}'.",
values: { script: rootCause?.script },
})
);
this.painlessStack = rootCause?.script_stack ? rootCause?.script_stack.join('\n') : undefined;
constructor(err: IEsError, request: IKibanaSearchRequest) {
super(err);
}

public getErrorMessage(application: ApplicationStart) {
Expand All @@ -47,14 +40,20 @@ export class PainlessError extends KbnError {
});
}

const rootCause = getRootCause(this.err);
const painlessStack = rootCause?.script_stack ? rootCause?.script_stack.join('\n') : undefined;

return (
<>
{this.message}
{i18n.translate('data.painlessError.painlessScriptedFieldErrorMessage', {
defaultMessage: "Error executing Painless script: '{script}'.",
values: { script: rootCause?.script },
})}
<EuiSpacer size="s" />
<EuiSpacer size="s" />
{this.painlessStack ? (
{painlessStack ? (
<EuiCodeBlock data-test-subj="painlessStackTrace" isCopyable={true} paddingSize="s">
{this.painlessStack}
{painlessStack}
</EuiCodeBlock>
) : null}
<EuiText textAlign="right">
Expand All @@ -67,21 +66,10 @@ export class PainlessError extends KbnError {
}
}

function getFailedShards(err: EsError) {
const failedShards =
err.body?.attributes?.error?.failed_shards ||
err.body?.attributes?.error?.caused_by?.failed_shards;
return failedShards ? failedShards[0] : undefined;
}

function getRootCause(err: EsError) {
return getFailedShards(err)?.reason;
}

export function isPainlessError(err: Error | EsError) {
export function isPainlessError(err: Error | IEsError) {
if (!isEsError(err)) return false;

const rootCause = getRootCause(err as EsError);
const rootCause = getRootCause(err as IEsError);
if (!rootCause) return false;

const { lang } = rootCause;
Expand Down
6 changes: 3 additions & 3 deletions src/plugins/data/public/search/errors/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
* under the License.
*/

interface FailedShard {
export interface FailedShard {
shard: number;
index: string;
node: string;
Expand All @@ -39,7 +39,7 @@ interface FailedShard {
};
}

export interface EsError {
export interface IEsError {
body: {
statusCode: number;
error: string;
Expand Down Expand Up @@ -68,6 +68,6 @@ export interface EsError {
};
}

export function isEsError(e: any): e is EsError {
export function isEsError(e: any): e is IEsError {
return !!e.body?.attributes;
}
31 changes: 31 additions & 0 deletions src/plugins/data/public/search/errors/utils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import { IEsError } from './types';

export function getFailedShards(err: IEsError) {
const failedShards =
err.body?.attributes?.error?.failed_shards ||
err.body?.attributes?.error?.caused_by?.failed_shards;
return failedShards ? failedShards[0] : undefined;
}

export function getRootCause(err: IEsError) {
return getFailedShards(err)?.reason;
}
47 changes: 32 additions & 15 deletions src/plugins/data/public/search/search_interceptor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { get, memoize, trimEnd } from 'lodash';
import { BehaviorSubject, throwError, timer, defer, from, Observable, NEVER } from 'rxjs';
import { catchError, finalize } from 'rxjs/operators';
import { CoreStart, CoreSetup, ToastsSetup } from 'kibana/public';
import { i18n } from '@kbn/i18n';
import {
getCombinedSignal,
AbortError,
Expand All @@ -31,7 +32,15 @@ import {
ISessionService,
} from '../../common';
import { SearchUsageCollector } from './collectors';
import { SearchTimeoutError, PainlessError, isPainlessError, TimeoutErrorMode } from './errors';
import {
SearchTimeoutError,
PainlessError,
isPainlessError,
TimeoutErrorMode,
isEsError,
EsError,
getHttpError,
} from './errors';
import { toMountPoint } from '../../../kibana_react/public';

export interface SearchInterceptorDeps {
Expand Down Expand Up @@ -101,8 +110,12 @@ export class SearchInterceptor {
} else if (options?.abortSignal?.aborted) {
// In the case an application initiated abort, throw the existing AbortError.
return e;
} else if (isPainlessError(e)) {
return new PainlessError(e, request);
} else if (isEsError(e)) {
if (isPainlessError(e)) {
return new PainlessError(e, request);
} else {
return new EsError(e);
}
} else {
return e;
}
Expand Down Expand Up @@ -236,24 +249,28 @@ export class SearchInterceptor {
*
*/
public showError(e: Error) {
if (e instanceof AbortError) return;

if (e instanceof SearchTimeoutError) {
if (e instanceof AbortError || e instanceof SearchTimeoutError) {
// The SearchTimeoutError is shown by the interceptor in getSearchError (regardless of how the app chooses to handle errors)
return;
}

if (e instanceof PainlessError) {
} else if (e instanceof EsError) {
this.deps.toasts.addDanger({
title: 'Search Error',
title: i18n.translate('data.search.esErrorTitle', {
defaultMessage: 'Cannot retrieve search results',
}),
text: toMountPoint(e.getErrorMessage(this.application)),
});
return;
} else if (e.constructor.name === 'HttpFetchError') {
this.deps.toasts.addDanger({
title: i18n.translate('data.search.httpErrorTitle', {
defaultMessage: 'Cannot retrieve your data',
}),
text: toMountPoint(getHttpError(e.message)),
});
} else {
this.deps.toasts.addError(e, {
title: 'Search Error',
});
}

this.deps.toasts.addError(e, {
title: 'Search Error',
});
}
}

Expand Down

0 comments on commit 046c840

Please sign in to comment.