Skip to content

Commit a403d4b

Browse files
authored
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.
1 parent 1cebf87 commit a403d4b

File tree

3 files changed

+13
-3
lines changed

3 files changed

+13
-3
lines changed

src/server/processLauncher.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -166,11 +166,12 @@ export async function launchProcess(options: LaunchProcessOptions): Promise<Laun
166166
export function waitForLine(progress: Progress, process: childProcess.ChildProcess, inputStream: stream.Readable, regex: RegExp): Promise<RegExpMatchArray> {
167167
return new Promise((resolve, reject) => {
168168
const rl = readline.createInterface({ input: inputStream });
169+
const failError = new Error('Process failed to launch!');
169170
const listeners = [
170171
helper.addEventListener(rl, 'line', onLine),
171-
helper.addEventListener(rl, 'close', reject),
172-
helper.addEventListener(process, 'exit', reject),
173-
helper.addEventListener(process, 'error', reject)
172+
helper.addEventListener(rl, 'close', reject.bind(null, failError)),
173+
helper.addEventListener(process, 'exit', reject.bind(null, failError)),
174+
helper.addEventListener(process, 'error', reject.bind(null, failError))
174175
];
175176

176177
progress.cleanupWhenAborted(cleanup);
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
#!/usr/bin/env node
2+
3+
process.exit(1);

test/launcher.spec.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,12 @@ describe('Playwright', function() {
4444
await browserType.launch(options).catch(e => waitError = e);
4545
expect(waitError.message).toContain('can not specify page');
4646
});
47+
it('should reject if launched browser fails immediately', async({browserType, defaultBrowserOptions}) => {
48+
const options = Object.assign({}, defaultBrowserOptions, {executablePath: path.join(__dirname, 'assets', 'dummy_bad_browser_executable.js')});
49+
let waitError = null;
50+
await browserType.launch(options).catch(e => waitError = e);
51+
expect(waitError.message).toContain('browserType.launch logs');
52+
});
4753
it('should reject if executable path is invalid', async({browserType, defaultBrowserOptions}) => {
4854
let waitError = null;
4955
const options = Object.assign({}, defaultBrowserOptions, {executablePath: 'random-invalid-path'});

0 commit comments

Comments
 (0)