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][Embeddable] It renders two error sections unexpectedly from Lens Embeddable #156811

Open
angorayc opened this issue May 5, 2023 · 10 comments
Assignees
Labels
blocked bug Fixes for quality problems that affect the customer experience Feature:Lens Charts Security Solution Lens Charts feature Feature:Lens impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.8.0

Comments

@angorayc
Copy link
Contributor

angorayc commented May 5, 2023

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.

DELETE auditbeat-custom-index-1

PUT auditbeat-custom-index-1

PUT auditbeat-custom-index-1/_mapping
{
  "properties": {
    "@timestamp": {
      "type": "date"
    },
    "event.category": {
      "type": "keyword",
      "ignore_above": 1024
    }
  }
}

POST auditbeat-custom-index-1/_doc
{
  "@timestamp": "2023-05-05T09:41:49.668Z",
  "host": {
    "name": "foo"
  },
  "event": {
    "category": "an_invalid_category"
  },
  "some.field": "this",
  "source": {
    "port": 90210,
    "ip": "10.1.2.3"
  }
}

POST auditbeat-custom-index-1/_doc
{
  "@timestamp": "2023-05-05T09:42:22.123Z",
  "host": {
    "name": "bar"
  },
  "event": {
    "category": "an_invalid_category"
  },
  "some.field": "space",
  "source": {
    "port": 867,
    "ip": "10.9.8.7"
  }
}

POST auditbeat-custom-index-1/_doc
{
  "@timestamp": "2023-05-05T09:43:35.456Z",
  "host": {
    "name": "baz"
  },
  "event": {
    "category": "theory"
  },
  "some.field": "for",
  "source": {
    "port": 5,
    "ip": "10.4.6.6"
  }
}

POST auditbeat-custom-index-1/_doc
{
  "@timestamp": "2023-05-05T09:44:36.700Z",
  "host": {
    "name": "@baz"
  },
  "event": {
    "category": "malware"
  },
  "some.field": "rent",
  "source": {
    "port": 309,
    "ip": "10.1.1.1"
  }
}

Observation: The error message is incomplete. It does not display the full error description. (But I'd expect to see the full error description.)

unique_ips_layer_1_error

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.

Screenshot 2023-05-05 at 10 34 45

Screen.Recording.2023-05-05.at.10.17.03.mov

But it displays correctly in Lens

Screenshot 2023-05-05 at 10 45 32

Originally posted by @angorayc in #156271 (comment)

@angorayc angorayc added bug Fixes for quality problems that affect the customer experience Team:Visualizations Visualization editors, elastic-charts and infrastructure Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. labels May 5, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-solution (Team: SecuritySolution)

@elasticmachine
Copy link
Contributor

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

@angorayc angorayc added the v8.8.0 label May 5, 2023
@angorayc angorayc changed the title [Lens][Embeddable] It renders two error sections unexpectedly [Lens][Embeddable] It renders two error sections unexpectedly from Lens Embeddable May 5, 2023
@stratoula
Copy link
Contributor

cc @drewdaemon I think we need to display all the errors? Can you take a look? Thanx

@angorayc angorayc added the Feature:Lens Charts Security Solution Lens Charts feature label May 5, 2023
@drewdaemon drewdaemon self-assigned this May 5, 2023
@drewdaemon
Copy link
Contributor

drewdaemon commented May 12, 2023

Why this is happening

I looked into this and realized that we're dealing with two lasagna layers of architecture, each with its own error handling system...

  • EmbeddablePanel
  • LensEmbeddable

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 VisualizationErrorPanel, while the visible one with the incorrect message is the embeddable panel's stuff.

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 shortMessage is for some reason incomplete. Sort of a hidden bug in the form-based datasource code in Lens.

Options

  1. Don't report errors in our embeddable output (not sure what the other ramifications of this would be)
  2. Introduce a way to tell the embeddable panel not to impose its own error display UI
  3. Accept the limitations of the embeddable panel's API and remove Lens's custom VisualizationErrorPanel. Fix the issue with that UserMessage's shortMessage so that a full error is displayed. Lose some capabilities and flexibility introduced with UserMessages.
  4. Extend the embeddable panel's API to match the current capabilities of Lens's UserMessage system and remove Lens's custom VisualizationErrorPanel.
  5. Remove the embeddable panel's error display capabilities entirely, putting the responsibility of rendering errors on the embeddables. Could export a set of shared components to promote consistency.

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 ?

@drewdaemon
Copy link
Contributor

drewdaemon commented May 12, 2023

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:

Screen Shot 2023-01-27 at 9 26 11 AM

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).

@stratoula
Copy link
Contributor

@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!

@dej611
Copy link
Contributor

dej611 commented May 15, 2023

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.

@ThomThomson
Copy link
Contributor

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.

@drewdaemon
Copy link
Contributor

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.

@stratoula stratoula added the impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. label Jun 1, 2023
@markov00
Copy link
Member

blocked by the new embeddable system refactoring #167429

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked bug Fixes for quality problems that affect the customer experience Feature:Lens Charts Security Solution Lens Charts feature Feature:Lens impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.8.0
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants