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

[Lens][Expressions] Fixes react Error When Rendering Recoverable Error #196285

Merged
merged 3 commits into from
Oct 17, 2024

Conversation

mbondyra
Copy link
Contributor

@mbondyra mbondyra commented Oct 15, 2024

Summary

  1. Fixes [Lens][Expressions] React Error When Rendering Recoverable Error #159146
  2. Makes sure that the data-render-error disappears once the expression renderer is recovered (ie. when user fixes the query) - before once the error appears, it stays there forever.
Screenshot 2024-10-15 at 13 11 14

@mbondyra mbondyra added release_note:fix Team:Visualizations Visualization editors, elastic-charts and infrastructure Feature:Lens backport:skip This commit does not require backporting labels Oct 15, 2024
@mbondyra mbondyra requested a review from a team as a code owner October 15, 2024 11:11
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-visualizations (Team:Visualizations)

@mbondyra mbondyra added backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) and removed backport:skip This commit does not require backporting labels Oct 15, 2024
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

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

id before after diff
lens 1.5MB 1.5MB +50.0B

Copy link
Member

@markov00 markov00 left a comment

Choose a reason for hiding this comment

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

Codewise looks fine. Just posted a question that could probably be used to improve a bit the code quality of that file (hopefully adding comments or renaming some props)

setIsRenderComplete(true);
onRender$();
}, [setIsRenderComplete, onRender$]);
}, [onRender$, setDynamicError, setIsRenderComplete]);
Copy link
Member

Choose a reason for hiding this comment

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

qq: Could be great to know what a DynamicError is since it looks like used only in this workspace_panel file.
Why is "dynamic"?

Copy link
Contributor Author

@mbondyra mbondyra Oct 17, 2024

Choose a reason for hiding this comment

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

I guess what we meant the error is runtime and coming from the fact we cannot generate expression (so most likely it would be a wrong kql syntax or a user mistake that can be recoverable). When it appears, ExpressionRendererComponent instead of trying to render the expression, renders the content of the renderError prop.

What if we call it hasRequestError / setHasRequestError instead? 🤔

@mbondyra mbondyra merged commit 8cfa396 into elastic:main Oct 17, 2024
21 checks passed
@mbondyra mbondyra deleted the lens/fix_error_message branch October 17, 2024 14:22
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/11386934530

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 17, 2024
elastic#196285)

## Summary

1. Fixes elastic#159146
2. Makes sure that the `data-render-error` disappears once the
expression renderer is recovered (ie. when user fixes the query) -
before once the error appears, it stays there forever.

<img width="651" alt="Screenshot 2024-10-15 at 13 11 14"
src="https://github.com/user-attachments/assets/9fc3639a-723f-4ab2-9508-85caa4052ab9">

(cherry picked from commit 8cfa396)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.x

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 Oct 17, 2024
…e Error (#196285) (#196714)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Lens][Expressions] Fixes react Error When Rendering Recoverable
Error (#196285)](#196285)

<!--- Backport version: 9.4.3 -->

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

<!--BACKPORT [{"author":{"name":"Marta
Bondyra","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-17T14:22:01Z","message":"[Lens][Expressions]
Fixes react Error When Rendering Recoverable Error (#196285)\n\n##
Summary\r\n\r\n1. Fixes
https://github.com/elastic/kibana/issues/159146\r\n2. Makes sure that
the `data-render-error` disappears once the\r\nexpression renderer is
recovered (ie. when user fixes the query) -\r\nbefore once the error
appears, it stays there forever.\r\n\r\n\r\n<img width=\"651\"
alt=\"Screenshot 2024-10-15 at 13 11
14\"\r\nsrc=\"https://github.com/user-attachments/assets/9fc3639a-723f-4ab2-9508-85caa4052ab9\">","sha":"8cfa396f463b10006f247f91c10c591ac19f6dc2","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:Visualizations","Feature:Lens","v9.0.0","backport:prev-minor","v8.17.0"],"title":"[Lens][Expressions]
Fixes react Error When Rendering Recoverable
Error","number":196285,"url":"https://github.com/elastic/kibana/pull/196285","mergeCommit":{"message":"[Lens][Expressions]
Fixes react Error When Rendering Recoverable Error (#196285)\n\n##
Summary\r\n\r\n1. Fixes
https://github.com/elastic/kibana/issues/159146\r\n2. Makes sure that
the `data-render-error` disappears once the\r\nexpression renderer is
recovered (ie. when user fixes the query) -\r\nbefore once the error
appears, it stays there forever.\r\n\r\n\r\n<img width=\"651\"
alt=\"Screenshot 2024-10-15 at 13 11
14\"\r\nsrc=\"https://github.com/user-attachments/assets/9fc3639a-723f-4ab2-9508-85caa4052ab9\">","sha":"8cfa396f463b10006f247f91c10c591ac19f6dc2"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/196285","number":196285,"mergeCommit":{"message":"[Lens][Expressions]
Fixes react Error When Rendering Recoverable Error (#196285)\n\n##
Summary\r\n\r\n1. Fixes
https://github.com/elastic/kibana/issues/159146\r\n2. Makes sure that
the `data-render-error` disappears once the\r\nexpression renderer is
recovered (ie. when user fixes the query) -\r\nbefore once the error
appears, it stays there forever.\r\n\r\n\r\n<img width=\"651\"
alt=\"Screenshot 2024-10-15 at 13 11
14\"\r\nsrc=\"https://github.com/user-attachments/assets/9fc3639a-723f-4ab2-9508-85caa4052ab9\">","sha":"8cfa396f463b10006f247f91c10c591ac19f6dc2"}},{"branch":"8.x","label":"v8.17.0","branchLabelMappingKey":"^v8.17.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Marta Bondyra <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) Feature:Lens release_note:fix Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.17.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Lens][Expressions] React Error When Rendering Recoverable Error
4 participants