Skip to content

Commit

Permalink
[Reporting] Add new data-render-error attribute (#114472) (#114792)
Browse files Browse the repository at this point in the history
* added new data-render-error attribute, read it and store it on job object

* added data-render-error to the example app

* added jest test

* address pr feedback

- make renderErrors optional in interfaces
- create separate selectors for data render error selector/attr
- Tidy up mergeMap behaviour

* fix observable.test.ts snapshots and browser driver mock

* updated jest snapshots

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
jloleysens and kibanamachine committed Oct 13, 2021
1 parent 7a9a427 commit 3fea97b
Show file tree
Hide file tree
Showing 14 changed files with 191 additions and 4 deletions.
7 changes: 6 additions & 1 deletion x-pack/examples/reporting_example/public/components/app.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,12 @@ export const ReportingExampleApp = ({
)}
</EuiFlexItem>
{logos.map((item, index) => (
<EuiFlexItem key={index} data-shared-item>
<EuiFlexItem
key={index}
data-shared-item
data-shared-render-error
data-render-error="This is an example error"
>
<EuiCard
icon={<EuiIcon size="xxl" type={`logo${item}`} />}
title={`Elastic ${item}`}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@ export async function generatePngObservableFactory(reporting: ReportingCore) {
if (current.error) {
found.push(current.error.message);
}
if (current.renderErrors) {
found.push(...current.renderErrors);
}
return found;
}, [] as string[]),
})),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,9 @@ export async function generatePdfObservableFactory(reporting: ReportingCore) {
if (current.error) {
found.push(current.error.message);
}
if (current.renderErrors) {
found.push(...current.renderErrors);
}
return found;
}, [] as string[]),
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,9 @@ export async function generatePdfObservableFactory(reporting: ReportingCore) {
if (current.error) {
found.push(current.error.message);
}
if (current.renderErrors) {
found.push(...current.renderErrors);
}
return found;
}, [] as string[]),
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ describe('Create Layout', () => {
"selectors": Object {
"itemsCountAttribute": "data-shared-items-count",
"renderComplete": "[data-shared-item]",
"renderError": "[data-render-error]",
"renderErrorAttribute": "data-render-error",
"screenshot": "[data-shared-items-container]",
"timefilterDurationAttribute": "data-shared-timefilter-duration",
},
Expand Down Expand Up @@ -63,6 +65,8 @@ describe('Create Layout', () => {
"selectors": Object {
"itemsCountAttribute": "data-shared-items-count",
"renderComplete": "[data-shared-item]",
"renderError": "[data-render-error]",
"renderErrorAttribute": "data-render-error",
"screenshot": "[data-shared-item]",
"timefilterDurationAttribute": "data-shared-timefilter-duration",
},
Expand All @@ -87,6 +91,8 @@ describe('Create Layout', () => {
"selectors": Object {
"itemsCountAttribute": "data-shared-items-count",
"renderComplete": "[data-shared-item]",
"renderError": "[data-render-error]",
"renderErrorAttribute": "data-render-error",
"screenshot": "[data-shared-items-container]",
"timefilterDurationAttribute": "data-shared-timefilter-duration",
},
Expand Down
4 changes: 4 additions & 0 deletions x-pack/plugins/reporting/server/lib/layouts/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import type { Layout } from './layout';
export interface LayoutSelectorDictionary {
screenshot: string;
renderComplete: string;
renderError: string;
renderErrorAttribute: string;
itemsCountAttribute: string;
timefilterDurationAttribute: string;
}
Expand All @@ -33,6 +35,8 @@ export const LayoutTypes = {
export const getDefaultLayoutSelectors = (): LayoutSelectorDictionary => ({
screenshot: '[data-shared-items-container]',
renderComplete: '[data-shared-item]',
renderError: '[data-render-error]',
renderErrorAttribute: 'data-render-error',
itemsCountAttribute: 'data-shared-items-count',
timefilterDurationAttribute: 'data-shared-timefilter-duration',
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ export const DEFAULT_PAGELOAD_SELECTOR = `.${APP_WRAPPER_CLASS}`;
export const CONTEXT_GETNUMBEROFITEMS = 'GetNumberOfItems';
export const CONTEXT_INJECTCSS = 'InjectCss';
export const CONTEXT_WAITFORRENDER = 'WaitForRender';
export const CONTEXT_GETRENDERERRORS = 'GetVisualisationsRenderErrors';
export const CONTEXT_GETTIMERANGE = 'GetTimeRange';
export const CONTEXT_ELEMENTATTRIBUTES = 'ElementPositionAndAttributes';
export const CONTEXT_WAITFORELEMENTSTOBEINDOM = 'WaitForElementsToBeInDOM';
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { HeadlessChromiumDriver } from '../../browsers';
import {
createMockBrowserDriverFactory,
createMockConfig,
createMockConfigSchema,
createMockLayoutInstance,
createMockLevelLogger,
createMockReportingCore,
} from '../../test_helpers';
import { CaptureConfig } from '../../types';
import { LayoutInstance } from '../layouts';
import { LevelLogger } from '../level_logger';
import { getRenderErrors } from './get_render_errors';

describe('getRenderErrors', () => {
let captureConfig: CaptureConfig;
let layout: LayoutInstance;
let logger: jest.Mocked<LevelLogger>;
let browser: HeadlessChromiumDriver;

beforeEach(async () => {
const schema = createMockConfigSchema();
const config = createMockConfig(schema);
const core = await createMockReportingCore(schema);

captureConfig = config.get('capture');
layout = createMockLayoutInstance(captureConfig);
logger = createMockLevelLogger();

await createMockBrowserDriverFactory(core, logger, {
evaluate: jest.fn(
async <T extends (...args: unknown[]) => unknown>({
fn,
args,
}: {
fn: T;
args: Parameters<T>;
}) => fn(...args)
),
getCreatePage: (driver) => {
browser = driver;

return jest.fn();
},
});
});

afterEach(() => {
document.body.innerHTML = '';
});

it('should extract the error messages', async () => {
document.body.innerHTML = `
<div dataRenderErrorSelector="a test error" />
<div dataRenderErrorSelector="a test error" />
<div dataRenderErrorSelector="a test error" />
<div dataRenderErrorSelector="a test error" />
`;

await expect(getRenderErrors(browser, layout, logger)).resolves.toEqual([
'a test error',
'a test error',
'a test error',
'a test error',
]);
});

it('should extract the error messages, even when there are none', async () => {
document.body.innerHTML = `
<renderedSelector />
`;

await expect(getRenderErrors(browser, layout, logger)).resolves.toEqual(undefined);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { i18n } from '@kbn/i18n';
import type { HeadlessChromiumDriver } from '../../browsers';
import type { LayoutInstance } from '../layouts';
import { LevelLogger, startTrace } from '../';
import { CONTEXT_GETRENDERERRORS } from './constants';

export const getRenderErrors = async (
browser: HeadlessChromiumDriver,
layout: LayoutInstance,
logger: LevelLogger
): Promise<undefined | string[]> => {
const endTrace = startTrace('get_render_errors', 'read');
logger.debug('reading render errors');
const errorsFound: undefined | string[] = await browser.evaluate(
{
fn: (errorSelector, errorAttribute) => {
const visualizations: Element[] = Array.from(document.querySelectorAll(errorSelector));
const errors: string[] = [];

visualizations.forEach((visualization) => {
const errorMessage = visualization.getAttribute(errorAttribute);
if (errorMessage) {
errors.push(errorMessage);
}
});

return errors.length ? errors : undefined;
},
args: [layout.selectors.renderError, layout.selectors.renderErrorAttribute],
},
{ context: CONTEXT_GETRENDERERRORS },
logger
);
endTrace();

if (errorsFound?.length) {
logger.warning(
i18n.translate('xpack.reporting.screencapture.renderErrorsFound', {
defaultMessage: 'Found {count} error messages. See report object for more information.',
values: { count: errorsFound.length },
})
);
}

return errorsFound;
};
7 changes: 7 additions & 0 deletions x-pack/plugins/reporting/server/lib/screenshots/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,5 +53,12 @@ export interface ScreenshotResults {
timeRange: string | null;
screenshots: Screenshot[];
error?: Error;

/**
* Individual visualizations might encounter errors at runtime. If there are any they are added to this
* field. Any text captured here is intended to be shown to the user for debugging purposes, reporting
* does no further sanitization on these strings.
*/
renderErrors?: string[];
elementsPositionAndAttributes?: ElementsPositionAndAttribute[]; // NOTE: for testing
}
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ describe('Screenshot Observable Pipeline', () => {
},
],
"error": undefined,
"renderErrors": undefined,
"screenshots": Array [
Object {
"data": Object {
Expand Down Expand Up @@ -172,6 +173,7 @@ describe('Screenshot Observable Pipeline', () => {
},
],
"error": undefined,
"renderErrors": undefined,
"screenshots": Array [
Object {
"data": Object {
Expand Down Expand Up @@ -223,6 +225,7 @@ describe('Screenshot Observable Pipeline', () => {
},
],
"error": undefined,
"renderErrors": undefined,
"screenshots": Array [
Object {
"data": Object {
Expand Down Expand Up @@ -312,6 +315,7 @@ describe('Screenshot Observable Pipeline', () => {
},
],
"error": [Error: An error occurred when trying to read the page for visualization panel info. You may need to increase 'xpack.reporting.capture.timeouts.waitForElements'. Error: Mock error!],
"renderErrors": undefined,
"screenshots": Array [
Object {
"data": Object {
Expand Down Expand Up @@ -354,6 +358,7 @@ describe('Screenshot Observable Pipeline', () => {
},
],
"error": [Error: An error occurred when trying to read the page for visualization panel info. You may need to increase 'xpack.reporting.capture.timeouts.waitForElements'. Error: Mock error!],
"renderErrors": undefined,
"screenshots": Array [
Object {
"data": Object {
Expand Down Expand Up @@ -460,6 +465,7 @@ describe('Screenshot Observable Pipeline', () => {
},
],
"error": undefined,
"renderErrors": undefined,
"screenshots": Array [
Object {
"data": Object {
Expand Down
15 changes: 12 additions & 3 deletions x-pack/plugins/reporting/server/lib/screenshots/observable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { getElementPositionAndAttributes } from './get_element_position_data';
import { getNumberOfItems } from './get_number_of_items';
import { getScreenshots } from './get_screenshots';
import { getTimeRange } from './get_time_range';
import { getRenderErrors } from './get_render_errors';
import { injectCustomCss } from './inject_css';
import { openUrl } from './open_url';
import { waitForRenderComplete } from './wait_for_render';
Expand All @@ -28,6 +29,7 @@ const DEFAULT_SCREENSHOT_CLIP_WIDTH = 1800;
interface ScreenSetupData {
elementsPositionAndAttributes: ElementsPositionAndAttribute[] | null;
timeRange: string | null;
renderErrors?: string[];
error?: Error;
}

Expand Down Expand Up @@ -101,16 +103,22 @@ export function getScreenshots$(
return await Promise.all([
getTimeRange(driver, layout, logger),
getElementPositionAndAttributes(driver, layout, logger),
]).then(([timeRange, elementsPositionAndAttributes]) => ({
getRenderErrors(driver, layout, logger),
]).then(([timeRange, elementsPositionAndAttributes, renderErrors]) => ({
elementsPositionAndAttributes,
timeRange,
renderErrors,
}));
}),
catchError((err) => {
checkPageIsOpen(driver); // if browser has closed, throw a relevant error about it

logger.error(err);
return Rx.of({ elementsPositionAndAttributes: null, timeRange: null, error: err });
return Rx.of({
elementsPositionAndAttributes: null,
timeRange: null,
error: err,
});
})
);

Expand All @@ -123,11 +131,12 @@ export function getScreenshots$(
? data.elementsPositionAndAttributes
: getDefaultElementPosition(layout.getViewport(1));
const screenshots = await getScreenshots(driver, elements, logger);
const { timeRange, error: setupError } = data;
const { timeRange, error: setupError, renderErrors } = data;
return {
timeRange,
screenshots,
error: setupError,
renderErrors,
elementsPositionAndAttributes: elements,
};
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,9 @@ mockBrowserEvaluate.mockImplementation(() => {
if (mockCall === contexts.CONTEXT_ELEMENTATTRIBUTES) {
return Promise.resolve(getMockElementsPositionAndAttributes('Default Mock Title', 'Default '));
}
if (mockCall === contexts.CONTEXT_GETRENDERERRORS) {
return Promise.resolve();
}
throw new Error(mockCall);
});
const mockScreenshot = jest.fn(async () => Buffer.from('screenshot'));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ export const createMockLayoutInstance = (captureConfig: CaptureConfig) => {
renderComplete: 'renderedSelector',
itemsCountAttribute: 'itemsSelector',
screenshot: 'screenshotSelector',
renderError: '[dataRenderErrorSelector]',
renderErrorAttribute: 'dataRenderErrorSelector',
timefilterDurationAttribute: 'timefilterDurationSelector',
};
return mockLayout;
Expand Down

0 comments on commit 3fea97b

Please sign in to comment.