-
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
[Lens][Embeddable] It renders two error sections unexpectedly from Lens Embeddable #156811
Comments
Pinging @elastic/security-solution (Team: SecuritySolution) |
Pinging @elastic/kibana-visualizations @elastic/kibana-visualizations-external (Team:Visualizations) |
cc @drewdaemon I think we need to display all the errors? Can you take a look? Thanx |
Why this is happeningI looked into this and realized that we're dealing with two lasagna layers of architecture, each with its own error handling system...
During my efforts to improve the error handling and display in the Lens layer with the new UserMessages architecture, I failed to realize that the EmbeddablePanel imposes its own error display. When our code updates the embeddable output to report an error state, it slaps its own UI on top of ours. This is why, as @angorayc noted, two DOM trees are present. The hidden one displaying the correct message is our own I think that the reason I didn't notice that this was happening before is that the embeddable panel's error UI looks similar to ours and it doesn't interfere with the (absolutely-positioned) non-blocking message list introduced in #147818. The final question is: if we are updating the embeddable output with the same error message as the one we are rendering in the Lens layer, why is the message displayed by the embeddable panel different/incomplete? The answer is that in the Lens layer we support displaying any React node, whereas the embeddable panel only supports strings. So, in this case, we're forced to use the short message on that UserMessage. Unfortunately, it looks like that Options
Option 2 is easiest and not as hacky as 1, but I also see arguments for trying to make embeddable error handling consistent via a unified API (this was attempted earlier in #143367). Thoughts @stratoula @dej611 @ThomThomson ? |
Another thing to think about is that right now the Lens embeddable only "counts" errors in the embeddable output if they are blocking. If they aren't blocking, we don't report them so that the EmbeddablePanel doesn't impose its error UI. So, we're already inconsistent. We show non-blocking errors and other messages like this: I guess if we chose to invest in option 4 (or 5), we could also move this ^^ to the embeddable level and other embeddables could make use of it for free (e.g. Vega). |
@drewdaemon thanx for the investigation! The second appears to be indeed the fastest solution but I really love the 4 and 5. I tend more to the 5th one but both of them seem as the correct way to go. I would love @ThomThomson 's input on that! |
I would vote for 2 and 5: basically moving the responsability of the error visualization on the renderer size, with a general fallback for those who do not register any error handling. |
I do very much like Option 5 here. In my opinion the Embeddable Error UI is for embeddable specific and completely unrecoverable errors... (could not locate that embeddable factory, an error was thrown in the embeddable creation process etc). I believe it should be possible to expose and render errors without putting them into the embeddable output stream - that way it would only be a concern of the individual Embeddable's implementation. |
Okay. To resolve this bug I will pursue strategy 1, removing these errors from the Lens embeddable's output stream. I have created #157894 to track progress toward solution 5. |
blocked by the new embeddable system refactoring #167429 |
Original issue: #156271 (comment)
Seems that it renders two error sections when using Lens Embeddable.
I intentionally indexed some documents with invalid field type so I can see the error view from Lens Embeddable.
Observation: The error message is incomplete. It does not display the full error description. (But I'd expect to see the full error description.)
broken_ip_ss_view.mov
After inspecting the elements, it seems that Lens Embeddable renders two error sections here. The first one with incomplete message, and the second one with complete message.
Screen.Recording.2023-05-05.at.10.17.03.mov
But it displays correctly in Lens
Originally posted by @angorayc in #156271 (comment)
The text was updated successfully, but these errors were encountered: