Propagate Errors in the chart component to parent

[drewdaemon]

Describe the issue A runtime error thrown from within the chart component tree does not trigger error handling logic in the parent tree. This makes it hard to build reliable error handling behavior as a client.

The error boundary will catch the error from BuggyThing if the boundary is within the Chart's children

<Chart>
  <ErrorBoundary>
    <BuggyThing />
  </ErrorBoundary>
</Chart>

The error boundary will NOT catch the error from BuggyThing if the boundary is NOT within the Chart's children

<ErrorBoundary>
  <Chart>
    <BuggyThing />
  </Chart>
</ErrorBoundary>

To Reproduce I created a sandbox

Expected behaviour There should be some way to reliably and easily detect when an error has occurred in the Chart's component tree. For example, this could mean allowing errors to propagate or adding an onError callback.

Version (please complete the following information):

• OS: OSX
• Browser: Edge, likely all browsers
• Elastic Charts: 47.2.0

Additional context Add any other context about the problem here.

Kibana Cross Issues https://github.com/elastic/kibana/issues/137095

Checklist

• [x] Every related Kibana issue is listed under Kibana Cross Issues list and the kibana cross issue tag is applied

[nickofthyme]

Yeah, we wrap the Chart component in an ErrorBoundary to prevent the chart from erroring to the parent. I think the onError idea is a great idea.

https://github.com/elastic/elastic-charts/blob/35bb737b7c5031a8b64b68eb85720bbe3f033df6/packages/charts/src/components/chart.tsx#L171-L176

Eventually, I'd like to have a complete error handling mechanism to surface hints about the chart config that are not good practice or log why things are breaking or not rendering.

[nickofthyme]

We also have the idea of a GracefulError see https://github.com/elastic/elastic-charts/pull/779. Another approach it to have an option to propagate errors allowing the use of an ErrorBoundary, otherwise we catch all component errors. Since this is expected behavior I'm gonna remove the bug label.

Let us know the urgency of this change.

[drewdaemon]

Thanks for the response, @nickofthyme ! All makes sense.

cc: @flash1293 WRT priority

[flash1293]

Not super urgent but it’s the last bit of our “proper error handling” initiative for Lens charts. Right now the behavior of errors within elastic-charts in a Lens embeddable is endless loading without a clear indication what’s going on which could be mistaken for a slow loading cluster.

[markov00]

If we consider that, except some edge cases like charts with big amount of elements, the loading time of a chart is relatively negligible in respect to the rest of the data pipeline, you could also consider stopping the loaders when the data is returned from ES and reached the chart API. The spinners could stop there. At least you have a clear indication that the data has loaded and reached the chart, but the chart isn't rendering.

I totally agree with providing an onError callback to the parent, the other reason why we wrapped the internals within an error boundary is to avoid that every chart user needs to implement their own error boundaries.

I'm also very curious if you can provide a list of errors that happens usually within charts in Lens to know if we can provide a graceful handling

[flash1293]

The spinners could stop there. At least you have a clear indication that the data has loaded and reached the chart, but the chart isn't rendering.

It would still leave an empty panel and the user doesn't know what's going on. This is about failling gracefully if an error of some kind occurs, which at the very least includes a "something went wrong" message in the panel.

I'm also very curious if you can provide a list of errors that happens usually within charts in Lens to know if we can provide a graceful handling

This is not about expected error cases - we would catch them before passing the data to elastic-charts. It's to behave better in the presence of unexpected behavior (like a bug which throws given some exotic configuration a user messed with manually in the export file, or Elasticsearch sending unexpected data for some reason, and so on)

[flash1293]

The graceful handling in this case needs to be to pass the error to the caller (in this case Lens), as we need to propagate it further up the dashboard to make sure all the right mechanisms kick in (e.g. failing a report with a meaningful error message instead of just "timeout")

[flash1293]

I totally agree with providing an onError callback to the parent, the other reason why we wrapped the internals within an error boundary is to avoid that every chart user needs to implement their own error boundaries.

I'm fine with a callback, but IMHO this isn't a good behavior - it's better to delegate error handling to the caller (this is how react components normally behave and what is expected by a user) - I don't see a strong argument why a chart component should break out of this.

[drewdaemon]

I'm also very curious if you can provide a list of errors that happens usually within charts in Lens to know if we can provide a graceful handling

I appreciate this, but the problem isn't confined to errors originating in elastic-charts code.

In the case of https://github.com/elastic/kibana/issues/137052 the error didn't originate from the elastic-charts library—it was actually on our side. But since our component was executing within the Chart's component tree, the elastic-charts error boundary was catching it, interrupting the error escalation.