[Dashboard] Fix alias redirect & update error handling

[ThomThomson]

Summary

This PR fixes a regression introduced in #150121 & #157437.

Basically what would happen is the following:

1. The Dashboard renderer would begin loading the dashboard container.
2. The validation would result in an alias match.
3. This would update the URL, but also throw an error.
4. The new fast navigation would see that the url was updated, but it wouldn't redirect because the DashboardContainer was never completed.
5. The dashboard would infinite load 🔥

This PR fixes that with proper error handling on the dashboard container factory side, and the ability for the dashboard renderer to retry loading the dashboard container if the last load was unsuccessful.

Before

Before.mp4

After

After.mp4

Bonus

Additionally, this PR straightens out the error handling on Dashboard creation. If an error embeddable is returned from the dashboard container factory, the dashboard renderer will render it until the id changes. I got this screenshot by throwing an error in create_dashboard.ts.

Checklist

Delete any items that are not applicable to this PR.

• Unit or functional tests were updated or added to match the most common scenarios

[elasticmachine]

Pinging @elastic/kibana-presentation (Team:Presentation)

[nreese]

Shouldn't setLoading(false); get called if container is an error embeddable? The container finished loading, even if it loaded in an error state

[ThomThomson]

Good call, I didn't catch that because the error was rendered if it was present no matter if the loading state was true or not. I've straightened that out!

[nreese]

LGTM
code review only

[nreese]

nit, there should never be a case where loadingSpinner and fatalError.render() are displayed but there could be if both loading and fatalError are set. Would it be clearer to move this logic to a function and use early returns so only a single item is rendered. For example

function renderContent() {
  if (loading) {
    return loadingSpinner;
  }

  if (fatalError) {
    return fatalError.render();
  }

  return (<div ref={dashboardRoot} />);
}

[ThomThomson]

That's true. Even if it wouldn't necessarily happen, it's still much cleaner and more understandable to do it this way.

[kibana-ci]

💚 Build Succeeded

• Buildkite Build
• Commit: 7228074

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
dashboard	358.6KB	358.1KB	-514.0B

Page load bundle

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

id	before	after	diff
dashboard	39.4KB	39.9KB	+477.0B

Unknown metric groups

ESLint disabled line counts

id	before	after	diff
dashboard	8	7	-1
enterpriseSearch	13	15	+2
securitySolution	410	414	+4
total			+5

Total ESLint disabled count

id	before	after	diff
dashboard	8	7	-1
enterpriseSearch	14	16	+2
securitySolution	493	497	+4
total			+5

History

• 💚 Build #135341 succeeded 9c4579a

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

[kibanamachine]

💔 All backports failed

Status	Branch	Result
❌	8.8	Backport failed because of merge conflicts

Manual backport

To create the backport manually run:

node scripts/backport --pr 159742

Questions ?

Please refer to the Backport tool documentation

[ThomThomson]

Waiting for #157437 to be backported to 8.8 before this one can be.

[renshuki]

@ThomThomson looks like #157437 got backported to 8.x right?
Could you please backport this as well so it could make it to 8.8.2? Thanks 🙏