diff --git a/x-pack/plugins/reporting/server/browsers/chromium/driver/index.js b/x-pack/plugins/reporting/server/browsers/chromium/driver/index.js index 8724473496f0d..b62dd313fe402 100644 --- a/x-pack/plugins/reporting/server/browsers/chromium/driver/index.js +++ b/x-pack/plugins/reporting/server/browsers/chromium/driver/index.js @@ -10,7 +10,7 @@ import moment from 'moment'; import { promisify, delay } from 'bluebird'; import { transformFn } from './transform_fn'; import { ignoreSSLErrorsBehavior } from './ignore_ssl_errors'; -import { screenshotStitcher } from './screenshot_stitcher'; +import { screenshotStitcher, CapturePngSizeError } from './screenshot_stitcher'; export class HeadlessChromiumDriver { constructor(client, { maxScreenshotDimension, logger }) { @@ -84,16 +84,34 @@ export class HeadlessChromiumDriver { }; } - return await screenshotStitcher(outputClip, this._zoom, this._maxScreenshotDimension, async screenshotClip => { - const { data } = await Page.captureScreenshot({ - clip: { - ...screenshotClip, - scale: 1 + // Wrapping screenshotStitcher function call in a retry because of this bug: + // https://github.com/elastic/kibana/issues/19563. The reason was never found - it only appeared on ci and + // debug logic right after Page.captureScreenshot to ensure the correct size made the bug disappear. + let retryCount = 0; + const MAX_RETRIES = 3; + while (true) { + try { + return await screenshotStitcher(outputClip, this._zoom, this._maxScreenshotDimension, async screenshotClip => { + const { data } = await Page.captureScreenshot({ + clip: { + ...screenshotClip, + scale: 1 + } + }); + this._logger.debug(`Captured screenshot clip ${JSON.stringify(screenshotClip)}`); + return data; + }, this._logger); + } catch (error) { + const isCapturePngSizeError = error instanceof CapturePngSizeError; + if (!isCapturePngSizeError || retryCount === MAX_RETRIES) { + throw error; + } else { + this._logger.error(error.message); + this._logger.error('Trying again...'); + retryCount++; } - }); - this._logger.debug(`Captured screenshot clip ${JSON.stringify(screenshotClip)}`); - return data; - }, this._logger); + } + } } async _writeData(writePath, base64EncodedData) { diff --git a/x-pack/plugins/reporting/server/browsers/chromium/driver/screenshot_stitcher/combine.ts b/x-pack/plugins/reporting/server/browsers/chromium/driver/screenshot_stitcher/combine.ts index 02a147ab16338..cc901ab72de1d 100644 --- a/x-pack/plugins/reporting/server/browsers/chromium/driver/screenshot_stitcher/combine.ts +++ b/x-pack/plugins/reporting/server/browsers/chromium/driver/screenshot_stitcher/combine.ts @@ -13,6 +13,20 @@ import { ObservableInput } from 'rxjs'; import { map, mergeMap, reduce, switchMap, tap, toArray } from 'rxjs/operators'; import { Logger, Screenshot, Size } from './types'; +export class CapturePngSizeError extends Error { + constructor( + actualSize: { width: number; height: number }, + expectedSize: { width: number; height: number } + ) { + super(); + this.message = + `Capture PNG size error. Please visit https://github.com/elastic/kibana/issues/19563 to report this error. ` + + `Screenshot captured of size ${actualSize.width}x${ + actualSize.height + } was not of expected size ${expectedSize.width}x${expectedSize.height}`; + } +} + // if we're only given one screenshot, and it matches the given size // we're going to skip the combination and just use it const canUseFirstScreenshot = ( @@ -69,14 +83,9 @@ export function $combine( png.width !== screenshot.rectangle.width || png.height !== screenshot.rectangle.height ) { - const errorMessage = `Screenshot captured with width:${png.width} and height: ${ - png.height - }) is not of expected width: ${screenshot.rectangle.width} and height: ${ - screenshot.rectangle.height - }`; - - logger.error(errorMessage); - throw new Error(errorMessage); + const error = new CapturePngSizeError(png, screenshot.rectangle); + logger.error(error.message); + throw error; } return { screenshot, png }; } diff --git a/x-pack/plugins/reporting/server/browsers/chromium/driver/screenshot_stitcher/index.ts b/x-pack/plugins/reporting/server/browsers/chromium/driver/screenshot_stitcher/index.ts index 09c0d9e3bbf4d..cb127cfd7f26f 100644 --- a/x-pack/plugins/reporting/server/browsers/chromium/driver/screenshot_stitcher/index.ts +++ b/x-pack/plugins/reporting/server/browsers/chromium/driver/screenshot_stitcher/index.ts @@ -4,88 +4,5 @@ * you may not use this file except in compliance with the Elastic License. */ -import { map, mergeMap, switchMap, toArray } from 'rxjs/operators'; -import { $combine } from './combine'; -import { $getClips } from './get_clips'; -import { Logger, Rectangle, Screenshot } from './types'; - -const scaleRect = (rect: Rectangle, scale: number): Rectangle => { - return { - height: rect.height * scale, - width: rect.width * scale, - x: rect.x * scale, - y: rect.y * scale, - }; -}; - -/** - * Returns a stream of data that should be of the size outputClip.width * zoom x outputClip.height * zoom - * @param outputClip - The dimensions the final image should take. - * @param zoom - Determines the resolution want the final screenshot to take. - * @param maxDimensionPerClip - The maximimum dimension, in any direction (width or height) that we should allow per - * screenshot clip. If zoom is 10 and maxDimensionPerClip is anything less than or - * equal to 10, each clip taken, before being zoomed in, should be no bigger than 1 x 1. - * If zoom is 10 and maxDimensionPerClip is 20, then each clip taken before being zoomed in should be no bigger than 2 x 2. - * @param captureScreenshotFn - a function to take a screenshot from the page using the dimensions given. The data - * returned should have dimensions not of the clip passed in, but of the clip passed in multiplied by zoom. - * @param logger - */ -export async function screenshotStitcher( - outputClip: Rectangle, - zoom: number, - maxDimensionPerClip: number, - captureScreenshotFn: (rect: Rectangle) => Promise, - logger: Logger -): Promise { - // We have to divide the max by the zoom because we will be multiplying each clip's dimensions - // later by zoom, and we don't want those dimensions to end up larger than max. - const maxDimensionBeforeZoom = Math.ceil(maxDimensionPerClip / zoom); - const screenshotClips$ = $getClips(outputClip, maxDimensionBeforeZoom); - - const screenshots$ = screenshotClips$.pipe( - mergeMap(clip => captureScreenshotFn(clip), (clip, data) => ({ clip, data }), 1) - ); - - // when we take the screenshots we don't have to scale the rects - // but the PNGs don't know about the zoom, so we have to scale them - const screenshotPngRects$ = screenshots$.pipe( - map(({ data, clip }) => { - // At this point we don't care about the offset - the screenshots have been taken. - // We need to adjust the x & y values so they all are adjusted for the top-left most - // clip being at 0, 0. - const x = clip.x - outputClip.x; - const y = clip.y - outputClip.y; - - const scaledScreenshotRects = scaleRect( - { - height: clip.height, - width: clip.width, - x, - y, - }, - zoom - ); - return { - data, - rectangle: scaledScreenshotRects, - }; - }) - ); - - const scaledOutputRects = scaleRect(outputClip, zoom); - return screenshotPngRects$ - .pipe( - toArray(), - switchMap((screenshots: Screenshot[]) => - $combine( - screenshots, - { - height: scaledOutputRects.height, - width: scaledOutputRects.width, - }, - logger - ) - ) - ) - .toPromise(); -} +export { screenshotStitcher } from './screenshot_stitcher'; +export { CapturePngSizeError } from './combine'; diff --git a/x-pack/plugins/reporting/server/browsers/chromium/driver/screenshot_stitcher/screenshot_stitcher.ts b/x-pack/plugins/reporting/server/browsers/chromium/driver/screenshot_stitcher/screenshot_stitcher.ts new file mode 100644 index 0000000000000..09c0d9e3bbf4d --- /dev/null +++ b/x-pack/plugins/reporting/server/browsers/chromium/driver/screenshot_stitcher/screenshot_stitcher.ts @@ -0,0 +1,91 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { map, mergeMap, switchMap, toArray } from 'rxjs/operators'; +import { $combine } from './combine'; +import { $getClips } from './get_clips'; +import { Logger, Rectangle, Screenshot } from './types'; + +const scaleRect = (rect: Rectangle, scale: number): Rectangle => { + return { + height: rect.height * scale, + width: rect.width * scale, + x: rect.x * scale, + y: rect.y * scale, + }; +}; + +/** + * Returns a stream of data that should be of the size outputClip.width * zoom x outputClip.height * zoom + * @param outputClip - The dimensions the final image should take. + * @param zoom - Determines the resolution want the final screenshot to take. + * @param maxDimensionPerClip - The maximimum dimension, in any direction (width or height) that we should allow per + * screenshot clip. If zoom is 10 and maxDimensionPerClip is anything less than or + * equal to 10, each clip taken, before being zoomed in, should be no bigger than 1 x 1. + * If zoom is 10 and maxDimensionPerClip is 20, then each clip taken before being zoomed in should be no bigger than 2 x 2. + * @param captureScreenshotFn - a function to take a screenshot from the page using the dimensions given. The data + * returned should have dimensions not of the clip passed in, but of the clip passed in multiplied by zoom. + * @param logger + */ +export async function screenshotStitcher( + outputClip: Rectangle, + zoom: number, + maxDimensionPerClip: number, + captureScreenshotFn: (rect: Rectangle) => Promise, + logger: Logger +): Promise { + // We have to divide the max by the zoom because we will be multiplying each clip's dimensions + // later by zoom, and we don't want those dimensions to end up larger than max. + const maxDimensionBeforeZoom = Math.ceil(maxDimensionPerClip / zoom); + const screenshotClips$ = $getClips(outputClip, maxDimensionBeforeZoom); + + const screenshots$ = screenshotClips$.pipe( + mergeMap(clip => captureScreenshotFn(clip), (clip, data) => ({ clip, data }), 1) + ); + + // when we take the screenshots we don't have to scale the rects + // but the PNGs don't know about the zoom, so we have to scale them + const screenshotPngRects$ = screenshots$.pipe( + map(({ data, clip }) => { + // At this point we don't care about the offset - the screenshots have been taken. + // We need to adjust the x & y values so they all are adjusted for the top-left most + // clip being at 0, 0. + const x = clip.x - outputClip.x; + const y = clip.y - outputClip.y; + + const scaledScreenshotRects = scaleRect( + { + height: clip.height, + width: clip.width, + x, + y, + }, + zoom + ); + return { + data, + rectangle: scaledScreenshotRects, + }; + }) + ); + + const scaledOutputRects = scaleRect(outputClip, zoom); + return screenshotPngRects$ + .pipe( + toArray(), + switchMap((screenshots: Screenshot[]) => + $combine( + screenshots, + { + height: scaledOutputRects.height, + width: scaledOutputRects.width, + }, + logger + ) + ) + ) + .toPromise(); +} diff --git a/x-pack/scripts/functional_tests.js b/x-pack/scripts/functional_tests.js index 0658878996ec2..e7e4579bbb2e5 100644 --- a/x-pack/scripts/functional_tests.js +++ b/x-pack/scripts/functional_tests.js @@ -6,9 +6,8 @@ require('@kbn/plugin-helpers').babelRegister(); require('@kbn/test').runTestsCli([ - // Uncomment when https://github.com/elastic/kibana/issues/19563 is resolved. - // require.resolve('../test/reporting/configs/chromium_api.js'), - // require.resolve('../test/reporting/configs/chromium_functional.js'), + require.resolve('../test/reporting/configs/chromium_api.js'), + require.resolve('../test/reporting/configs/chromium_functional.js'), require.resolve('../test/reporting/configs/phantom_api.js'), require.resolve('../test/reporting/configs/phantom_functional.js'), require.resolve('../test/functional/config.js'), diff --git a/x-pack/test/reporting/configs/chromium_api.js b/x-pack/test/reporting/configs/chromium_api.js index 9085505275575..9ebbfce0ac3ac 100644 --- a/x-pack/test/reporting/configs/chromium_api.js +++ b/x-pack/test/reporting/configs/chromium_api.js @@ -21,7 +21,6 @@ export default async function ({ readConfigFile }) { serverArgs: [ ...reportingApiConfig.kbnTestServer.serverArgs, `--xpack.reporting.capture.browser.type=chromium`, - `--logging.verbose=true`, ], }, }; diff --git a/x-pack/test/reporting/configs/chromium_functional.js b/x-pack/test/reporting/configs/chromium_functional.js index c76f2489d0cb0..f14d943746645 100644 --- a/x-pack/test/reporting/configs/chromium_functional.js +++ b/x-pack/test/reporting/configs/chromium_functional.js @@ -21,7 +21,6 @@ export default async function ({ readConfigFile }) { serverArgs: [ ...functionalConfig.kbnTestServer.serverArgs, `--xpack.reporting.capture.browser.type=chromium`, - `--logging.verbose=true`, ], }, };