Skip to content

Commit

Permalink
use the openUrl timeout as the default for navigation
Browse files Browse the repository at this point in the history
  • Loading branch information
tsullivan committed Mar 5, 2020
1 parent 99066b2 commit 926b694
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 42 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import * as Rx from 'rxjs';
import { ignoreElements, map, mergeMap, tap } from 'rxjs/operators';
import { InnerSubscriber } from 'rxjs/internal/InnerSubscriber';

import { BrowserConfig, NetworkPolicy } from '../../../../types';
import { BrowserConfig, CaptureConfig } from '../../../../types';
import { LevelLogger as Logger } from '../../../lib/level_logger';
import { HeadlessChromiumDriver } from '../driver';
import { safeChildProcess } from '../../safe_child_process';
Expand All @@ -27,30 +27,27 @@ import { getChromeLogLocation } from '../paths';
import { args } from './args';

type binaryPath = string;
type queueTimeout = number;
type ViewportConfig = BrowserConfig['viewport'];

export class HeadlessChromiumDriverFactory {
private binaryPath: binaryPath;
private captureConfig: CaptureConfig;
private browserConfig: BrowserConfig;
private queueTimeout: queueTimeout;
private networkPolicy: NetworkPolicy;
private userDataDir: string;
private getChromiumArgs: (viewport: BrowserConfig['viewport']) => string[];
private getChromiumArgs: (viewport: ViewportConfig) => string[];

constructor(
binaryPath: binaryPath,
logger: Logger,
browserConfig: BrowserConfig,
queueTimeout: queueTimeout,
networkPolicy: NetworkPolicy
captureConfig: CaptureConfig
) {
this.binaryPath = binaryPath;
this.browserConfig = browserConfig;
this.queueTimeout = queueTimeout;
this.networkPolicy = networkPolicy;
this.captureConfig = captureConfig;

this.userDataDir = fs.mkdtempSync(path.join(os.tmpdir(), 'chromium-'));
this.getChromiumArgs = (viewport: BrowserConfig['viewport']) =>
this.getChromiumArgs = (viewport: ViewportConfig) =>
args({
userDataDir: this.userDataDir,
viewport,
Expand Down Expand Up @@ -88,7 +85,7 @@ export class HeadlessChromiumDriverFactory {
* Return an observable to objects which will drive screenshot capture for a page
*/
createPage(
{ viewport, browserTimezone }: { viewport: BrowserConfig['viewport']; browserTimezone: string },
{ viewport, browserTimezone }: { viewport: ViewportConfig; browserTimezone: string },
pLogger: Logger
): Rx.Observable<{ driver: HeadlessChromiumDriver; exit$: Rx.Observable<never> }> {
return Rx.Observable.create(async (observer: InnerSubscriber<any, any>) => {
Expand All @@ -113,11 +110,9 @@ export class HeadlessChromiumDriverFactory {

page = await browser.newPage();

// All navigation/waitFor methods default to 30 seconds,
// which can cause the job to fail even if we bump timeouts in
// the config. Help alleviate errors like
// "TimeoutError: waiting for selector ".application" failed: timeout 30000ms exceeded"
page.setDefaultTimeout(this.queueTimeout);
// All navigation methods default to 30 seconds,
// All waitFor methods have their own timeout config
page.setDefaultTimeout(this.captureConfig.timeouts.openUrl);

logger.debug(`Browser page driver created`);
} catch (err) {
Expand Down Expand Up @@ -158,7 +153,7 @@ export class HeadlessChromiumDriverFactory {
// HeadlessChromiumDriver: object to "drive" a browser page
const driver = new HeadlessChromiumDriver(page, {
inspect: this.browserConfig.inspect,
networkPolicy: this.networkPolicy,
networkPolicy: this.captureConfig.networkPolicy,
});

// Rx.Observable<never>: stream to interrupt page capture
Expand Down
13 changes: 3 additions & 10 deletions x-pack/legacy/plugins/reporting/server/browsers/chromium/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { BrowserConfig, NetworkPolicy } from '../../../types';
import { BrowserConfig, CaptureConfig } from '../../../types';
import { LevelLogger } from '../../lib';
import { HeadlessChromiumDriverFactory } from './driver_factory';

Expand All @@ -14,14 +14,7 @@ export async function createDriverFactory(
binaryPath: string,
logger: LevelLogger,
browserConfig: BrowserConfig,
queueTimeout: number,
networkPolicy: NetworkPolicy
captureConfig: CaptureConfig
): Promise<HeadlessChromiumDriverFactory> {
return new HeadlessChromiumDriverFactory(
binaryPath,
logger,
browserConfig,
queueTimeout,
networkPolicy
);
return new HeadlessChromiumDriverFactory(binaryPath, logger, browserConfig, captureConfig);
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@ export async function createBrowserDriverFactory(
const browserType = captureConfig.browser.type;
const browserAutoDownload = captureConfig.browser.autoDownload;
const browserConfig = captureConfig.browser[BROWSER_TYPE];
const networkPolicy = captureConfig.networkPolicy;
const reportingTimeout: number = config.get('xpack.reporting.queue.timeout');

if (browserConfig.disableSandbox) {
logger.warning(`Enabling the Chromium sandbox provides an additional layer of protection.`);
Expand All @@ -34,13 +32,7 @@ export async function createBrowserDriverFactory(

try {
const { binaryPath } = await installBrowser(logger, chromium, dataDir);
return chromium.createDriverFactory(
binaryPath,
logger,
browserConfig,
reportingTimeout,
networkPolicy
);
return chromium.createDriverFactory(binaryPath, logger, browserConfig, captureConfig);
} catch (error) {
if (error.cause && ['EACCES', 'EEXIST'].includes(error.cause.code)) {
logger.error(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import * as contexts from '../export_types/common/lib/screenshots/constants';
import { ElementsPositionAndAttribute } from '../export_types/common/lib/screenshots/types';
import { HeadlessChromiumDriver, HeadlessChromiumDriverFactory } from '../server/browsers';
import { createDriverFactory } from '../server/browsers/chromium';
import { BrowserConfig, Logger, NetworkPolicy } from '../types';
import { BrowserConfig, CaptureConfig, Logger } from '../types';

interface CreateMockBrowserDriverFactoryOpts {
evaluate: jest.Mock<Promise<any>, any[]>;
Expand Down Expand Up @@ -102,19 +102,20 @@ export const createMockBrowserDriverFactory = async (
} as BrowserConfig;

const binaryPath = '/usr/local/share/common/secure/';
const queueTimeout = 55;
const networkPolicy = {} as NetworkPolicy;
const captureConfig = { networkPolicy: {}, timeouts: {} } as CaptureConfig;

const mockBrowserDriverFactory = await createDriverFactory(
binaryPath,
logger,
browserConfig,
queueTimeout,
networkPolicy
captureConfig
);

const mockPage = {} as Page;
const mockBrowserDriver = new HeadlessChromiumDriver(mockPage, { inspect: true, networkPolicy });
const mockBrowserDriver = new HeadlessChromiumDriver(mockPage, {
inspect: true,
networkPolicy: captureConfig.networkPolicy,
});

// mock the driver methods as either default mocks or passed-in
mockBrowserDriver.waitForSelector = opts.waitForSelector ? opts.waitForSelector : defaultOpts.waitForSelector; // prettier-ignore
Expand Down
5 changes: 5 additions & 0 deletions x-pack/legacy/plugins/reporting/types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,11 @@ export interface CaptureConfig {
maxAttempts: number;
networkPolicy: NetworkPolicy;
loadDelay: number;
timeouts: {
openUrl: number;
waitForElements: number;
renderComplet: number;
};
}

export interface BrowserConfig {
Expand Down

0 comments on commit 926b694

Please sign in to comment.