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

Throttle KbnUrlStateStorage error messages #152983

Merged
merged 2 commits into from
Mar 9, 2023

Conversation

kertal
Copy link
Member

@kertal kertal commented Mar 9, 2023

Summary

This PR throttles the error messages caused by invalid URLs in KbnUrlStateStorage. So e.g. in Discover & Dashboard an invalid url state caused duplicate error toasts:

Bildschirmfoto 2023-03-09 um 09 09 45

Bildschirmfoto 2023-03-09 um 09 10 26

With this PR, there's just one error.

Bildschirmfoto 2023-03-09 um 10 04 44

In an upcoming PR, the URL state is requested more often initially in Discover, which leads to more toasts, and this was causing an functional test to fail. I researched and found a solution to fix this problem and decided to extract the solution to a separate PR

Testing

  • Go to Discover / Dashboard
  • Manipulate the URL so it's invalid (removing , or ))
  • There should be just 1 toast indicating the error restoring state from URL

Checklist

@kertal kertal requested a review from Dosant March 9, 2023 09:15
@kertal
Copy link
Member Author

kertal commented Mar 9, 2023

@Dosant this is still owned by @elastic/kibana-app-services, but I guess @elastic/appex-sharedux would be a better fit?

@kertal kertal self-assigned this Mar 9, 2023
@kertal kertal added Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. release_note:skip Skip the PR/issue when compiling release notes labels Mar 9, 2023
@kertal kertal marked this pull request as ready for review March 9, 2023 10:07
@kertal kertal requested a review from a team as a code owner March 9, 2023 10:07
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-data-discovery (Team:DataDiscovery)

@kertal kertal added Feature:Discover Discover Application Feature:Dashboard Dashboard related features labels Mar 9, 2023
@kibanamachine kibanamachine added the Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas label Mar 9, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-presentation (Team:Presentation)

@kertal kertal added v8.8.0 impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort labels Mar 9, 2023
@kertal
Copy link
Member Author

kertal commented Mar 9, 2023

it's pretty strange GitHub is telling be to add impact & loe labels? and adding those doesn't unblock the PR???

@nreese
Copy link
Contributor

nreese commented Mar 9, 2023

it's pretty strange GitHub is telling me to add impact & loe labels? and adding those doesn't unblock the PR???

I have seen that on a few of my PRs as well.

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Page load bundle

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

id before after diff
kibanaUtils 69.7KB 69.7KB +45.0B
Unknown metric groups

ESLint disabled line counts

id before after diff
securitySolution 432 434 +2

Total ESLint disabled count

id before after diff
securitySolution 509 511 +2

History

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

cc @kertal

@kertal
Copy link
Member Author

kertal commented Mar 9, 2023

@nreese thx, now it seems to work ... merging

@kertal kertal merged commit 63da800 into elastic:main Mar 9, 2023
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Mar 9, 2023
bmorelli25 pushed a commit to bmorelli25/kibana that referenced this pull request Mar 10, 2023
Fix duplicate error messages caused by invalid URLs in Discover & Dashboard.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Feature:Dashboard Dashboard related features Feature:Discover Discover Application impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort release_note:skip Skip the PR/issue when compiling release notes Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas v8.8.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants