[Search Sessions] Improve search session errors #88613

lizozom · 2021-01-18T16:07:26Z

Resolves #88570

Normalizes and adds test (unit and api integration) to the error format returned from the server for search, msearch, async_search and bsearch. The new format is

{
    statusCode: number;
    message: string;
    attributes?: T
}

Adds actual elasticsearch resopnses for better testing

While before, the error messages were not shown, now the errors look like this:

Summary

Summarize your PR. If it involves visual changes include a screenshot or gif.

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
If a plugin configuration key changed, check if it needs to be whitelisted in the cloud and added to the docker list
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

For maintainers

This was checked for breaking API changes and was labeled appropriately

Fix bfetch error (was recognized as unknown error) Make sure handleSearchError always returns an error object.

elasticmachine · 2021-01-18T16:07:29Z

Pinging @elastic/kibana-app-services (Team:AppServices)

Dosant · 2021-01-18T16:52:13Z

Noticed a problem with scripted field error:

Branch:

Master:

probably worth checking other special cases, like maybe, shards failures and request timeouts

…r-format

Validate abort

Adjusted bsearch response format Added verification of error's root cause

…r-format

lizozom · 2021-01-27T09:40:03Z

@elasticmachine merge upstream

lukasolson

Overall, LGTM. Left a couple of comments below.

src/plugins/kibana_utils/common/errors/types.ts

lukasolson · 2021-01-28T00:05:26Z

x-pack/plugins/data_enhanced/server/search/es_search_strategy.ts

+    try {
+      await esClient.asCurrentUser.asyncSearch.delete({ id });
+    } catch (e) {
+      throw getKbnServerError(e);


Is it worth updating the other search strategies, or at least letting them know that this is the preferred way to handle errors?

Dosant

in general LGTM. great effort and love all tests!

Some feedback:

Noticed one empty fail that seems to be not used:
src/plugins/data/common/search/test_data/illegal_state_exception.json
It seems that any usage grew up? I would really like it if we could avoid that. For example, I think, that toKibanaServerError should handle unknown and do all needed runtime checks inside
Since this affects all data plugin consumers and errors are something that is often skipped in automated tests, do you think it makes sense to ask some of the plugin consumers teams to test this also in their apps? Maybe someone implicitly relies somewhere on older error formats.

Dosant · 2021-01-28T09:58:34Z

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<string, any>, mode: TimeoutErrorMode) {


Is this change necessary?
Looks like we creating SearchTimeoutError where we known we are passing KibanaServerError
Could it be KibanaServerError then?

src/plugins/data/public/search/search_interceptor.ts

src/plugins/data/server/search/es_search/es_search_strategy.ts

src/plugins/data/server/search/routes/bsearch.ts

test/api_integration/apis/search/bsearch.ts

…r-format

…o sessions/error-format

…r-format

lizozom · 2021-01-28T18:24:11Z

@elasticmachine merge upstream

…o sessions/error-format

lizozom · 2021-01-28T20:27:31Z

@elasticmachine merge upstream

lizozom · 2021-01-29T09:18:05Z

@elasticmachine merge upstream

lizozom · 2021-01-31T08:20:12Z

@elasticmachine merge upstream

kibanamachine · 2021-01-31T08:20:25Z

expected head sha didn’t match current head ref.

…r-format

…o sessions/error-format

kibanamachine · 2021-01-31T10:09:43Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 98a1bb4

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`kibanaUtils`	192	193	+1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`kibanaUtils`	123.2KB	123.2KB	+13.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`data`	799.3KB	798.5KB	-830.0B
`kibanaUtils`	142.1KB	154.5KB	+12.4KB
total			+11.6KB

History

💛 Build #102062 was flaky 6057ece
💔 Build #101927 failed 244a93b
💔 Build #101874 failed 7c94fe0
💔 Build #101816 failed edaa188
💔 Build #101777 failed f74e58b

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

* 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>

* 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> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Detect ESError correctly

e140045

Fix bfetch error (was recognized as unknown error) Make sure handleSearchError always returns an error object.

lizozom added Feature:Search Querying infrastructure in Kibana v8.0.0 Team:AppServices v7.12.0 labels Jan 18, 2021

lizozom requested a review from a team as a code owner January 18, 2021 16:07

lizozom self-assigned this Jan 18, 2021

lizozom changed the title ~~Improve search session errors~~ [Search Sessions] Improve search session errors Jan 18, 2021

lizozom added the release_note:skip Skip the PR/issue when compiling release notes label Jan 18, 2021

Liza K added 15 commits January 18, 2021 20:51

fix tests and improve types

738b8a1

type

7f94977

normalize search error response format for search and bsearch

cdf80e2

Merge branch 'master' of github.com:elastic/kibana into sessions/erro…

6203a02

…r-format

type

5e3eb1f

Added es search exception examples

f57c316

Normalize and validate errors thrown from oss es_search_strategy

883fd9f

Validate abort

Added tests for search service error handling

07d0f7d

Update msearch tests to test for errors

f1ef0c4

Moved bsearch route to routes folder

20fff53

Adjusted bsearch response format Added verification of error's root cause

Align painless error object

f562140

eslint

f99eb8a

Add to seach interceptor tests

e74b40d

Merge branch 'master' of github.com:elastic/kibana into sessions/erro…

bca6410

…r-format

add json to tsconfig

41a6efa

lizozom marked this pull request as draft January 20, 2021 19:55

Liza K added 3 commits January 21, 2021 12:20

docs

57d35a0

Merge branch 'master' of github.com:elastic/kibana into sessions/erro…

1e0baaf

…r-format

updated xpack search strategy tests

38ad50f

Merge branch 'master' into sessions/error-format

582d76c

elastic deleted a comment from alonxprt Jan 27, 2021

lukasolson approved these changes Jan 28, 2021

View reviewed changes

Dosant reviewed Jan 28, 2021

View reviewed changes

Liza K added 6 commits January 28, 2021 16:57

Merge branch 'master' of github.com:elastic/kibana into sessions/erro…

48a44b4

…r-format

code review

237ed01

Merge branch 'sessions/error-format' of github.com:lizozom/kibana int…

f3f3a84

…o sessions/error-format

Merge branch 'master' of github.com:elastic/kibana into sessions/erro…

f74e58b

…r-format

fix

edaa188

Merge branch 'master' of github.com:elastic/kibana into sessions/erro…

e6648a1

…r-format

kibanamachine and others added 3 commits January 28, 2021 13:24

Merge branch 'master' into sessions/error-format

1435ec1

code review 2

434fc62

Merge branch 'sessions/error-format' of github.com:lizozom/kibana int…

7c94fe0

…o sessions/error-format

Merge branch 'master' into sessions/error-format

244a93b

Merge branch 'master' into sessions/error-format

6057ece

Merge branch 'master' into sessions/error-format

b6950d2

Liza K added 2 commits January 31, 2021 10:31

Merge branch 'master' of github.com:elastic/kibana into sessions/erro…

7d7fca5

…r-format

Merge branch 'sessions/error-format' of github.com:lizozom/kibana int…

98a1bb4

…o sessions/error-format

lizozom merged commit 841ab70 into elastic:master Jan 31, 2021

lizozom mentioned this pull request Jan 31, 2021

[7.x] [Search Sessions] Improve search session errors (#88613) #89823

Merged

lizozom mentioned this pull request Feb 1, 2021

[Data plugin] "root_cause" stripped from error returned #84617

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Search Sessions] Improve search session errors #88613

[Search Sessions] Improve search session errors #88613

lizozom commented Jan 18, 2021 •

edited

Loading

elasticmachine commented Jan 18, 2021

Dosant commented Jan 18, 2021 •

edited

Loading

lizozom commented Jan 27, 2021

lukasolson left a comment

lukasolson Jan 28, 2021

Dosant left a comment

Dosant Jan 28, 2021

lizozom commented Jan 28, 2021

lizozom commented Jan 28, 2021

lizozom commented Jan 29, 2021

lizozom commented Jan 31, 2021

kibanamachine commented Jan 31, 2021

kibanamachine commented Jan 31, 2021

[Search Sessions] Improve search session errors #88613

[Search Sessions] Improve search session errors #88613

Conversation

lizozom commented Jan 18, 2021 • edited Loading

Summary

Checklist

For maintainers

elasticmachine commented Jan 18, 2021

Dosant commented Jan 18, 2021 • edited Loading

lizozom commented Jan 27, 2021

lukasolson left a comment

Choose a reason for hiding this comment

lukasolson Jan 28, 2021

Choose a reason for hiding this comment

Dosant left a comment

Choose a reason for hiding this comment

Dosant Jan 28, 2021

Choose a reason for hiding this comment

lizozom commented Jan 28, 2021

lizozom commented Jan 28, 2021

lizozom commented Jan 29, 2021

lizozom commented Jan 31, 2021

kibanamachine commented Jan 31, 2021

kibanamachine commented Jan 31, 2021

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Page load bundle

History

lizozom commented Jan 18, 2021 •

edited

Loading

Dosant commented Jan 18, 2021 •

edited

Loading