Skip to content

Commit

Permalink
[Reporting] Fix unhandled promise rejection thrown when launching Chr…
Browse files Browse the repository at this point in the history
…omium (elastic#118119)

* [Reporting] Fix unhandled promise rejection thrown when launching Chromium

* comment correction

* simplify

* unit test

* Update x-pack/plugins/reporting/server/browsers/chromium/driver_factory/index.ts

Co-authored-by: Michael Dokolin <[email protected]>

Co-authored-by: Kibana Machine <[email protected]>
Co-authored-by: Michael Dokolin <[email protected]>
  • Loading branch information
3 people committed Nov 11, 2021
1 parent 26f499b commit 2aa6245
Show file tree
Hide file tree
Showing 2 changed files with 97 additions and 25 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
/*
* 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 puppeteer from 'puppeteer';
import * as Rx from 'rxjs';
import { take } from 'rxjs/operators';
import { HeadlessChromiumDriverFactory } from '.';
import type { ReportingCore } from '../../..';
import {
createMockConfigSchema,
createMockLevelLogger,
createMockReportingCore,
} from '../../../test_helpers';

jest.mock('puppeteer');

const mock = (browserDriverFactory: HeadlessChromiumDriverFactory) => {
browserDriverFactory.getBrowserLogger = jest.fn(() => new Rx.Observable());
browserDriverFactory.getProcessLogger = jest.fn(() => new Rx.Observable());
browserDriverFactory.getPageExit = jest.fn(() => new Rx.Observable());
return browserDriverFactory;
};

describe('class HeadlessChromiumDriverFactory', () => {
let reporting: ReportingCore;
const logger = createMockLevelLogger();
const path = 'path/to/headless_shell';

beforeEach(async () => {
(puppeteer as jest.Mocked<typeof puppeteer>).launch.mockResolvedValue({
newPage: jest.fn().mockResolvedValue({
target: jest.fn(() => ({
createCDPSession: jest.fn().mockResolvedValue({
send: jest.fn(),
}),
})),
emulateTimezone: jest.fn(),
setDefaultTimeout: jest.fn(),
}),
close: jest.fn(),
process: jest.fn(),
} as unknown as puppeteer.Browser);

reporting = await createMockReportingCore(
createMockConfigSchema({
capture: {
browser: { chromium: { proxy: {} } },
timeouts: { openUrl: 50000 },
},
})
);
});

it('createPage returns browser driver and process exit observable', async () => {
const factory = mock(new HeadlessChromiumDriverFactory(reporting, path, logger));
const utils = await factory.createPage({}).pipe(take(1)).toPromise();
expect(utils).toHaveProperty('driver');
expect(utils).toHaveProperty('exit$');
});

it('createPage rejects if Puppeteer launch fails', async () => {
(puppeteer as jest.Mocked<typeof puppeteer>).launch.mockRejectedValue(
`Puppeteer Launch mock fail.`
);
const factory = mock(new HeadlessChromiumDriverFactory(reporting, path, logger));
expect(() =>
factory.createPage({}).pipe(take(1)).toPromise()
).rejects.toThrowErrorMatchingInlineSnapshot(
`"Error spawning Chromium browser! Puppeteer Launch mock fail."`
);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import { LevelLogger } from '../../../lib';
import { safeChildProcess } from '../../safe_child_process';
import { HeadlessChromiumDriver } from '../driver';
import { args } from './args';
import { getMetrics, Metrics } from './metrics';
import { getMetrics } from './metrics';

type BrowserConfig = CaptureConfig['browser']['chromium'];

Expand All @@ -35,7 +35,7 @@ export class HeadlessChromiumDriverFactory {
private getChromiumArgs: () => string[];
private core: ReportingCore;

constructor(core: ReportingCore, binaryPath: string, logger: LevelLogger) {
constructor(core: ReportingCore, binaryPath: string, private logger: LevelLogger) {
this.core = core;
this.binaryPath = binaryPath;
const config = core.getConfig();
Expand All @@ -62,7 +62,7 @@ export class HeadlessChromiumDriverFactory {
*/
createPage(
{ browserTimezone }: { browserTimezone?: string },
pLogger: LevelLogger
pLogger = this.logger
): Rx.Observable<{ driver: HeadlessChromiumDriver; exit$: Rx.Observable<never> }> {
// FIXME: 'create' is deprecated
return Rx.Observable.create(async (observer: InnerSubscriber<unknown, unknown>) => {
Expand All @@ -72,10 +72,7 @@ export class HeadlessChromiumDriverFactory {
const chromiumArgs = this.getChromiumArgs();
logger.debug(`Chromium launch args set to: ${chromiumArgs}`);

let browser: puppeteer.Browser;
let page: puppeteer.Page;
let devTools: puppeteer.CDPSession | undefined;
let startMetrics: Metrics | undefined;
let browser: puppeteer.Browser | null = null;

try {
browser = await puppeteer.launch({
Expand All @@ -89,29 +86,28 @@ export class HeadlessChromiumDriverFactory {
TZ: browserTimezone,
},
});
} catch (err) {
observer.error(new Error(`Error spawning Chromium browser! ${err}`));
return;
}

page = await browser.newPage();
devTools = await page.target().createCDPSession();
const page = await browser.newPage();
const devTools = await page.target().createCDPSession();

await devTools.send('Performance.enable', { timeDomain: 'timeTicks' });
startMetrics = await devTools.send('Performance.getMetrics');
await devTools.send('Performance.enable', { timeDomain: 'timeTicks' });
const startMetrics = await devTools.send('Performance.getMetrics');

// Log version info for debugging / maintenance
const versionInfo = await devTools.send('Browser.getVersion');
logger.debug(`Browser version: ${JSON.stringify(versionInfo)}`);
// Log version info for debugging / maintenance
const versionInfo = await devTools.send('Browser.getVersion');
logger.debug(`Browser version: ${JSON.stringify(versionInfo)}`);

await page.emulateTimezone(browserTimezone);
await page.emulateTimezone(browserTimezone);

// Set the default timeout for all navigation methods to the openUrl timeout (30 seconds)
// All waitFor methods have their own timeout config passed in to them
page.setDefaultTimeout(durationToNumber(this.captureConfig.timeouts.openUrl));
// Set the default timeout for all navigation methods to the openUrl timeout
// All waitFor methods have their own timeout config passed in to them
page.setDefaultTimeout(durationToNumber(this.captureConfig.timeouts.openUrl));

logger.debug(`Browser page driver created`);
} catch (err) {
observer.error(new Error(`Error spawning Chromium browser!`));
observer.error(err);
throw err;
}
logger.debug(`Browser page driver created`);

const childProcess = {
async kill() {
Expand All @@ -134,7 +130,7 @@ export class HeadlessChromiumDriverFactory {
}

try {
await browser.close();
await browser?.close();
} catch (err) {
// do not throw
logger.error(err);
Expand Down

0 comments on commit 2aa6245

Please sign in to comment.