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] Emit errors from EQL search strategy #81071

Merged
merged 1 commit into from
Oct 20, 2020

Conversation

rylnd
Copy link
Contributor

@rylnd rylnd commented Oct 19, 2020

Summary

This effectively wraps our EQL strategy in a try/catch, resulting in the observable emitting errors when e.g. the EQL client has an error response. Since the first use case for this strategy involves heavy use of the ignore option, we had not hit this behavior initially.

Checklist

For maintainers

Errors thrown by e.g. the EQL client would not be caught by the search
strategy.
@rylnd rylnd added bug Fixes for quality problems that affect the customer experience v8.0.0 Team:AppArch release_note:skip Skip the PR/issue when compiling release notes v7.10.0 v7.11.0 labels Oct 19, 2020
@rylnd rylnd requested a review from lizozom October 19, 2020 21:30
@rylnd rylnd self-assigned this Oct 19, 2020
@rylnd rylnd changed the title [Search] Handle errors in EQL search strategy [Search] Emit errors from EQL search strategy Oct 19, 2020
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

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

@rylnd rylnd marked this pull request as ready for review October 19, 2020 23:16
@rylnd rylnd requested a review from a team as a code owner October 19, 2020 23:16
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-arch (Team:AppArch)

@yctercero
Copy link
Contributor

This looks great Ryland, thanks so much for the quick fix! Would it be possible to add a mock response for sequences and non sequences here? Not sure if data plugin uses different mock pattern, but thought it could be handy.

Copy link
Member

@lukeelmers lukeelmers left a comment

Choose a reason for hiding this comment

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

Code LGTM!

@rylnd rylnd merged commit cdf276c into elastic:master Oct 20, 2020
@rylnd rylnd deleted the handle_eql_client_errors branch October 20, 2020 19:49
rylnd added a commit that referenced this pull request Oct 20, 2020
Errors thrown by e.g. the EQL client would not be caught by the search
strategy.

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@rylnd rylnd removed the v7.10.0 label Oct 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience release_note:skip Skip the PR/issue when compiling release notes v7.11.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants