[8.8] [Dashboard] Fix alias redirect & update error handling (#159742) (#159937)

# Backport

This will backport the following commits from `main` to `8.8`:
- [[Dashboard] Fix alias redirect & update error handling
(#159742)](https://github.com/elastic/kibana/pull/159742)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Devon
Thomson","email":"devon.thomson@elastic.co"},"sourceCommit":{"committedDate":"2023-06-15T15:57:43Z","message":"[Dashboard]
Fix alias redirect & update error handling (#159742)\n\nMakes dashboard
load errors recoverable. Fixes a regression where Alias redirects
resulted in infinite
loading.","sha":"e7528a2372c846020d565f229da9052ba316284c","branchLabelMapping":{"^v8.9.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Feature:Dashboard","release_note:fix","Team:Presentation","loe:days","impact:critical","backport:prev-minor","v8.9.0"],"number":159742,"url":"https://github.com/elastic/kibana/pull/159742","mergeCommit":{"message":"[Dashboard]
Fix alias redirect & update error handling (#159742)\n\nMakes dashboard
load errors recoverable. Fixes a regression where Alias redirects
resulted in infinite
loading.","sha":"e7528a2372c846020d565f229da9052ba316284c"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.9.0","labelRegex":"^v8.9.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/159742","number":159742,"mergeCommit":{"message":"[Dashboard]
Fix alias redirect & update error handling (#159742)\n\nMakes dashboard
load errors recoverable. Fixes a regression where Alias redirects
resulted in infinite
loading.","sha":"e7528a2372c846020d565f229da9052ba316284c"}}]}]
BACKPORT-->
This commit is contained in:
Devon Thomson 2023-06-19 11:29:41 -04:00 committed by GitHub
parent 8109a55fb2
commit b2bed7c290
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 114 additions and 25 deletions

View file

@ -144,6 +144,7 @@ export const initializeDashboard = async ({
validateLoadedSavedObject &&
!validateLoadedSavedObject(loadDashboardReturn)
) {
// throw error to stop the rest of Dashboard loading and make the factory return an ErrorEmbeddable.
throw new Error('Dashboard failed saved object result validation');
}

View file

@ -97,11 +97,14 @@ export class DashboardContainerFactoryDefinition
const dashboardCreationStartTime = performance.now();
const { createDashboard } = await import('./create/create_dashboard');
try {
return Promise.resolve(
createDashboard(creationOptions, dashboardCreationStartTime, savedObjectId)
const dashboard = await createDashboard(
creationOptions,
dashboardCreationStartTime,
savedObjectId
);
return dashboard;
} catch (e) {
return new ErrorEmbeddable(e.text, { id: e.id });
return new ErrorEmbeddable(e, { id: e.id });
}
};
}

View file

@ -90,4 +90,76 @@ describe('dashboard renderer', () => {
'saved_object_kibanakiwi'
);
});
test('renders and destroys an error embeddable when the dashboard factory create method throws an error', async () => {
const mockErrorEmbeddable = {
error: 'oh my goodness an error',
destroy: jest.fn(),
render: jest.fn(),
} as unknown as DashboardContainer;
mockDashboardFactory = {
create: jest.fn().mockReturnValue(mockErrorEmbeddable),
} as unknown as DashboardContainerFactory;
pluginServices.getServices().embeddable.getEmbeddableFactory = jest
.fn()
.mockReturnValue(mockDashboardFactory);
let wrapper: ReactWrapper;
await act(async () => {
wrapper = await mountWithIntl(<DashboardRenderer savedObjectId="saved_object_kibanana" />);
});
expect(mockErrorEmbeddable.render).toHaveBeenCalled();
wrapper!.unmount();
expect(mockErrorEmbeddable.destroy).toHaveBeenCalledTimes(1);
});
test('creates a new dashboard container when the ID changes, and the first created dashboard resulted in an error', async () => {
// ensure that the first attempt at creating a dashboard results in an error embeddable
const mockErrorEmbeddable = {
error: 'oh my goodness an error',
destroy: jest.fn(),
render: jest.fn(),
} as unknown as DashboardContainer;
const mockErrorFactory = {
create: jest.fn().mockReturnValue(mockErrorEmbeddable),
} as unknown as DashboardContainerFactory;
pluginServices.getServices().embeddable.getEmbeddableFactory = jest
.fn()
.mockReturnValue(mockErrorFactory);
// render the dashboard - it should run into an error and render the error embeddable.
let wrapper: ReactWrapper;
await act(async () => {
wrapper = await mountWithIntl(<DashboardRenderer savedObjectId="saved_object_kibanana" />);
});
expect(mockErrorEmbeddable.render).toHaveBeenCalled();
expect(mockErrorFactory.create).toHaveBeenCalledTimes(1);
// ensure that the next attempt at creating a dashboard is successfull.
const mockSuccessEmbeddable = {
destroy: jest.fn(),
render: jest.fn(),
navigateToDashboard: jest.fn(),
} as unknown as DashboardContainer;
const mockSuccessFactory = {
create: jest.fn().mockReturnValue(mockSuccessEmbeddable),
} as unknown as DashboardContainerFactory;
pluginServices.getServices().embeddable.getEmbeddableFactory = jest
.fn()
.mockReturnValue(mockSuccessFactory);
// update the saved object id to trigger another dashboard load.
await act(async () => {
await wrapper.setProps({ savedObjectId: 'saved_object_kibanakiwi' });
});
expect(mockErrorEmbeddable.destroy).toHaveBeenCalled();
// because a new dashboard container has been created, we should not call navigate.
expect(mockSuccessEmbeddable.navigateToDashboard).not.toHaveBeenCalled();
// instead we should call create on the factory again.
expect(mockSuccessFactory.create).toHaveBeenCalledTimes(1);
});
});

View file

@ -18,8 +18,10 @@ import React, {
} from 'react';
import { v4 as uuidv4 } from 'uuid';
import classNames from 'classnames';
import useUnmount from 'react-use/lib/useUnmount';
import { EuiLoadingElastic, EuiLoadingSpinner } from '@elastic/eui';
import { ErrorEmbeddable, isErrorEmbeddable } from '@kbn/embeddable-plugin/public';
import {
DashboardAPI,
@ -47,6 +49,7 @@ export const DashboardRenderer = forwardRef<AwaitingDashboardAPI, DashboardRende
const [loading, setLoading] = useState(true);
const [screenshotMode, setScreenshotMode] = useState(false);
const [dashboardContainer, setDashboardContainer] = useState<DashboardContainer>();
const [fatalError, setFatalError] = useState<ErrorEmbeddable | undefined>();
useImperativeHandle(
ref,
@ -65,23 +68,22 @@ export const DashboardRenderer = forwardRef<AwaitingDashboardAPI, DashboardRende
})();
}, []);
useEffect(() => {
if (!dashboardContainer) return;
// When a dashboard already exists, don't rebuild it, just set a new id.
dashboardContainer.navigateToDashboard(savedObjectId);
// Disabling exhaustive deps because this useEffect should only be triggered when the savedObjectId changes.
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [savedObjectId]);
const id = useMemo(() => uuidv4(), []);
useEffect(() => {
let canceled = false;
let destroyContainer: () => void;
if (dashboardContainer) {
// When a dashboard already exists, don't rebuild it, just set a new id.
dashboardContainer.navigateToDashboard(savedObjectId);
return;
}
setLoading(true);
let canceled = false;
(async () => {
fatalError?.destroy();
setFatalError(undefined);
const creationOptions = await getCreationOptions?.();
// Lazy loading all services is required in this component because it is exported and contributes to the bundle size.
@ -91,12 +93,12 @@ export const DashboardRenderer = forwardRef<AwaitingDashboardAPI, DashboardRende
const dashboardFactory = embeddable.getEmbeddableFactory(
DASHBOARD_CONTAINER_TYPE
) as DashboardContainerFactory & { create: DashboardContainerFactoryDefinition['create'] };
const container = (await dashboardFactory?.create(
const container = await dashboardFactory?.create(
{ id } as unknown as DashboardContainerInput, // Input from creationOptions is used instead.
undefined,
creationOptions,
savedObjectId
)) as DashboardContainer;
);
if (canceled) {
container.destroy();
@ -104,20 +106,29 @@ export const DashboardRenderer = forwardRef<AwaitingDashboardAPI, DashboardRende
}
setLoading(false);
if (isErrorEmbeddable(container)) {
setFatalError(container);
return;
}
if (dashboardRoot.current) {
container.render(dashboardRoot.current);
}
setDashboardContainer(container);
destroyContainer = () => container.destroy();
})();
return () => {
canceled = true;
destroyContainer?.();
};
// Disabling exhaustive deps because embeddable should only be created on first render.
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);
}, [savedObjectId]);
useUnmount(() => {
fatalError?.destroy();
dashboardContainer?.destroy();
});
const viewportClasses = classNames(
'dashboardViewport',
@ -131,10 +142,12 @@ export const DashboardRenderer = forwardRef<AwaitingDashboardAPI, DashboardRende
<EuiLoadingElastic size="xxl" />
);
return (
<div className={viewportClasses}>
{loading ? loadingSpinner : <div ref={dashboardRoot} />}
</div>
);
const renderDashboardContents = () => {
if (fatalError) return fatalError.render();
if (loading) return loadingSpinner;
return <div ref={dashboardRoot} />;
};
return <div className={viewportClasses}>{renderDashboardContents()}</div>;
}
);