From 6b18e8cef52bd9667ca5510b136c17e82817c762 Mon Sep 17 00:00:00 2001 From: Andrey Lushnikov Date: Thu, 7 Feb 2019 15:18:43 -0800 Subject: [PATCH] feat(firefox): introduce async stacks for Puppeteer-Firefox (#3948) This patch refactors Puppeteer-Firefox code to declare public API in `/lib/api.js` and use it to setup async stack hooks over the public API method calls. --- experimental/puppeteer-firefox/index.js | 47 ++++--------------- experimental/puppeteer-firefox/lib/Browser.js | 2 +- .../puppeteer-firefox/lib/BrowserFetcher.js | 2 +- .../puppeteer-firefox/lib/Launcher.js | 20 ++++++-- experimental/puppeteer-firefox/lib/Page.js | 2 +- .../puppeteer-firefox/lib/Puppeteer.js | 27 +++++++++++ experimental/puppeteer-firefox/lib/api.js | 16 +++++++ experimental/puppeteer-firefox/lib/helper.js | 21 +++++++++ test/page.spec.js | 4 +- 9 files changed, 93 insertions(+), 48 deletions(-) create mode 100644 experimental/puppeteer-firefox/lib/Puppeteer.js create mode 100644 experimental/puppeteer-firefox/lib/api.js diff --git a/experimental/puppeteer-firefox/index.js b/experimental/puppeteer-firefox/index.js index 3c6c81d0a05..08b942520ef 100644 --- a/experimental/puppeteer-firefox/index.js +++ b/experimental/puppeteer-firefox/index.js @@ -13,44 +13,13 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -const FirefoxLauncher = require('./lib/Launcher.js').Launcher; -const BrowserFetcher = require('./lib/BrowserFetcher.js'); -class Puppeteer { - constructor() { - this._firefoxLauncher = new FirefoxLauncher(); - } +const {helper} = require('./lib/helper'); +const api = require('./lib/api'); +for (const className in api) + helper.installAsyncStackHooks(api[className]); - async launch(options = {}) { - const { - args = [], - dumpio = !!process.env.DUMPIO, - handleSIGHUP = true, - handleSIGINT = true, - handleSIGTERM = true, - headless = (process.env.HEADLESS || 'true').trim().toLowerCase() === 'true', - defaultViewport = {width: 800, height: 600}, - ignoreHTTPSErrors = false, - slowMo = 0, - executablePath = this.executablePath(), - } = options; - options = { - args, slowMo, dumpio, executablePath, handleSIGHUP, handleSIGINT, handleSIGTERM, headless, defaultViewport, - ignoreHTTPSErrors - }; - return await this._firefoxLauncher.launch(options); - } - - createBrowserFetcher(options) { - return new BrowserFetcher(__dirname, options); - } - - executablePath() { - const browserFetcher = new BrowserFetcher(__dirname, { product: 'firefox' }); - const revision = require('./package.json').puppeteer.firefox_revision; - const revisionInfo = browserFetcher.revisionInfo(revision); - return revisionInfo.executablePath; - } -} - -module.exports = new Puppeteer(); +const {Puppeteer} = require('./lib/Puppeteer'); +const packageJson = require('./package.json'); +const preferredRevision = packageJson.puppeteer.firefox_revision; +module.exports = new Puppeteer(__dirname, preferredRevision); diff --git a/experimental/puppeteer-firefox/lib/Browser.js b/experimental/puppeteer-firefox/lib/Browser.js index 58ff5ed06fd..9893e528dd9 100644 --- a/experimental/puppeteer-firefox/lib/Browser.js +++ b/experimental/puppeteer-firefox/lib/Browser.js @@ -312,4 +312,4 @@ BrowserContext.Events = { TargetDestroyed: 'targetdestroyed' } -module.exports = {Browser, Target}; +module.exports = {Browser, BrowserContext, Target}; diff --git a/experimental/puppeteer-firefox/lib/BrowserFetcher.js b/experimental/puppeteer-firefox/lib/BrowserFetcher.js index b9c5eafe16d..8a645e045a5 100644 --- a/experimental/puppeteer-firefox/lib/BrowserFetcher.js +++ b/experimental/puppeteer-firefox/lib/BrowserFetcher.js @@ -228,7 +228,7 @@ class BrowserFetcher { } } -module.exports = BrowserFetcher; +module.exports = {BrowserFetcher}; /** * @param {string} folderPath diff --git a/experimental/puppeteer-firefox/lib/Launcher.js b/experimental/puppeteer-firefox/lib/Launcher.js index d3d0e24d22e..2786e064e87 100644 --- a/experimental/puppeteer-firefox/lib/Launcher.js +++ b/experimental/puppeteer-firefox/lib/Launcher.js @@ -19,6 +19,7 @@ const removeFolder = require('rimraf'); const childProcess = require('child_process'); const {Connection} = require('./Connection'); const {Browser} = require('./Browser'); +const {BrowserFetcher} = require('./BrowserFetcher'); const readline = require('readline'); const fs = require('fs'); const util = require('util'); @@ -35,6 +36,11 @@ const FIREFOX_PROFILE_PATH = path.join(os.tmpdir(), 'puppeteer_firefox_profile-' * @internal */ class Launcher { + constructor(projectRoot, preferredRevision) { + this._projectRoot = projectRoot; + this._preferredRevision = preferredRevision; + } + /** * @param {Object} options * @return {!Promise} @@ -43,7 +49,7 @@ class Launcher { const { args = [], dumpio = false, - executablePath = null, + executablePath = this.executablePath(), handleSIGHUP = true, handleSIGINT = true, handleSIGTERM = true, @@ -53,9 +59,6 @@ class Launcher { slowMo = 0, } = options; - if (!executablePath) - throw new Error('Firefox launching is only supported with local version of firefox!'); - const firefoxArguments = args.slice(); firefoxArguments.push('-no-remote'); firefoxArguments.push('-juggler', '0'); @@ -152,6 +155,15 @@ class Launcher { } catch (e) { } } } + + /** + * @return {string} + */ + executablePath() { + const browserFetcher = new BrowserFetcher(this._projectRoot, { product: 'firefox' }); + const revisionInfo = browserFetcher.revisionInfo(this._preferredRevision); + return revisionInfo.executablePath; + } } /** diff --git a/experimental/puppeteer-firefox/lib/Page.js b/experimental/puppeteer-firefox/lib/Page.js index 39943300bc2..367e33bd4f4 100644 --- a/experimental/puppeteer-firefox/lib/Page.js +++ b/experimental/puppeteer-firefox/lib/Page.js @@ -1446,4 +1446,4 @@ class NavigationWatchdog { } } -module.exports = {Page}; +module.exports = {Page, Frame, ConsoleMessage}; diff --git a/experimental/puppeteer-firefox/lib/Puppeteer.js b/experimental/puppeteer-firefox/lib/Puppeteer.js new file mode 100644 index 00000000000..b1beffe280d --- /dev/null +++ b/experimental/puppeteer-firefox/lib/Puppeteer.js @@ -0,0 +1,27 @@ +const {Launcher} = require('./Launcher.js'); +const {BrowserFetcher} = require('./BrowserFetcher.js'); + +class Puppeteer { + /** + * @param {string} projectRoot + * @param {string} preferredRevision + */ + constructor(projectRoot, preferredRevision) { + this._projectRoot = projectRoot; + this._launcher = new Launcher(projectRoot, preferredRevision); + } + + async launch(options = {}) { + return this._launcher.launch(options); + } + + createBrowserFetcher(options) { + return new BrowserFetcher(this._projectRoot, options); + } + + executablePath() { + return this._launcher.executablePath(); + } +} + +module.exports = {Puppeteer}; diff --git a/experimental/puppeteer-firefox/lib/api.js b/experimental/puppeteer-firefox/lib/api.js new file mode 100644 index 00000000000..d1a77087ca4 --- /dev/null +++ b/experimental/puppeteer-firefox/lib/api.js @@ -0,0 +1,16 @@ +module.exports = { + Browser: require('./Browser').Browser, + BrowserContext: require('./Browser').BrowserContext, + BrowserFetcher: require('./BrowserFetcher').BrowserFetcher, + ConsoleMessage: require('./Page').ConsoleMessage, + Dialog: require('./Dialog').Dialog, + ElementHandle: require('./JSHandle').ElementHandle, + Frame: require('./Page').Frame, + JSHandle: require('./JSHandle').JSHandle, + Keyboard: require('./Input').Keyboard, + Mouse: require('./Input').Mouse, + Page: require('./Page').Page, + Puppeteer: require('./Puppeteer').Puppeteer, + Target: require('./Browser').Target, + TimeoutError: require('./Errors').TimeoutError, +}; diff --git a/experimental/puppeteer-firefox/lib/helper.js b/experimental/puppeteer-firefox/lib/helper.js index 1e4ba423e4a..6aeaba01f7c 100644 --- a/experimental/puppeteer-firefox/lib/helper.js +++ b/experimental/puppeteer-firefox/lib/helper.js @@ -19,6 +19,27 @@ const {TimeoutError} = require('./Errors'); * @internal */ class Helper { + /** + * @param {!Object} classType + */ + static installAsyncStackHooks(classType) { + for (const methodName of Reflect.ownKeys(classType.prototype)) { + const method = Reflect.get(classType.prototype, methodName); + if (methodName === 'constructor' || typeof methodName !== 'string' || methodName.startsWith('_') || typeof method !== 'function' || method.constructor.name !== 'AsyncFunction') + continue; + Reflect.set(classType.prototype, methodName, function(...args) { + const syncStack = new Error(); + return method.call(this, ...args).catch(e => { + const stack = syncStack.stack.substring(syncStack.stack.indexOf('\n') + 1); + const clientStack = stack.substring(stack.indexOf('\n')); + if (!e.stack.includes(clientStack)) + e.stack += '\n -- ASYNC --\n' + stack; + throw e; + }); + }); + } + } + /** * @param {Function|string} fun * @param {!Array<*>} args diff --git a/test/page.spec.js b/test/page.spec.js index f454ad18340..8bb6392bc9c 100644 --- a/test/page.spec.js +++ b/test/page.spec.js @@ -79,7 +79,7 @@ module.exports.addTests = function({testRunner, expect, headless, Errors, Device }); }); - (asyncawait ? describe_fails_ffox : xdescribe)('Async stacks', () => { + (asyncawait ? describe : xdescribe)('Async stacks', () => { it('should work', async({page, server}) => { server.setRoute('/empty.html', (req, res) => { res.statusCode = 204; @@ -88,7 +88,7 @@ module.exports.addTests = function({testRunner, expect, headless, Errors, Device let error = null; await page.goto(server.EMPTY_PAGE).catch(e => error = e); expect(error).not.toBe(null); - expect(error.message).toContain('net::ERR_ABORTED'); + expect(error.stack).toContain(__filename); }); });