[User Experience] Add error boundary to prevent UX dashboard from cra…

…shing the application (#117583) * wrap UX dashboard into an error boundary (fixes #117543) * refactor APM root app tests to reuse coreMock Before this change, the tests for the root application component of the APM app were manually mocking the `coreStart` objects required to render the component. After this change, these tests will now reuse the relevant `coreMock` methods. * refactor: fix typo on createAppMountParameters test utility Co-authored-by: Lucas Fernandes da Costa <[email protected]> Co-authored-by: Kibana Machine <[email protected]>
elastic · Nov 8, 2021 · 1b82502 · 1b82502
1 parent 635cad4
commit 1b82502
Show file tree

Hide file tree

Showing 7 changed files with 105 additions and 89 deletions.
diff --git a/dev_docs/tutorials/testing_plugins.mdx b/dev_docs/tutorials/testing_plugins.mdx
@@ -458,7 +458,7 @@ describe('Plugin', () => {
     const [coreStartMock, startDepsMock] = await coreSetup.getStartServices();
     const unmountMock = jest.fn();
     renderAppMock.mockReturnValue(unmountMock);
-    const params = coreMock.createAppMountParamters('/fake/base/path');
+    const params = coreMock.createAppMountParameters('/fake/base/path');
 
     new Plugin(coreMock.createPluginInitializerContext()).setup(coreSetup);
     // Grab registered mount function
@@ -528,7 +528,7 @@ import { renderApp } from './application';
 
 describe('renderApp', () => {
   it('mounts and unmounts UI', () => {
-    const params = coreMock.createAppMountParamters('/fake/base/path');
+    const params = coreMock.createAppMountParameters('/fake/base/path');
     const core = coreMock.createStart();
 
     // Verify some expected DOM element is rendered into the element
@@ -540,7 +540,7 @@ describe('renderApp', () => {
   });
 
   it('unsubscribes from uiSettings', () => {
-    const params = coreMock.createAppMountParamters('/fake/base/path');
+    const params = coreMock.createAppMountParameters('/fake/base/path');
     const core = coreMock.createStart();
     // Create a fake Subject you can use to monitor observers
     const settings$ = new Subject();
@@ -555,7 +555,7 @@ describe('renderApp', () => {
   });
 
   it('resets chrome visibility', () => {
-    const params = coreMock.createAppMountParamters('/fake/base/path');
+    const params = coreMock.createAppMountParameters('/fake/base/path');
     const core = coreMock.createStart();
 
     // Verify stateful Core API was called on mount

diff --git a/src/core/public/mocks.ts b/src/core/public/mocks.ts
@@ -169,5 +169,5 @@ export const coreMock = {
   createStart: createCoreStartMock,
   createPluginInitializerContext: pluginInitializerContextMock,
   createStorage: createStorageMock,
-  createAppMountParamters: createAppMountParametersMock,
+  createAppMountParameters: createAppMountParametersMock,
 };
diff --git a/x-pack/plugins/apm/public/application/application.test.tsx b/x-pack/plugins/apm/public/application/application.test.tsx
@@ -7,24 +7,31 @@
 
 import React from 'react';
 import { act } from '@testing-library/react';
+import { EuiErrorBoundary } from '@elastic/eui';
+import { mount } from 'enzyme';
 import { createMemoryHistory } from 'history';
 import { Observable } from 'rxjs';
-import { CoreStart, DocLinksStart, HttpStart } from 'src/core/public';
+import { AppMountParameters, DocLinksStart, HttpStart } from 'src/core/public';
 import { mockApmPluginContextValue } from '../context/apm_plugin/mock_apm_plugin_context';
 import { createCallApmApi } from '../services/rest/createCallApmApi';
-import { renderApp } from './';
+import { renderApp as renderApmApp } from './';
+import { UXAppRoot } from './uxApp';
 import { disableConsoleWarning } from '../utils/testHelpers';
 import { dataPluginMock } from 'src/plugins/data/public/mocks';
 import { embeddablePluginMock } from 'src/plugins/embeddable/public/mocks';
-import { ApmPluginStartDeps } from '../plugin';
+import { ApmPluginSetupDeps, ApmPluginStartDeps } from '../plugin';
+import { RumHome } from '../components/app/RumDashboard/RumHome';
 
 jest.mock('../services/rest/data_view', () => ({
   createStaticDataView: () => Promise.resolve(undefined),
 }));
 
-describe('renderApp', () => {
-  let mockConsole: jest.SpyInstance;
+jest.mock('../components/app/RumDashboard/RumHome', () => ({
+  RumHome: () => <p>Home Mock</p>,
+}));
 
+describe('renderApp (APM)', () => {
+  let mockConsole: jest.SpyInstance;
   beforeAll(() => {
     // The RUM agent logs an unnecessary message here. There's a couple open
     // issues need to be fixed to get the ability to turn off all of the logging:
@@ -40,11 +47,15 @@ describe('renderApp', () => {
     mockConsole.mockRestore();
   });
 
-  it('renders the app', () => {
-    const { core, config, observabilityRuleTypeRegistry } =
-      mockApmPluginContextValue;
+  const getApmMountProps = () => {
+    const {
+      core: coreStart,
+      config,
+      observabilityRuleTypeRegistry,
+      corePlugins,
+    } = mockApmPluginContextValue;
 
-    const plugins = {
+    const pluginsSetup = {
       licensing: { license$: new Observable() },
       triggersActionsUi: { actionTypeRegistry: {}, ruleTypeRegistry: {} },
       data: {
@@ -99,7 +110,7 @@ describe('renderApp', () => {
     } as unknown as ApmPluginStartDeps;
 
     jest.spyOn(window, 'scrollTo').mockReturnValueOnce(undefined);
-    createCallApmApi(core as unknown as CoreStart);
+    createCallApmApi(coreStart);
 
     jest
       .spyOn(window.console, 'warn')
@@ -111,21 +122,46 @@ describe('renderApp', () => {
         }
       });
 
+    return {
+      coreStart,
+      pluginsSetup: pluginsSetup as unknown as ApmPluginSetupDeps,
+      appMountParameters: appMountParameters as unknown as AppMountParameters,
+      pluginsStart,
+      config,
+      observabilityRuleTypeRegistry,
+      corePlugins,
+    };
+  };
+
+  it('renders the app', () => {
+    const mountProps = getApmMountProps();
+
     let unmount: () => void;
 
     act(() => {
-      unmount = renderApp({
-        coreStart: core as any,
-        pluginsSetup: plugins as any,
-        appMountParameters: appMountParameters as any,
-        pluginsStart,
-        config,
-        observabilityRuleTypeRegistry,
-      });
+      unmount = renderApmApp(mountProps);
     });
 
     expect(() => {
       unmount();
     }).not.toThrowError();
   });
 });
+
+describe('renderUxApp', () => {
+  it('has an error boundary for the UXAppRoot', async () => {
+    const uxMountProps = mockApmPluginContextValue;
+
+    const wrapper = mount(<UXAppRoot {...(uxMountProps as any)} />);
+
+    wrapper
+      .find(RumHome)
+      .simulateError(new Error('Oh no, an unexpected error!'));
+
+    expect(wrapper.find(RumHome)).toHaveLength(0);
+    expect(wrapper.find(EuiErrorBoundary)).toHaveLength(1);
+    expect(wrapper.find(EuiErrorBoundary).text()).toMatch(
+      /Error: Oh no, an unexpected error!/
+    );
+  });
+});
diff --git a/x-pack/plugins/apm/public/application/uxApp.tsx b/x-pack/plugins/apm/public/application/uxApp.tsx
@@ -7,6 +7,7 @@
 
 import euiDarkVars from '@elastic/eui/dist/eui_theme_dark.json';
 import euiLightVars from '@elastic/eui/dist/eui_theme_light.json';
+import { EuiErrorBoundary } from '@elastic/eui';
 import { AppMountParameters, CoreStart } from 'kibana/public';
 import React from 'react';
 import ReactDOM from 'react-dom';
@@ -133,7 +134,9 @@ export function UXAppRoot({
             <RouterProvider history={history} router={uxRouter}>
               <InspectorContextProvider>
                 <UrlParamsProvider>
-                  <UxApp />
+                  <EuiErrorBoundary>
+                    <UxApp />
+                  </EuiErrorBoundary>
                   <UXActionMenu appMountParameters={appMountParameters} />
                 </UrlParamsProvider>
               </InspectorContextProvider>

diff --git a/x-pack/plugins/apm/public/context/apm_plugin/mock_apm_plugin_context.tsx b/x-pack/plugins/apm/public/context/apm_plugin/mock_apm_plugin_context.tsx
@@ -6,11 +6,11 @@
  */
 
 import React, { ReactNode, useMemo } from 'react';
-import { Observable, of } from 'rxjs';
 import { RouterProvider } from '@kbn/typed-react-router-config';
 import { useHistory } from 'react-router-dom';
 import { createMemoryHistory, History } from 'history';
 import { merge } from 'lodash';
+import { coreMock } from '../../../../../../src/core/public/mocks';
 import { UrlService } from '../../../../../../src/plugins/share/common/url_service';
 import { createObservabilityRuleTypeRegistryMock } from '../../../../observability/public';
 import { ApmPluginContext, ApmPluginContextValue } from './apm_plugin_context';
@@ -20,72 +20,43 @@ import { createCallApmApi } from '../../services/rest/createCallApmApi';
 import { apmRouter } from '../../components/routing/apm_route_config';
 import { MlLocatorDefinition } from '../../../../ml/public';
 
-const uiSettings: Record<string, unknown> = {
-  [UI_SETTINGS.TIMEPICKER_QUICK_RANGES]: [
-    {
-      from: 'now/d',
-      to: 'now/d',
-      display: 'Today',
-    },
-    {
-      from: 'now/w',
-      to: 'now/w',
-      display: 'This week',
-    },
-  ],
-  [UI_SETTINGS.TIMEPICKER_TIME_DEFAULTS]: {
-    from: 'now-15m',
-    to: 'now',
-  },
-  [UI_SETTINGS.TIMEPICKER_REFRESH_INTERVAL_DEFAULTS]: {
-    pause: false,
-    value: 100000,
-  },
-};
+const coreStart = coreMock.createStart({ basePath: '/basepath' });
 
-const mockCore = {
+const mockCore = merge({}, coreStart, {
   application: {
     capabilities: {
       apm: {},
       ml: {},
     },
-    currentAppId$: new Observable(),
-    getUrlForApp: (appId: string) => '',
-    navigateToUrl: (url: string) => {},
-  },
-  chrome: {
-    docTitle: { change: () => {} },
-    setBreadcrumbs: () => {},
-    setHelpExtension: () => {},
-    setBadge: () => {},
-  },
-  docLinks: {
-    DOC_LINK_VERSION: '0',
-    ELASTIC_WEBSITE_URL: 'https://www.elastic.co/',
-    links: {
-      apm: {},
-    },
-  },
-  http: {
-    basePath: {
-      prepend: (path: string) => `/basepath${path}`,
-      get: () => `/basepath`,
-    },
-  },
-  i18n: {
-    Context: ({ children }: { children: ReactNode }) => children,
-  },
-  notifications: {
-    toasts: {
-      addWarning: () => {},
-      addDanger: () => {},
-    },
   },
   uiSettings: {
-    get: (key: string) => uiSettings[key],
-    get$: (key: string) => of(mockCore.uiSettings.get(key)),
+    get: (key: string) => {
+      const uiSettings: Record<string, unknown> = {
+        [UI_SETTINGS.TIMEPICKER_QUICK_RANGES]: [
+          {
+            from: 'now/d',
+            to: 'now/d',
+            display: 'Today',
+          },
+          {
+            from: 'now/w',
+            to: 'now/w',
+            display: 'This week',
+          },
+        ],
+        [UI_SETTINGS.TIMEPICKER_TIME_DEFAULTS]: {
+          from: 'now-15m',
+          to: 'now',
+        },
+        [UI_SETTINGS.TIMEPICKER_REFRESH_INTERVAL_DEFAULTS]: {
+          pause: false,
+          value: 100000,
+        },
+      };
+      return uiSettings[key];
+    },
   },
-};
+});
 
 const mockConfig: ConfigSchema = {
   serviceMapEnabled: true,
@@ -118,24 +89,30 @@ const mockPlugin = {
   },
 };
 
-const mockAppMountParameters = {
-  setHeaderActionMenu: () => {},
+const mockCorePlugins = {
+  embeddable: {},
+  inspector: {},
+  maps: {},
+  observability: {},
+  data: {},
 };
 
 export const mockApmPluginContextValue = {
-  appMountParameters: mockAppMountParameters,
+  appMountParameters: coreMock.createAppMountParameters('/basepath'),
   config: mockConfig,
   core: mockCore,
   plugins: mockPlugin,
   observabilityRuleTypeRegistry: createObservabilityRuleTypeRegistryMock(),
+  corePlugins: mockCorePlugins,
+  deps: {},
 };
 
 export function MockApmPluginContextWrapper({
   children,
   value = {} as ApmPluginContextValue,
   history,
 }: {
-  children?: React.ReactNode;
+  children?: ReactNode;
   value?: ApmPluginContextValue;
   history?: History;
 }) {

diff --git a/x-pack/plugins/enterprise_search/public/applications/index.test.tsx b/x-pack/plugins/enterprise_search/public/applications/index.test.tsx
@@ -23,7 +23,7 @@ import { renderApp, renderHeaderActions } from './';
 
 describe('renderApp', () => {
   const kibanaDeps = {
-    params: coreMock.createAppMountParamters(),
+    params: coreMock.createAppMountParameters(),
     core: coreMock.createStart(),
     plugins: {
       licensing: licensingMock.createStart(),

diff --git a/x-pack/plugins/security/public/authentication/capture_url/capture_url_app.test.ts b/x-pack/plugins/security/public/authentication/capture_url/capture_url_app.test.ts
@@ -59,7 +59,7 @@ describe('captureURLApp', () => {
     captureURLApp.create(coreSetupMock);
 
     const [[{ mount }]] = coreSetupMock.application.register.mock.calls;
-    await mount(coreMock.createAppMountParamters());
+    await mount(coreMock.createAppMountParameters());
 
     expect(mockLocationReplace).toHaveBeenCalledTimes(1);
     expect(mockLocationReplace).toHaveBeenCalledWith(
@@ -77,7 +77,7 @@ describe('captureURLApp', () => {
     captureURLApp.create(coreSetupMock);
 
     const [[{ mount }]] = coreSetupMock.application.register.mock.calls;
-    await mount(coreMock.createAppMountParamters());
+    await mount(coreMock.createAppMountParameters());
 
     expect(mockLocationReplace).toHaveBeenCalledTimes(1);
     expect(mockLocationReplace).toHaveBeenCalledWith(