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.

Sign up for GitHub

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

Jump to bottom

[UnifiedFieldList] Handle exceptions in FieldPopover better #155866

Merged
merged 5 commits into from
Apr 27, 2023
Merged

[UnifiedFieldList] Handle exceptions in FieldPopover better #155866

merged 5 commits into from
Apr 27, 2023

Conversation

jughosta
Copy link
Contributor

@jughosta jughosta commented Apr 26, 2023 •
edited by kibanamachine
Loading

Summary

This PR wraps rendering of FieldPopover with additional try/catch and error boundary so we can look into exceptions if they happen.

@jughosta jughosta self-assigned this Apr 26, 2023
@jughosta jughosta added Team:DataDiscovery Discover App Team (Document Explorer, Saved Search, Surrounding documents, Graph) Feature:UnifiedFieldList The unified field list component used by Lens & Discover release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting labels Apr 26, 2023
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
unifiedFieldList 96 98 +2

Async chunks

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

id before after diff
unifiedFieldList 45.8KB 45.6KB -223.0B

Page load bundle

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

id before after diff
unifiedFieldList 52.3KB 52.8KB +533.0B
Unknown metric groups

ESLint disabled line counts

id before after diff
enterpriseSearch 17 19 +2
securitySolution 399 402 +3
unifiedFieldList 17 21 +4
total +9

Total ESLint disabled count

id before after diff
enterpriseSearch 18 20 +2
securitySolution 479 482 +3
unifiedFieldList 17 21 +4
total +9

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

cc @jughosta

@jughosta jughosta marked this pull request as ready for review April 26, 2023 15:11
@jughosta jughosta requested a review from a team as a code owner April 26, 2023 15:11
@elasticmachine
Copy link
Contributor

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

kertal
kertal approved these changes Apr 27, 2023
View reviewed changes
Copy link
Member

@kertal kertal left a comment

Choose a reason for hiding this comment

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

Checked out, tested and the error message when throwing an Exception initially was:

Bildschirmfoto 2023-04-27 um 08 51 14

With this PR it became

Bildschirmfoto 2023-04-27 um 08 52 18

We might consider showing a error message in the UI, but given this is a special case that should just surface when there are bootstrapping errors in development mode, it shouldn't be necessary ... 👍 for have this covered

@jughosta jughosta added backport:prev-minor Backport to the previous minor version (i.e. one version back from main) and removed backport:skip This commit does not require backporting labels Apr 27, 2023
@jughosta jughosta merged commit c571a38 into elastic:main Apr 27, 2023
@jughosta jughosta deleted the fieldlist-error branch April 27, 2023 08:10
@kibanamachine kibanamachine mentioned this pull request Apr 27, 2023
Merged
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Apr 27, 2023
@jughosta
[UnifiedFieldList] Handle exceptions in FieldPopover better (elastic#…
0606f75 
…155866)

## Summary

This PR wraps rendering of FieldPopover with additional try/catch and
error boundary so we can look into exceptions if they happen.

(cherry picked from commit c571a38)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
✅ 8.8

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Apr 27, 2023
@kibanamachine @jughosta
[8.8] [UnifiedFieldList] Handle exceptions in FieldPopover better (#1…
211f716 
…55866) (#155973)

# Backport

This will backport the following commits from `main` to `8.8`:
- [[UnifiedFieldList] Handle exceptions in FieldPopover better
(#155866)](#155866)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Julia
Rechkunova","email":"julia.rechkunova@elastic.co"},"sourceCommit":{"committedDate":"2023-04-27T08:09:56Z","message":"[UnifiedFieldList]
Handle exceptions in FieldPopover better (#155866)\n\n##
Summary\r\n\r\nThis PR wraps rendering of FieldPopover with additional
try/catch and\r\nerror boundary so we can look into exceptions if they
happen.","sha":"c571a389efaa826650172ff44d3de5f3eab3855b","branchLabelMapping":{"^v8.9.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:DataDiscovery","backport:prev-minor","Feature:UnifiedFieldList","v8.9.0"],"number":155866,"url":"#155866
Handle exceptions in FieldPopover better (#155866)\n\n##
Summary\r\n\r\nThis PR wraps rendering of FieldPopover with additional
try/catch and\r\nerror boundary so we can look into exceptions if they
happen.","sha":"c571a389efaa826650172ff44d3de5f3eab3855b"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.9.0","labelRegex":"^v8.9.0$","isSourceBranch":true,"state":"MERGED","url":"#155866
Handle exceptions in FieldPopover better (#155866)\n\n##
Summary\r\n\r\nThis PR wraps rendering of FieldPopover with additional
try/catch and\r\nerror boundary so we can look into exceptions if they
happen.","sha":"c571a389efaa826650172ff44d3de5f3eab3855b"}}]}]
BACKPORT-->

Co-authored-by: Julia Rechkunova <julia.rechkunova@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kertal kertal kertal approved these changes

Assignees

@jughosta jughosta

Labels
backport:prev-minor Backport to the previous minor version (i.e. one version back from main) Feature:UnifiedFieldList The unified field list component used by Lens & Discover release_note:skip Skip the PR/issue when compiling release notes Team:DataDiscovery Discover App Team (Document Explorer, Saved Search, Surrounding documents, Graph) v8.8.0 v8.9.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@jughosta @kibana-ci @elasticmachine @kibanamachine @kertal