Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Search] Error Alignment 2 #80965

Merged
merged 10 commits into from
Oct 26, 2020
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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the cast here necessary (since you've checked isEsError above, which is implemented as a type check)?

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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Prefer ?? to ||

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