From 2aa624521045dd49134411b700838bdac3e0e7e4 Mon Sep 17 00:00:00 2001 From: Tim Sullivan Date: Wed, 10 Nov 2021 14:05:28 -0700 Subject: [PATCH] [Reporting] Fix unhandled promise rejection thrown when launching Chromium (#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 Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Michael Dokolin --- .../chromium/driver_factory/index.test.ts | 76 +++++++++++++++++++ .../browsers/chromium/driver_factory/index.ts | 46 +++++------ 2 files changed, 97 insertions(+), 25 deletions(-) create mode 100644 x-pack/plugins/reporting/server/browsers/chromium/driver_factory/index.test.ts diff --git a/x-pack/plugins/reporting/server/browsers/chromium/driver_factory/index.test.ts b/x-pack/plugins/reporting/server/browsers/chromium/driver_factory/index.test.ts new file mode 100644 index 0000000000000..dae692fae8825 --- /dev/null +++ b/x-pack/plugins/reporting/server/browsers/chromium/driver_factory/index.test.ts @@ -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).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).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."` + ); + }); +}); diff --git a/x-pack/plugins/reporting/server/browsers/chromium/driver_factory/index.ts b/x-pack/plugins/reporting/server/browsers/chromium/driver_factory/index.ts index 264e673d2bf74..2aef62f59985b 100644 --- a/x-pack/plugins/reporting/server/browsers/chromium/driver_factory/index.ts +++ b/x-pack/plugins/reporting/server/browsers/chromium/driver_factory/index.ts @@ -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']; @@ -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(); @@ -62,7 +62,7 @@ export class HeadlessChromiumDriverFactory { */ createPage( { browserTimezone }: { browserTimezone?: string }, - pLogger: LevelLogger + pLogger = this.logger ): Rx.Observable<{ driver: HeadlessChromiumDriver; exit$: Rx.Observable }> { // FIXME: 'create' is deprecated return Rx.Observable.create(async (observer: InnerSubscriber) => { @@ -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({ @@ -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() { @@ -134,7 +130,7 @@ export class HeadlessChromiumDriverFactory { } try { - await browser.close(); + await browser?.close(); } catch (err) { // do not throw logger.error(err);