mirror of
https://github.com/elastic/kibana.git
synced 2025-04-24 09:48:58 -04:00
Retry the entire screenshotStitcher call (#20770)
* Retry the entire screenshotStitcher call * Go back to a single run * Only retry for this specific error. Post more information including the git issue link
This commit is contained in:
parent
30d4e70507
commit
0078e66f16
7 changed files with 140 additions and 108 deletions
|
@ -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) {
|
||||
|
|
|
@ -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 };
|
||||
}
|
||||
|
|
|
@ -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<string>,
|
||||
logger: Logger
|
||||
): Promise<string> {
|
||||
// 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<Screenshot[], string>((screenshots: Screenshot[]) =>
|
||||
$combine(
|
||||
screenshots,
|
||||
{
|
||||
height: scaledOutputRects.height,
|
||||
width: scaledOutputRects.width,
|
||||
},
|
||||
logger
|
||||
)
|
||||
)
|
||||
)
|
||||
.toPromise<string>();
|
||||
}
|
||||
export { screenshotStitcher } from './screenshot_stitcher';
|
||||
export { CapturePngSizeError } from './combine';
|
||||
|
|
|
@ -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<string>,
|
||||
logger: Logger
|
||||
): Promise<string> {
|
||||
// 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<Screenshot[], string>((screenshots: Screenshot[]) =>
|
||||
$combine(
|
||||
screenshots,
|
||||
{
|
||||
height: scaledOutputRects.height,
|
||||
width: scaledOutputRects.width,
|
||||
},
|
||||
logger
|
||||
)
|
||||
)
|
||||
)
|
||||
.toPromise<string>();
|
||||
}
|
|
@ -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'),
|
||||
|
|
|
@ -21,7 +21,6 @@ export default async function ({ readConfigFile }) {
|
|||
serverArgs: [
|
||||
...reportingApiConfig.kbnTestServer.serverArgs,
|
||||
`--xpack.reporting.capture.browser.type=chromium`,
|
||||
`--logging.verbose=true`,
|
||||
],
|
||||
},
|
||||
};
|
||||
|
|
|
@ -21,7 +21,6 @@ export default async function ({ readConfigFile }) {
|
|||
serverArgs: [
|
||||
...functionalConfig.kbnTestServer.serverArgs,
|
||||
`--xpack.reporting.capture.browser.type=chromium`,
|
||||
`--logging.verbose=true`,
|
||||
],
|
||||
},
|
||||
};
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue