[Controls] Add Fatal Error Handling #133579

ThomThomson · 2022-06-03T22:04:52Z

Summary

Adds simple fatal error handling to the control wrappers, which is similar to how the embeddable frame handles it.

This PR also changes the Options List and Range Sliders to catch and throw errors properly when their data views are missing.

ThomThomson · 2022-06-06T13:45:14Z

@elasticmachine merge upstream

ThomThomson · 2022-06-06T14:06:41Z

@andreadelrio, this may need a styling once-over. I've added a new compressed layout to the ErrorEmbeddable class which isn't the best looking thing at the moment!

Heenawter

Tested locally + code review - everything works great from a behaviour perspective! Tested both deleting just the data view and deleting the entire index.

This is more of a question for @andreadelrio I think, but the error embeddable doesn't display nicely on small or medium sized controls where "Expand width" is set to false - it only shows properly on large controls:

Perhaps we could display a shortened version of the error in these cases, with the full error being displayed via popover or something? Or, at the very least, I think that we should ensure that the alert icon is always visible, since the above small and medium views only show part of data view ID, which doesn't really make it clear that an error occurred.

Heenawter · 2022-06-06T17:02:28Z

src/plugins/embeddable/public/lib/embeddables/error_embeddable.tsx

+    const node = this.compact ? (
+      <EuiFlexGroup
+        style={{ height: '100%', whiteSpace: 'nowrap' }}
+        gutterSize="none"


Perhaps gutterSize="xs" might be a little better here?

@Heenawter, you really caught how little attention I put into the design of this hahah!

I decided to cut my time fiddling with CSS short, but I'm hoping that @andreadelrio can take a look and fix some things I wasn't able to get to:

I couldn't change the error background color to white - for some reason the background-color rule is disabled, but only in error cases.

Using any gutter size shifts the EuiFlexGroup content upwards, because it adds a negative margin. You can see this even in the XS size in the screenshot above, it isn't exactly centered anymore. Is there something that the flex group needs to be wrapped in to avoid this?

I attempted to wrap the error text Markdown in a box that would use text-overflow:elipsis, but it didn't apply at all. I'm wondering if the markdown component comes with its own styling that overrides that.

Overall, this is a weird case, because the actual error text includes a link which needs to be rendered as markdown. I honestly wonder if we could just have a generic a fatal error has occured message on the control itself with a tooltip that shows the full message... Even then, the hyperlink wouldn't be created so the error message would look pretty bad, including all the markdown code in a popover hah.

andreadelrio · 2022-06-06T18:51:02Z

@ThomThomson Curious, why is editing the control disabled when this error occurs?

Should we align the wording with the error in panels? locate vs find, for example.
I think since space is so limited we need to shorten this as much as possible. Can we say Re-create it instead of click here to re-create it?
Following up on the comments regarding a tooltip/popover. I think given that the error is not really useful in small and medium sizes we could shorten the text and move the details to a popover. @ThomThomson is that possible?

ThomThomson · 2022-06-06T19:00:46Z

@andreadelrio, I've disabled editing when the control is in a fatal error state, because these are technically un-recoverable errors, which replace the Control with an ErrorEmbeddable. Plenty of other embeddables work this way when they get into an error state, but embeddables like Lens have their own specialized error handling which allows them to recover.

The wording here actually can't be modified without changing it everywhere, because this is unfortunately the actual error text that the data views service throws when it is unable to locate a data view. The way the Embeddable panel, and the control frame after this change, handle fatal errors is by exposing the actual error message. This helps with DX, but is sometimes not great from a UI perspective.

I was thinking of a tooltip, but if we used a popover, we could actually potentially render the markdown correctly on the inside of the popover, that's a good idea as long as you think the UX is okay.

So maybe the condensed error would show like this:

And on click we render markdown inside a popover

andreadelrio · 2022-06-06T19:11:35Z

@andreadelrio, I've disabled editing when the control is in a fatal error state, because these are technically un-recoverable errors, which replace the Control with an ErrorEmbeddable. Plenty of other embeddables work this way when they get into an error state, but embeddables like Lens have their own specialized error handling which allows them to recover.

The wording here actually can't be modified without changing it everywhere, because this is unfortunately the actual error text that the data views service throws when it is unable to locate a data view. The way the Embeddable panel, and the control frame after this change, handle fatal errors is by exposing the actual error message. This helps with DX, but is sometimes not great from a UI perspective.

I see! thanks for all the context.

I was thinking of a tooltip, but if we used a popover, we could actually potentially render the markdown correctly on the inside of the popover, that's a good idea as long as you think the UX is okay.

So maybe the condensed error would show like this:

And on click we render markdown inside a popover

I like this! The reason I was suggesting a popover instead of a tooltip is to allow for the inclusion of a click action (which is not possible when using a tooltip). I think a simple popover (no footer, no title) would work here. If possible, I would like for us to add "Learn more" at the end of the error text. We can show it only for non-small/medium controls if that works better. Just to account for the possibility that users might not know this error text is clickable and to let them know we're trying to provide them more info so they can recover from the error.

andreadelrio · 2022-06-06T19:14:26Z

Also, @Heenawter you're developing what we call an eagle eye for design. Very useful to have!

…splay the error info

ThomThomson · 2022-06-07T20:59:32Z

I've quickly re-designed the compact error embeddable to hide the error message in a popover. It still needs some design work, but this makes it a lot easier to control how the error control looks.

[Controls] Improve fatal error message styling

…rrorHandling

cchaos · 2022-06-08T20:29:10Z

I'd ask that you get the @elastic/ux-writers involved on this PR. At quick glance "Fatal" seems like extremely dangerous language. As a quick option I would just put "Can't find data view" as the main input text.

ThomThomson · 2022-06-08T20:31:50Z

That is true - Fatal is a pretty extreme word to use.

We can't use the actual error text as the main input text because the error could be anything and it could include any amount of markdown, which would potentially make it too large for this small space - hence the popover.

Maybe we just say An error has occurred. Short and sweet.

@KOTungseth, I'm open to any suggestion!

kellyemurphy · 2022-06-08T21:55:48Z

Hi @ThomThomson, Learn more links typically go to the documentation not a popover, this is a new UI pattern to use it that way. If I'm understanding correctly, the issue is that there is limited space and a multitude of reasons this could error out. The popover in the latest image is repeating the information in the box below, so maybe we don't need to show it in both places.

Just a general note about error messages, the most important things are to A. let them know what went wrong and B. tell them what they can do about it. So what do we want them to do with a generic error? Try again? Contact support?

ThomThomson · 2022-06-08T22:01:12Z

@kellyemurphy, that latest image is a little confusing - it's actually a dashboard, with 2 Controls and one Panel. All three of these things are in an error state, so that is the cause of the repetition.

I'm absolutely open to wording suggestions here. Basically we need one generic error message that fits in a small space and would prompt the user to click on it in order to see the actual error text.

It would be the actual error text inside the popover which would let them know what went wrong, and also tell them how to fix it.

ThomThomson · 2022-06-08T22:07:09Z

Here is a clearer picture of what one of these error controls looks like after the design PR by @andreadelrio. Hopefully that makes this a little clearer. The generic part is at the top, and the part that gives the real error and instructions on how to fix it is in the popover at the bottom.

kellyemurphy · 2022-06-08T22:13:06Z

Ok, so where does the Learn more link go? If clicking that opens the popoever, we need different words.

For phrasing, I'd recommend:

An error has occurred. Expand

or

There is an error. Read more

And then inside, it would say Couldn't locate the data view (ID: xxxxx), you need to <LINK>recreate</LINK> it.

ThomThomson · 2022-06-08T22:15:05Z

The Learn more link is actually the same as the other text in that it just opens the popover. I like An error has occurred. Read more that sounds much better than what we had. Thank you for the help!

gchaps · 2022-06-08T22:20:44Z

Maybe we can shorten it a bit:

Can't locate the data view (ID: xxxxx). <LINK>Recreate it</LINK>.

ThomThomson · 2022-06-09T14:25:16Z

@gchaps, the text there is actually coming from the error message itself, we could definitely change it to be shorter, but that is outside of the scope of this PR.

ThomThomson · 2022-06-13T15:45:20Z

@elasticmachine merge upstream

andreadelrio

Thanks for making all those updates. Design LGTM.

tsullivan · 2022-06-14T18:35:25Z

src/plugins/embeddable/public/lib/embeddables/error_embeddable.tsx

+    error: Error | string,
+    input: EmbeddableInput,
+    parent?: IContainer,
+    private compact: boolean = false


Can we have a unit test (snapshot) to cover the new param?

…Thomson/kibana into controls/fatalErrorHandling

ThomThomson · 2022-06-15T20:13:07Z

@elasticmachine merge upstream

tsullivan

LGTM

kibana-ci · 2022-06-15T21:14:31Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`embeddable`	75	80	+5

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`embeddable`	397	398	+1

Async chunks

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

id	before	after	diff
`controls`	426.5KB	427.4KB	+874.0B

Canvas Sharable Runtime

The Canvas "shareable runtime" is an bundle produced to enable running Canvas workpads outside of Kibana. This bundle is included in third-party webpages that embed canvas and therefor should be as slim as possible.

id	before	after	diff
`module count`	4956	4958	+2
`total size`	7.7MB	7.7MB	+3.3KB

Page load bundle

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

id	before	after	diff
`embeddable`	66.3KB	67.7KB	+1.5KB

Unknown metric groups

API count

id	before	after	diff
`embeddable`	487	488	+1

History

💚 Build #50874 succeeded 2a39ec5
💚 Build #50549 succeeded a99575e
💚 Build #49989 succeeded ca0b01f
💛 Build #49722 was flaky c8cce8a
💚 Build #49347 succeeded d56f2e7
💚 Build #49215 succeeded cf8757f

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

* Add fatal error handling to Controls Co-authored-by: andreadelrio <andrea.delrio@elastic.co> (cherry picked from commit 7757dbc)

* [Controls] Add Fatal Error Handling (#133579) * Add fatal error handling to Controls Co-authored-by: andreadelrio <andrea.delrio@elastic.co> (cherry picked from commit 7757dbc) * manually updated snapshot

Add fatal error handling to Controls

cf8757f

ThomThomson marked this pull request as ready for review June 6, 2022 13:44

ThomThomson requested review from a team as code owners June 6, 2022 13:44

ThomThomson added the release_note:fix label Jun 6, 2022

Merge branch 'main' into controls/fatalErrorHandling

d56f2e7

botelastic bot added the Feature:Embedding Embedding content via iFrame label Jun 6, 2022

Heenawter approved these changes Jun 6, 2022

View reviewed changes

ThomThomson added 2 commits June 7, 2022 16:55

redesigned compact error embeddable to include a popover to better di…

a6cd1bd

…splay the error info

capitalize Learn more.

c8cce8a

andreadelrio and others added 4 commits June 8, 2022 10:37

improve error message styling

ac61023

Merge pull request #17 from andreadelrio/controlsErrorStyle

26b484d

[Controls] Improve fatal error message styling

update class names to highlight compact view

0570fd9

Merge branch 'main' of github.com:elastic/kibana into controls/fatalE…

ca0b01f

…rrorHandling

i18n update

c588679

Merge branch 'main' into controls/fatalErrorHandling

a99575e

andreadelrio approved these changes Jun 13, 2022

View reviewed changes

tsullivan reviewed Jun 14, 2022

View reviewed changes

ThomThomson added 2 commits June 14, 2022 14:51

Snapshot testing for compact mode

d8103f9

Merge branch 'controls/fatalErrorHandling' of https://github.com/Thom…

2a39ec5

…Thomson/kibana into controls/fatalErrorHandling

Merge branch 'main' into controls/fatalErrorHandling

129f9dc

tsullivan approved these changes Jun 15, 2022

View reviewed changes

ThomThomson merged commit 7757dbc into elastic:main Jun 15, 2022

ThomThomson added a commit to ThomThomson/kibana that referenced this pull request Jun 15, 2022

[Controls] Add Fatal Error Handling (elastic#133579)

db578fa

* Add fatal error handling to Controls Co-authored-by: andreadelrio <andrea.delrio@elastic.co> (cherry picked from commit 7757dbc)

ThomThomson mentioned this pull request Jun 15, 2022

[8.3] [Controls] Add Fatal Error Handling (#133579) #134528

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Controls] Add Fatal Error Handling #133579

[Controls] Add Fatal Error Handling #133579

ThomThomson commented Jun 3, 2022

ThomThomson commented Jun 6, 2022

ThomThomson commented Jun 6, 2022

Heenawter left a comment

Heenawter Jun 6, 2022

ThomThomson Jun 6, 2022 •

edited

andreadelrio commented Jun 6, 2022

ThomThomson commented Jun 6, 2022

andreadelrio commented Jun 6, 2022

andreadelrio commented Jun 6, 2022

ThomThomson commented Jun 7, 2022

cchaos commented Jun 8, 2022

ThomThomson commented Jun 8, 2022

kellyemurphy commented Jun 8, 2022

ThomThomson commented Jun 8, 2022

ThomThomson commented Jun 8, 2022

kellyemurphy commented Jun 8, 2022

ThomThomson commented Jun 8, 2022 •

edited

gchaps commented Jun 8, 2022

ThomThomson commented Jun 9, 2022

ThomThomson commented Jun 13, 2022

andreadelrio left a comment

tsullivan Jun 14, 2022

ThomThomson commented Jun 15, 2022

tsullivan left a comment

kibana-ci commented Jun 15, 2022

API count

[Controls] Add Fatal Error Handling #133579

[Controls] Add Fatal Error Handling #133579

Conversation

ThomThomson commented Jun 3, 2022

Summary

ThomThomson commented Jun 6, 2022

ThomThomson commented Jun 6, 2022

Heenawter left a comment

Choose a reason for hiding this comment

Heenawter Jun 6, 2022

Choose a reason for hiding this comment

ThomThomson Jun 6, 2022 • edited

Choose a reason for hiding this comment

andreadelrio commented Jun 6, 2022

ThomThomson commented Jun 6, 2022

andreadelrio commented Jun 6, 2022

andreadelrio commented Jun 6, 2022

ThomThomson commented Jun 7, 2022

cchaos commented Jun 8, 2022

ThomThomson commented Jun 8, 2022

kellyemurphy commented Jun 8, 2022

ThomThomson commented Jun 8, 2022

ThomThomson commented Jun 8, 2022

kellyemurphy commented Jun 8, 2022

ThomThomson commented Jun 8, 2022 • edited

gchaps commented Jun 8, 2022

ThomThomson commented Jun 9, 2022

ThomThomson commented Jun 13, 2022

andreadelrio left a comment

Choose a reason for hiding this comment

tsullivan Jun 14, 2022

Choose a reason for hiding this comment

ThomThomson commented Jun 15, 2022

tsullivan left a comment

Choose a reason for hiding this comment

kibana-ci commented Jun 15, 2022

💚 Build Succeeded

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

Canvas Sharable Runtime

Page load bundle

API count

History

ThomThomson Jun 6, 2022 •

edited

ThomThomson commented Jun 8, 2022 •

edited