From 2885f9230932d62d3c52721813a0693c67b1f4c9 Mon Sep 17 00:00:00 2001 From: AVVS Date: Thu, 25 Jul 2019 12:41:22 -0700 Subject: [PATCH] fix: disposing of tabs & targets --- __tests__/unit/chrome.spec.js | 17 +++++---- src/utils/chrome.js | 66 ++++++++++++++++++++++++----------- 2 files changed, 57 insertions(+), 26 deletions(-) diff --git a/__tests__/unit/chrome.spec.js b/__tests__/unit/chrome.spec.js index 17ab23f..5e7291d 100644 --- a/__tests__/unit/chrome.spec.js +++ b/__tests__/unit/chrome.spec.js @@ -21,12 +21,17 @@ describe('chrome', () => { }); it('should able to open a tab', async () => { - return Promise.using(chrome.openTab(), async ({ Page }) => { - await Page.navigate({ url }); - await Page.loadEventFired(); - - // TODO: check page contents - }); + const tab = await chrome.openTab(); + const { client: { Page } } = tab; + + try { + await Promise.all([ + Page.navigate({ url }), + Page.loadEventFired(), + ]); + } finally { + await Chrome.disposer(tab.target, tab.client); + } }); it('should able to render a pdf', async () => { diff --git a/src/utils/chrome.js b/src/utils/chrome.js index e0a48fe..98af589 100644 --- a/src/utils/chrome.js +++ b/src/utils/chrome.js @@ -55,7 +55,11 @@ class Chrome { }, restOpts); // use custom logger if provided - this.log = logger || debug; + this.log = logger || { + info: (...args) => debug('[info]', ...args), + debug: (...args) => debug('[debug]', ...args), + }; + this.onLog = params => this.log.info(params.message.text); this.timeout = timeout; @@ -149,6 +153,17 @@ class Chrome { return this.status(1); } + /** + * Disposer for assosiated tab/client + * @param {Target} target + * @param {ChromeRemote} client + * @return {Promise} + */ + static async disposer(target, client) { + if (target) await ChromeRemote.Close({ id: target.id }).catch(noop); + if (client) await client.close().catch(noop); + } + /** * Opens a new Chrome tab * @return {Promise} @@ -156,19 +171,26 @@ class Chrome { async openTab() { await this.waitForStart(); - return Promise - .bind(ChromeRemote, this.launcher) - .then(ChromeRemote.New) - .then(target => ChromeRemote({ target, port: this.launcher.port })) - .tap((tab) => { - const { Network, Page, Console } = tab; + let client; + let target; + + try { + target = await ChromeRemote.New(this.launcher); + this.log.debug({ target }, 'opened new target'); + client = await ChromeRemote({ target, port: this.launcher.port }); - Console.messageAdded(this.onLog); + const { Network, Page, Console } = client; + Console.messageAdded(this.onLog); - return Promise.all([Page.enable(), Network.enable()]); - }) - .timeout(this.timeout) - .disposer(tab => tab.close().catch(noop)); + await Promise + .all([Page.enable(), Network.enable()]) + .timeout(this.timeout); + } catch (e) { + Chrome.disposer(target, client); + throw e; + } + + return { target, client }; } /** @@ -177,22 +199,26 @@ class Chrome { * @param {Object} opts page printing options * @return {Promise} base64 encoded PDF document */ - printToPdf(html, opts = {}) { - return Promise.using(this.openTab(), ({ Page }) => { + async printToPdf(html, opts = {}) { + let tab; + try { + tab = await this.openTab(); + const { client: { Page } } = tab; + const url = /^(https?|file|data):/i.test(html) ? html : `data:text/html;charset=utf-8;base64,${encode(Buffer.from(html))}`; - return Promise - .all([ - Page.loadEventFired(), - Page.navigate({ url }), - ]) + return await Promise + .all([Page.loadEventFired(), Page.navigate({ url })]) .return(Page) .call('printToPDF', opts) .get('data') .timeout(this.timeout); - }); + } finally { + // schedules in the "background", no await is needed + Chrome.disposer(tab.target, tab.client); + } } }