[Reporting] Revisit handling timeouts for different phases of screens…

…hot capture (#113807) * [Reporting] Revisit handling timeouts for different phases of screenshot capture * remove translations for changed text * add wip unit test * simplify class * todo more testing * fix ts * update snapshots * simplify open_url * fixup me * move setupPage to a method of the ObservableHandler class * do not pass entire config object to helper functions * distinguish internal timeouts vs external timeout * add tests for waitUntil * checkIsPageOpen test * restore passing of renderErrors * updates per feedback * Update x-pack/plugins/reporting/server/lib/screenshots/observable_handler.ts Co-authored-by: Michael Dokolin <[email protected]> * Update x-pack/plugins/reporting/server/lib/screenshots/observable_handler.ts Co-authored-by: Michael Dokolin <[email protected]> * Update x-pack/plugins/reporting/server/lib/screenshots/observable_handler.ts Co-authored-by: Michael Dokolin <[email protected]> * Update x-pack/plugins/reporting/server/lib/screenshots/observable_handler.ts Co-authored-by: Michael Dokolin <[email protected]> * Update x-pack/plugins/reporting/server/lib/screenshots/observable_handler.ts Co-authored-by: Michael Dokolin <[email protected]> * fix parsing * apply simplifications consistently * dont main waitUntil a higher order component * resolve the timeouts options outside of the service * comment correction Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Michael Dokolin <[email protected]>
elastic · Oct 18, 2021 · 584d09e · 584d09e
1 parent fcf8662
commit 584d09e
Show file tree

Hide file tree

Showing 13 changed files with 465 additions and 206 deletions.
diff --git a/x-pack/plugins/reporting/server/lib/screenshots/check_browser_open.ts b/x-pack/plugins/reporting/server/lib/screenshots/check_browser_open.ts
diff --git a/x-pack/plugins/reporting/server/lib/screenshots/get_number_of_items.test.ts b/x-pack/plugins/reporting/server/lib/screenshots/get_number_of_items.test.ts
@@ -6,6 +6,7 @@
  */
 
 import { set } from 'lodash';
+import { durationToNumber } from '../../../common/schema_utils';
 import { HeadlessChromiumDriver } from '../../browsers';
 import {
   createMockBrowserDriverFactory,
@@ -25,6 +26,7 @@ describe('getNumberOfItems', () => {
   let layout: LayoutInstance;
   let logger: jest.Mocked<LevelLogger>;
   let browser: HeadlessChromiumDriver;
+  let timeout: number;
 
   beforeEach(async () => {
     const schema = createMockConfigSchema(set({}, 'capture.timeouts.waitForElements', 0));
@@ -34,6 +36,7 @@ describe('getNumberOfItems', () => {
     captureConfig = config.get('capture');
     layout = createMockLayoutInstance(captureConfig);
     logger = createMockLevelLogger();
+    timeout = durationToNumber(captureConfig.timeouts.waitForElements);
 
     await createMockBrowserDriverFactory(core, logger, {
       evaluate: jest.fn(
@@ -62,7 +65,7 @@ describe('getNumberOfItems', () => {
       <div itemsSelector="10" />
     `;
 
-    await expect(getNumberOfItems(captureConfig, browser, layout, logger)).resolves.toBe(10);
+    await expect(getNumberOfItems(timeout, browser, layout, logger)).resolves.toBe(10);
   });
 
   it('should determine the number of items by selector ', async () => {
@@ -72,7 +75,7 @@ describe('getNumberOfItems', () => {
       <renderedSelector />
     `;
 
-    await expect(getNumberOfItems(captureConfig, browser, layout, logger)).resolves.toBe(3);
+    await expect(getNumberOfItems(timeout, browser, layout, logger)).resolves.toBe(3);
   });
 
   it('should fall back to the selector when the attribute is empty', async () => {
@@ -82,6 +85,6 @@ describe('getNumberOfItems', () => {
       <renderedSelector />
     `;
 
-    await expect(getNumberOfItems(captureConfig, browser, layout, logger)).resolves.toBe(2);
+    await expect(getNumberOfItems(timeout, browser, layout, logger)).resolves.toBe(2);
   });
 });
diff --git a/x-pack/plugins/reporting/server/lib/screenshots/get_number_of_items.ts b/x-pack/plugins/reporting/server/lib/screenshots/get_number_of_items.ts
@@ -6,15 +6,13 @@
  */
 
 import { i18n } from '@kbn/i18n';
-import { durationToNumber } from '../../../common/schema_utils';
 import { LevelLogger, startTrace } from '../';
 import { HeadlessChromiumDriver } from '../../browsers';
-import { CaptureConfig } from '../../types';
 import { LayoutInstance } from '../layouts';
 import { CONTEXT_GETNUMBEROFITEMS, CONTEXT_READMETADATA } from './constants';
 
 export const getNumberOfItems = async (
-  captureConfig: CaptureConfig,
+  timeout: number,
   browser: HeadlessChromiumDriver,
   layout: LayoutInstance,
   logger: LevelLogger
@@ -33,7 +31,6 @@ export const getNumberOfItems = async (
     // the dashboard is using the `itemsCountAttribute` attribute to let us
     // know how many items to expect since gridster incrementally adds panels
     // we have to use this hint to wait for all of them
-    const timeout = durationToNumber(captureConfig.timeouts.waitForElements);
     await browser.waitForSelector(
       `${renderCompleteSelector},[${itemsCountAttribute}]`,
       { timeout },
@@ -65,11 +62,8 @@ export const getNumberOfItems = async (
     logger.error(err);
     throw new Error(
       i18n.translate('xpack.reporting.screencapture.readVisualizationsError', {
-        defaultMessage: `An error occurred when trying to read the page for visualization panel info. You may need to increase '{configKey}'. {error}`,
-        values: {
-          error: err,
-          configKey: 'xpack.reporting.capture.timeouts.waitForElements',
-        },
+        defaultMessage: `An error occurred when trying to read the page for visualization panel info: {error}`,
+        values: { error: err },
       })
     );
   }

diff --git a/x-pack/plugins/reporting/server/lib/screenshots/index.ts b/x-pack/plugins/reporting/server/lib/screenshots/index.ts
@@ -12,6 +12,19 @@ import { LayoutInstance } from '../layouts';
 
 export { getScreenshots$ } from './observable';
 
+export interface PhaseInstance {
+  timeoutValue: number;
+  configValue: string;
+  label: string;
+}
+
+export interface PhaseTimeouts {
+  openUrl: PhaseInstance;
+  waitForElements: PhaseInstance;
+  renderComplete: PhaseInstance;
+  loadDelay: number;
+}
+
 export interface ScreenshotObservableOpts {
   logger: LevelLogger;
   urlsOrUrlLocatorTuples: UrlOrUrlLocatorTuple[];
@@ -49,6 +62,12 @@ export interface Screenshot {
   description: string | null;
 }
 
+export interface PageSetupResults {
+  elementsPositionAndAttributes: ElementsPositionAndAttribute[] | null;
+  timeRange: string | null;
+  error?: Error;
+}
+
 export interface ScreenshotResults {
   timeRange: string | null;
   screenshots: Screenshot[];

diff --git a/x-pack/plugins/reporting/server/lib/screenshots/observable.test.ts b/x-pack/plugins/reporting/server/lib/screenshots/observable.test.ts
@@ -98,7 +98,6 @@ describe('Screenshot Observable Pipeline', () => {
             },
           ],
           "error": undefined,
-          "renderErrors": undefined,
           "screenshots": Array [
             Object {
               "data": Object {
@@ -173,7 +172,6 @@ describe('Screenshot Observable Pipeline', () => {
             },
           ],
           "error": undefined,
-          "renderErrors": undefined,
           "screenshots": Array [
             Object {
               "data": Object {
@@ -225,7 +223,6 @@ describe('Screenshot Observable Pipeline', () => {
             },
           ],
           "error": undefined,
-          "renderErrors": undefined,
           "screenshots": Array [
             Object {
               "data": Object {
@@ -314,8 +311,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,
+                  "error": [Error: The "wait for elements" phase encountered an error: Error: An error occurred when trying to read the page for visualization panel info: Error: Mock error!],
                   "screenshots": Array [
                     Object {
                       "data": Object {
@@ -357,8 +353,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,
+                  "error": [Error: An error occurred when trying to read the page for visualization panel info: Error: Mock error!],
                   "screenshots": Array [
                     Object {
                       "data": Object {
@@ -465,7 +460,6 @@ describe('Screenshot Observable Pipeline', () => {
               },
             ],
             "error": undefined,
-            "renderErrors": undefined,
             "screenshots": Array [
               Object {
                 "data": Object {