-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Watcher] Fix search bug that crashes the app #162687
[Watcher] Fix search bug that crashes the app #162687
Conversation
@elasticmachine merge upstream |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
💚 Build Succeeded
Metrics [docs]Async chunks
History
To update your PR or re-run it, just comment with: cc @sabarasaba |
Pinging @elastic/platform-deployment-management (Team:Deployment Management) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for the fix, @sabarasaba!
Tested locally and the search doesn't crush the page anymore 👍
Code changes LGTM too, would be great to maybe also add a test for the bug if possible.
@sabarasaba I think we need to take another look at this one and approach it differently. I re-enabled the watcher jest tests in #162592 and I see this test fails with your change. The list view is refreshed automatically, which causes the query to reset. |
If you look at the EUI docs, the |
Fixes #123071
Summary
The table config for the watcher list had a managed state for the search query that wasn't accounting for possible query errors (in our case, using special characters) causing the app to crash. Since it was unecessary for the query state to be managed, I removed that and we now have the default behaviour we have in other tables when these sort of errors happen.
Note to reviewers: in order to test this, just create a regular watcher and try typing
or
in the search and verify that the app doesnt crash and that the search box turns into error state.