fix: validate ffmpeg on context creation (#33903)
This commit is contained in:
parent
081f455ee9
commit
e3629dc1df
|
@ -211,6 +211,10 @@ export class BidiBrowserContext extends BrowserContext {
|
|||
return this._bidiPages().map(bidiPage => bidiPage._initializedPage).filter(Boolean) as Page[];
|
||||
}
|
||||
|
||||
pagesOrErrors() {
|
||||
return this._bidiPages().map(bidiPage => bidiPage.pageOrError());
|
||||
}
|
||||
|
||||
async newPageDelegate(): Promise<PageDelegate> {
|
||||
assertBrowserContextIsNotOwned(this);
|
||||
const { context } = await this._browser._browserSession.send('browsingContext.create', {
|
||||
|
|
|
@ -259,6 +259,7 @@ export abstract class BrowserContext extends SdkObject {
|
|||
|
||||
// BrowserContext methods.
|
||||
abstract pages(): Page[];
|
||||
abstract pagesOrErrors(): Promise<Page | Error>[];
|
||||
abstract newPageDelegate(): Promise<PageDelegate>;
|
||||
abstract addCookies(cookies: channels.SetNetworkCookie[]): Promise<void>;
|
||||
abstract setGeolocation(geolocation?: types.Geolocation): Promise<void>;
|
||||
|
@ -358,29 +359,36 @@ export abstract class BrowserContext extends SdkObject {
|
|||
this._timeoutSettings.setDefaultTimeout(timeout);
|
||||
}
|
||||
|
||||
async _loadDefaultContextAsIs(progress: Progress): Promise<Page[]> {
|
||||
if (!this.pages().length) {
|
||||
async _loadDefaultContextAsIs(progress: Progress): Promise<Page> {
|
||||
let pageOrError;
|
||||
if (!this.pagesOrErrors().length) {
|
||||
const waitForEvent = helper.waitForEvent(progress, this, BrowserContext.Events.Page);
|
||||
progress.cleanupWhenAborted(() => waitForEvent.dispose);
|
||||
const page = (await waitForEvent.promise) as Page;
|
||||
if (page._pageIsError)
|
||||
throw page._pageIsError;
|
||||
// Race against BrowserContext.close
|
||||
pageOrError = await Promise.race([
|
||||
waitForEvent.promise as Promise<Page>,
|
||||
this._closePromise,
|
||||
]);
|
||||
// Consider Page initialization errors
|
||||
if (pageOrError instanceof Page)
|
||||
pageOrError = await pageOrError._delegate.pageOrError();
|
||||
} else {
|
||||
pageOrError = await this.pagesOrErrors()[0];
|
||||
}
|
||||
const pages = this.pages();
|
||||
if (pages[0]._pageIsError)
|
||||
throw pages[0]._pageIsError;
|
||||
await pages[0].mainFrame()._waitForLoadState(progress, 'load');
|
||||
return pages;
|
||||
if (pageOrError instanceof Error)
|
||||
throw pageOrError;
|
||||
await pageOrError.mainFrame()._waitForLoadState(progress, 'load');
|
||||
return pageOrError;
|
||||
}
|
||||
|
||||
async _loadDefaultContext(progress: Progress) {
|
||||
const pages = await this._loadDefaultContextAsIs(progress);
|
||||
const defaultPage = await this._loadDefaultContextAsIs(progress);
|
||||
const browserName = this._browser.options.name;
|
||||
if ((this._options.isMobile && browserName === 'chromium') || (this._options.locale && browserName === 'webkit')) {
|
||||
// Workaround for:
|
||||
// - chromium fails to change isMobile for existing page;
|
||||
// - webkit fails to change locale for existing page.
|
||||
const oldPage = pages[0];
|
||||
const oldPage = defaultPage;
|
||||
await this.newPage(progress.metadata);
|
||||
await oldPage.close(progress.metadata);
|
||||
}
|
||||
|
|
|
@ -368,6 +368,10 @@ export class CRBrowserContext extends BrowserContext {
|
|||
return this._crPages().map(crPage => crPage._initializedPage).filter(Boolean) as Page[];
|
||||
}
|
||||
|
||||
pagesOrErrors() {
|
||||
return this._crPages().map(crPage => crPage.pageOrError());
|
||||
}
|
||||
|
||||
async newPageDelegate(): Promise<PageDelegate> {
|
||||
assertBrowserContextIsNotOwned(this);
|
||||
|
||||
|
|
|
@ -432,6 +432,9 @@ class FrameSession {
|
|||
this._firstNonInitialNavigationCommittedFulfill = f;
|
||||
this._firstNonInitialNavigationCommittedReject = r;
|
||||
});
|
||||
// The Promise is not always awaited (e.g. FrameSession._initialize can throw)
|
||||
// so we catch errors here to prevent unhandled promise rejection.
|
||||
this._firstNonInitialNavigationCommittedPromise.catch(() => {});
|
||||
}
|
||||
|
||||
_isMainFrame(): boolean {
|
||||
|
|
|
@ -271,6 +271,10 @@ export class FFBrowserContext extends BrowserContext {
|
|||
return this._ffPages().map(ffPage => ffPage._initializedPage).filter(pageOrNull => !!pageOrNull) as Page[];
|
||||
}
|
||||
|
||||
pagesOrErrors() {
|
||||
return this._ffPages().map(ffPage => ffPage.pageOrError());
|
||||
}
|
||||
|
||||
async newPageDelegate(): Promise<PageDelegate> {
|
||||
assertBrowserContextIsNotOwned(this);
|
||||
const { targetId } = await this._browser.session.send('Browser.newPage', {
|
||||
|
|
|
@ -164,7 +164,6 @@ export class Page extends SdkObject {
|
|||
_clientRequestInterceptor: network.RouteHandler | undefined;
|
||||
_serverRequestInterceptor: network.RouteHandler | undefined;
|
||||
_ownedContext: BrowserContext | undefined;
|
||||
_pageIsError: Error | undefined;
|
||||
_video: Artifact | null = null;
|
||||
_opener: Page | undefined;
|
||||
private _isServerSideOnly = false;
|
||||
|
@ -208,7 +207,7 @@ export class Page extends SdkObject {
|
|||
// context/browser closure. Just ignore the page.
|
||||
if (this._browserContext.isClosingOrClosed())
|
||||
return;
|
||||
this._setIsError(error);
|
||||
this._frameManager.createDummyMainFrameIfNeeded();
|
||||
}
|
||||
this._initialized = true;
|
||||
this.emitOnContext(contextEvent, this);
|
||||
|
@ -709,11 +708,6 @@ export class Page extends SdkObject {
|
|||
await this._ownedContext.close(options);
|
||||
}
|
||||
|
||||
private _setIsError(error: Error) {
|
||||
this._pageIsError = error;
|
||||
this._frameManager.createDummyMainFrameIfNeeded();
|
||||
}
|
||||
|
||||
isClosed(): boolean {
|
||||
return this._closedState === 'closed';
|
||||
}
|
||||
|
|
|
@ -243,6 +243,10 @@ export class WKBrowserContext extends BrowserContext {
|
|||
return this._wkPages().map(wkPage => wkPage._initializedPage).filter(pageOrNull => !!pageOrNull) as Page[];
|
||||
}
|
||||
|
||||
pagesOrErrors() {
|
||||
return this._wkPages().map(wkPage => wkPage.pageOrError());
|
||||
}
|
||||
|
||||
async newPageDelegate(): Promise<PageDelegate> {
|
||||
assertBrowserContextIsNotOwned(this);
|
||||
const { pageProxyId } = await this._browser._browserSession.send('Playwright.createPage', { browserContextId: this._browserContextId });
|
||||
|
|
|
@ -14,6 +14,7 @@
|
|||
* limitations under the License.
|
||||
*/
|
||||
import { test, expect } from './npmTest';
|
||||
import { chromium } from '@playwright/test';
|
||||
import path from 'path';
|
||||
|
||||
test.use({ isolateBrowsers: true });
|
||||
|
@ -95,3 +96,46 @@ test('install playwright-chromium should work', async ({ exec, installedSoftware
|
|||
await exec('npx playwright install chromium');
|
||||
await exec('node sanity.js playwright-chromium chromium');
|
||||
});
|
||||
|
||||
test('should print error if recording video without ffmpeg', async ({ exec, writeFiles }) => {
|
||||
await exec('npm i playwright');
|
||||
|
||||
await writeFiles({
|
||||
'launch.js': `
|
||||
const playwright = require('playwright');
|
||||
(async () => {
|
||||
const browser = await playwright.chromium.launch({ executablePath: ${JSON.stringify(chromium.executablePath())} });
|
||||
try {
|
||||
const context = await browser.newContext({ recordVideo: { dir: 'videos' } });
|
||||
const page = await context.newPage();
|
||||
} finally {
|
||||
await browser.close();
|
||||
}
|
||||
})().catch(e => {
|
||||
console.error(e);
|
||||
process.exit(1);
|
||||
});
|
||||
`,
|
||||
'launchPersistentContext.js': `
|
||||
const playwright = require('playwright');
|
||||
process.on('unhandledRejection', (e) => console.error('unhandledRejection', e));
|
||||
(async () => {
|
||||
const context = await playwright.chromium.launchPersistentContext('', { executablePath: ${JSON.stringify(chromium.executablePath())}, recordVideo: { dir: 'videos' } });
|
||||
})().catch(e => {
|
||||
console.error(e);
|
||||
process.exit(1);
|
||||
});
|
||||
`,
|
||||
});
|
||||
|
||||
await test.step('BrowserType.launch', async () => {
|
||||
const result = await exec('node', 'launch.js', { expectToExitWithError: true });
|
||||
expect(result).toContain(`browserContext.newPage: Executable doesn't exist at`);
|
||||
});
|
||||
|
||||
await test.step('BrowserType.launchPersistentContext', async () => {
|
||||
const result = await exec('node', 'launchPersistentContext.js', { expectToExitWithError: true });
|
||||
expect(result).not.toContain('unhandledRejection');
|
||||
expect(result).toContain(`browserType.launchPersistentContext: Executable doesn't exist at`);
|
||||
});
|
||||
});
|
||||
|
|
Loading…
Reference in New Issue