From a403d4beff0a024917913e206625cbbff06c6b05 Mon Sep 17 00:00:00 2001 From: Andrey Lushnikov Date: Fri, 10 Jul 2020 16:04:19 -0700 Subject: [PATCH] fix(firefox): fix launching firefox without dependencies (#2900) We always have to reject promises with some error. Otherwise, our error-rewriting logic in try-catch miserably fails. With this patch, attempt to launch Firefox when it's missing dependencies will actually result in a thrown exception with pretty logs. Without this patch, Playwright throws internal error. --- src/server/processLauncher.ts | 7 ++++--- test/assets/dummy_bad_browser_executable.js | 3 +++ test/launcher.spec.js | 6 ++++++ 3 files changed, 13 insertions(+), 3 deletions(-) create mode 100755 test/assets/dummy_bad_browser_executable.js diff --git a/src/server/processLauncher.ts b/src/server/processLauncher.ts index d323d1bb4f..08d2da0aa3 100644 --- a/src/server/processLauncher.ts +++ b/src/server/processLauncher.ts @@ -166,11 +166,12 @@ export async function launchProcess(options: LaunchProcessOptions): Promise { return new Promise((resolve, reject) => { const rl = readline.createInterface({ input: inputStream }); + const failError = new Error('Process failed to launch!'); const listeners = [ helper.addEventListener(rl, 'line', onLine), - helper.addEventListener(rl, 'close', reject), - helper.addEventListener(process, 'exit', reject), - helper.addEventListener(process, 'error', reject) + helper.addEventListener(rl, 'close', reject.bind(null, failError)), + helper.addEventListener(process, 'exit', reject.bind(null, failError)), + helper.addEventListener(process, 'error', reject.bind(null, failError)) ]; progress.cleanupWhenAborted(cleanup); diff --git a/test/assets/dummy_bad_browser_executable.js b/test/assets/dummy_bad_browser_executable.js new file mode 100755 index 0000000000..92f1453c55 --- /dev/null +++ b/test/assets/dummy_bad_browser_executable.js @@ -0,0 +1,3 @@ +#!/usr/bin/env node + +process.exit(1); diff --git a/test/launcher.spec.js b/test/launcher.spec.js index 5dea06ee73..e2f612faa7 100644 --- a/test/launcher.spec.js +++ b/test/launcher.spec.js @@ -44,6 +44,12 @@ describe('Playwright', function() { await browserType.launch(options).catch(e => waitError = e); expect(waitError.message).toContain('can not specify page'); }); + it('should reject if launched browser fails immediately', async({browserType, defaultBrowserOptions}) => { + const options = Object.assign({}, defaultBrowserOptions, {executablePath: path.join(__dirname, 'assets', 'dummy_bad_browser_executable.js')}); + let waitError = null; + await browserType.launch(options).catch(e => waitError = e); + expect(waitError.message).toContain('browserType.launch logs'); + }); it('should reject if executable path is invalid', async({browserType, defaultBrowserOptions}) => { let waitError = null; const options = Object.assign({}, defaultBrowserOptions, {executablePath: 'random-invalid-path'});