Skip to content

Commit

Permalink
chore(stable): throw user-friendly message when ffmpeg is missing (#5865
Browse files Browse the repository at this point in the history
)
  • Loading branch information
pavelfeldman authored Mar 18, 2021
1 parent 141583c commit 2367039
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 19 deletions.
18 changes: 17 additions & 1 deletion src/server/chromium/crPage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ import { CRBrowserContext } from './crBrowser';
import * as types from '../types';
import { ConsoleMessage } from '../console';
import { rewriteErrorMessage } from '../../utils/stackTrace';
import { assert, headersArrayToObject, createGuid } from '../../utils/utils';
import { assert, headersArrayToObject, createGuid, canAccessFile } from '../../utils/utils';
import { VideoRecorder } from './videoRecorder';


Expand Down Expand Up @@ -816,6 +816,22 @@ class FrameSession {
const ffmpegPath = this._crPage._browserContext._browser.options.registry.executablePath('ffmpeg');
if (!ffmpegPath)
throw new Error('ffmpeg executable was not found');
if (!canAccessFile(ffmpegPath)) {
let message: string = '';
switch (this._page._browserContext._options.sdkLanguage) {
case 'python': message = 'playwright install ffmpeg'; break;
case 'python-async': message = 'playwright install ffmpeg'; break;
case 'javascript': message = 'npx playwright install ffmpeg'; break;
case 'java': message = 'mvn exec:java -e -Dexec.mainClass=com.microsoft.playwright.CLI -Dexec.args="install ffmpeg"'; break;
}
throw new Error(`
============================================================
Please install ffmpeg in order to record video.
$ ${message}
============================================================
`);
}
this._videoRecorder = await VideoRecorder.launch(this._crPage._page, ffmpegPath, options);
this._screencastId = screencastId;
const gotFirstFrame = new Promise(f => this._client.once('Page.screencastFrame', f));
Expand Down
18 changes: 3 additions & 15 deletions src/server/chromium/findChromiumChannel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@
* limitations under the License.
*/

import fs from 'fs';
import path from 'path';
import { canAccessFile } from '../../utils/utils';

function darwin(channel: string): string | undefined {
switch (channel) {
Expand Down Expand Up @@ -60,24 +60,12 @@ function win32(channel: string): string | undefined {
let result: string | undefined;
prefixes.forEach(prefix => {
const chromePath = path.join(prefix, suffix!);
if (canAccess(chromePath))
if (canAccessFile(chromePath))
result = chromePath;
});
return result;
}

function canAccess(file: string) {
if (!file)
return false;

try {
fs.accessSync(file);
return true;
} catch (e) {
return false;
}
}

export function findChromiumChannel(channel: string): string {
let result: string | undefined;
if (process.platform === 'linux')
Expand All @@ -90,7 +78,7 @@ export function findChromiumChannel(channel: string): string {
if (!result)
throw new Error(`Chromium distribution '${channel}' is not supported on ${process.platform}`);

if (canAccess(result))
if (canAccessFile(result))
return result;
throw new Error(`Chromium distribution was not found: ${channel}`);
}
4 changes: 2 additions & 2 deletions src/server/processLauncher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ export async function launchProcess(options: LaunchProcessOptions): Promise<Laun
let failed: (e: Error) => void;
const failedPromise = new Promise<Error>((f, r) => failed = f);
spawnedProcess.once('error', error => {
failed(new Error('Failed to launch browser: ' + error));
failed(new Error('Failed to launch: ' + error));
});
return cleanup().then(() => failedPromise).then(e => Promise.reject(e));
}
Expand Down Expand Up @@ -164,7 +164,7 @@ export async function launchProcess(options: LaunchProcessOptions): Promise<Laun
// Force kill the browser.
try {
if (process.platform === 'win32')
childProcess.execSync(`taskkill /pid ${spawnedProcess.pid} /T /F`);
childProcess.execSync(`taskkill /pid ${spawnedProcess.pid} /T /F`, { stdio: 'ignore' });
else
process.kill(-spawnedProcess.pid, 'SIGKILL');
} catch (e) {
Expand Down
12 changes: 12 additions & 0 deletions src/utils/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -158,3 +158,15 @@ export async function removeFolders(dirs: string[]) {
});
}));
}

export function canAccessFile(file: string) {
if (!file)
return false;

try {
fs.accessSync(file);
return true;
} catch (e) {
return false;
}
}
3 changes: 2 additions & 1 deletion test/defaultbrowsercontext-2.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,9 @@ it('should restore state from userDataDir', (test, { browserName }) => {
await browserContext3.close();
});

it('should restore cookies from userDataDir', (test, { browserName }) => {
it('should restore cookies from userDataDir', (test, { platform, browserChannel }) => {
test.slow();
test.fixme(platform === 'win32' && browserChannel === 'chrome');
}, async ({browserType, browserOptions, server, createUserDataDir}) => {
const userDataDir = await createUserDataDir();
const browserContext = await browserType.launchPersistentContext(userDataDir, browserOptions);
Expand Down

0 comments on commit 2367039

Please sign in to comment.